-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update retrying client documentation #352
Conversation
ba56026
to
85726ca
Compare
docs/RetryingTarantoolClient.md
Outdated
Example of client API usage if you specify using the default CRUD proxy operations mapping configuration. | ||
https://github.com/tarantool/cartridge-java/blob/8a880423da1ce2bc0e82557d70ab46c9e7eba618/src/test/java/io/tarantool/driver/integration/ProxyTarantoolClientExampleIT.java#L64-L107 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can have a more visual example and complex example for it cause it's more complex API:
...
.withProxyMethodMapping(builder -> builder.withDeleteFunctionName("delete_with_error_if_not_found"))
.withRetryingByNumberOfAttempts(5,
// You can use default predicates from TarantoolRequestRetryPolicies for checking errors
retryNetworkErrors()
// Also you can use your own predicates and combine them with each other
.or(e -> e.getMessage().contains("Unsuccessful attempt"))
// Or with defaults
.or(retryTarantoolNoSuchProcedureErrors())
.or(retryDeleteError()),
// Also you can set delay in millisecond between attempts
factory -> factory.withDelay(300)
)
...
space(...).delete(); // retrying
space(...).insert();
cb018c6
to
9ea2638
Compare
// Start another thread that will insert value after 100 ms | ||
new Thread(() -> { | ||
try { | ||
Thread.sleep(100); | ||
} catch (InterruptedException e) { | ||
e.printStackTrace(); | ||
} | ||
space.insert(tupleFactory.create(1, null, "FIO", 50, 100)).join(); | ||
}).start(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You dont' have to use Thread API, you can't guarantee that your insert method will be called after deletion. It'd better to use CompletableFuture API, for e.g.
AtomicInteger retryingCounter = new...
...
.or(e -> {
retryingCounter.addAndGet();
return e.getMessage().contains("Records not found");
})
...
CompletableFuture deleteFuture = retryingClient.space(SPACE_NAME).delete(conditions);
retry(() -> assertEquals(1, retryingCounter.get()))
space.insert(tupleFactory.create(1, null, "FIO", 50, 100)).get();
deleteFuture.get();
But for this you need it in TarantoolUtils.java
public static Integer DEFAULT_RETRYING_ATTEMPTS = 5;
public static Integer DEFAULT_RETRYING_DELAY = 100;
public static void retry(Runnable fn) throws InterruptedException {
retry(DEFAULT_RETRYING_ATTEMPTS, DEFAULT_RETRYING_DELAY, fn);
}
public static void retry(Integer attempts, Integer delay, Runnable fn) throws InterruptedException {
while (attempts > 0) {
try {
fn.run();
return;
} catch (AssertionError ignored) {
}
--attempts;
Thread.sleep(delay);
}
fn.run();
}
Update getRetryingTarantoolClient in ReconnectIT to use it in retrying client documentation Needed for #309
Update retrying client documentation according to our new documentation policy Closes #309
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still don't believe that it works from the first attempt, but good for you :)
``` | ||
connection pool and basic client settings, and only augment the behavior of the client. | ||
|
||
In this example I use custom delete function. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"I use" -> "we use" looks better I believe
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or "let's consider a custom delete function"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"I use" -> "we use" looks better I believe
I don't think so
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or "let's consider a custom delete function"
This looks better. I'll increase the formality of language in documentation soon #356
Update retrying client documentation
I haven't forgotten about:
Related issues:
Closes #309