RobertIndie commented on code in PR #23686:
URL: https://github.com/apache/pulsar/pull/23686#discussion_r1875469975
##########
pulsar-metadata/src/main/java/org/apache/pulsar/metadata/cache/impl/MetadataCacheImpl.java:
##########
@@ -321,22 +325,25 @@ public void accept(Notification t) {
}
}
- private CompletableFuture<T>
executeWithRetry(Supplier<CompletableFuture<T>> op, String key) {
- CompletableFuture<T> result = new CompletableFuture<>();
+ private void execute(Supplier<CompletableFuture<T>> op, String key,
CompletableFuture<T> result, Backoff backoff) {
op.get().thenAccept(result::complete).exceptionally((ex) -> {
if (ex.getCause() instanceof BadVersionException) {
// if resource is updated by other than metadata-cache then
metadata-cache will get bad-version
// exception. so, try to invalidate the cache and try one more
time.
objCache.synchronous().invalidate(key);
- op.get().thenAccept(result::complete).exceptionally((ex1) -> {
- result.completeExceptionally(ex1.getCause());
- return null;
- });
+ backoffExecutor.schedule(() -> execute(op, key, result,
backoff), backoff.next(),
+ TimeUnit.MILLISECONDS);
return null;
}
result.completeExceptionally(ex.getCause());
return null;
});
+ }
+
+ private CompletableFuture<T>
executeWithRetry(Supplier<CompletableFuture<T>> op, String key) {
+ final var backoff = new Backoff(100, TimeUnit.MILLISECONDS, 1,
TimeUnit.MINUTES, 0, TimeUnit.MILLISECONDS);
Review Comment:
> Why not just add Backoff as its field?
OK. That's also fine.
> Regarding the timeout, I mean if a modification operation is not done
after the timeout, just fail the future with TimeoutException. It's independent
from the Backoff
Should the caller or the metadata cache fail the future? My idea is to let
the MetadataCache fail the future with the TimeoutException, so we could use
the backoff mandatory stop time as the timeout. But if it's the caller, then it
doesn't make sense. Failing the future this way won't stop the retry task. We
could check the future before each retry, but the bigger issue is that the
current caller won't fail the future. Without introducing a mandatory stop
time, the retry task could run indefinitely.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
