codelipenghui commented on code in PR #23686:
URL: https://github.com/apache/pulsar/pull/23686#discussion_r1873819885
##########
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:
After reviewing the details, it seems the retry mechanism is primarily
designed to avoid invalidating the cache on the broker. However, I believe this
is also incorrectly implemented.
Retries should not occur when another user has updated the data to a new
version. Instead, the system should throw a ConcurrentModificationException.
This allows users to review the updated data before deciding whether to proceed
with their changes. For example, if two users attempt to set the data retention
policy for a namespace, one user should receive a
ConcurrentModificationException if there is a conflict. Typically, this
requires the user to check the latest policy before retrying.
Regarding the incorrect implementation of the retry mechanism, it should
verify the data version between the cache and the metadata store. If the
versions match, retries should stop, as this indicates the data has been
updated elsewhere.
--
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]
