[GitHub] [pulsar] merlimat commented on a change in pull request #9412: [pulsar-broker] Handle bad-version in metadata-store if broker updates zk-metadata from zk-client

[GitBox <gi...@apache.org>]

merlimat commented on a change in pull request #9412:
URL: https://github.com/apache/pulsar/pull/9412#discussion_r568211593



##########
File path: pulsar-metadata/src/main/java/org/apache/pulsar/metadata/cache/impl/MetadataCacheImpl.java
##########
@@ -253,4 +255,25 @@ public void accept(Notification t) {
             break;
         }
     }
+
+    private CompletableFuture<Void> executeWithRetry(Supplier<CompletableFuture<Void>> op, String key) {
+        CompletableFuture<Void> result = new CompletableFuture<>();
+        op.get().handle((r, ex) -> {

Review comment:
       We should try to avoid `handle()` because any exception that is thrown in the handler will cause the `result` future not to be completed.

##########
File path: pulsar-metadata/src/test/java/org/apache/pulsar/metadata/MetadataCacheTest.java
##########
@@ -258,4 +263,28 @@ public void readModifyUpdateOrCreate(String provider, String url) throws Excepti
         assertEquals(newValue1.get().b, 2);
     }
 
+    @Test
+    public void readModifyUpdateBadVersionRetry() throws Exception {
+        String url =  zks.getConnectionString(); 
+        @Cleanup
+        ZKMetadataStore store = (ZKMetadataStore) MetadataStoreFactory.create(url, MetadataStoreConfig.builder().build());

Review comment:
       Instead of making the test to be ZK specific, we should instead try to make a write from a different instance of the `MetatadataStore`, writing into the same data.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org