Technoboy- commented on a change in pull request #14643:
URL: https://github.com/apache/pulsar/pull/14643#discussion_r825249794



##########
File path: 
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java
##########
@@ -1391,44 +1391,40 @@ public void openLedgerComplete(ManagedLedger ledger, 
Object ctx) {
                                 PersistentTopic persistentTopic = 
isSystemTopic(topic)
                                         ? new SystemTopic(topic, ledger, 
BrokerService.this)
                                         : new PersistentTopic(topic, ledger, 
BrokerService.this);
-                                CompletableFuture<Void> 
preCreateSubForCompaction =
-                                        
persistentTopic.preCreateSubscriptionForCompactionIfNeeded();
-                                CompletableFuture<Void> replicationFuture = 
persistentTopic
+                                persistentTopic
                                         .initialize()
-                                        .thenCompose(__ -> 
persistentTopic.checkReplication());
-
-                                
CompletableFuture.allOf(preCreateSubForCompaction, replicationFuture)
-                                .thenCompose(v -> {
-                                    // Also check dedup status
-                                    return 
persistentTopic.checkDeduplicationStatus();
-                                }).thenRun(() -> {
-                                    log.info("Created topic {} - dedup is {}", 
topic,
+                                        .thenCompose(__ -> 
persistentTopic.preCreateSubscriptionForCompactionIfNeeded())
+                                        .thenCompose(__ -> 
persistentTopic.checkReplication())
+                                        .thenCompose(v -> {
+                                            // Also check dedup status
+                                            return 
persistentTopic.checkDeduplicationStatus();

Review comment:
       For convenient review, a little explanation here:
   Move `preCreateSubscriptionForCompactionIfNeeded` after `initialize`.  
because in `preCreateSubscriptionForCompactionIfNeeded` it will retrieve the 
compaction threshold, it could only get the value at the broker level. Only 
after `initialize`,  namespace and topic level values will load into the topic. 
 So it's the reason to do this.  Besides, I think  
`preCreateSubscriptionForCompactionIfNeeded` should stay after `initialize` is 
more accurate . 




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


Reply via email to