heesung-sn commented on code in PR #21332:
URL: https://github.com/apache/pulsar/pull/21332#discussion_r1357628439


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/channel/ServiceUnitStateChannelImpl.java:
##########
@@ -503,13 +503,22 @@ public CompletableFuture<Optional<String>> 
getOwnerAsync(String serviceUnit) {
             case Splitting -> {
                 return 
CompletableFuture.completedFuture(Optional.of(data.sourceBroker()));
             }
-            case Assigning, Releasing -> {
+            case Assigning -> {
                 return deferGetOwnerRequest(serviceUnit).whenComplete((__, e) 
-> {
                     if (e != null) {
                         
ownerLookUpCounters.get(state).getFailure().incrementAndGet();
                     }
-                }).thenApply(
-                        broker -> broker == null ? Optional.empty() : 
Optional.of(broker));
+                }).thenApply(broker -> broker == null ? Optional.empty() : 
Optional.of(broker));
+            }
+            case Releasing -> {
+                if (isTransferCommand(data)) {
+                    return deferGetOwnerRequest(serviceUnit).whenComplete((__, 
e) -> {
+                        if (e != null) {
+                            
ownerLookUpCounters.get(state).getFailure().incrementAndGet();
+                        }
+                    }).thenApply(broker -> broker == null ? Optional.empty() : 
Optional.of(broker));
+                }
+                return CompletableFuture.completedFuture(Optional.empty());

Review Comment:
   > Or, can we skip the ownership check when the ML update fails(if extensible 
lb)? I think it can just get the ledger info again no matter what. When the 
monitor fixes the stuck state, it should clean the ledger again.
   
   >> Yes, it can work for tests, but still, I'm not sure if that is this 
unload result is correct(Releasing -> Own).
   
   As long as the source unblocks and returns error, I think we are good here.
   
   I believe we need a separate PR to fix the case when the ledger info update 
fails with BadVersion. This shouldn't happen as the only source should own the 
topic.



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