lhotari commented on code in PR #24156:
URL: https://github.com/apache/pulsar/pull/24156#discussion_r2032738545


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/channel/ServiceUnitStateChannelImpl.java:
##########
@@ -1505,7 +1502,21 @@ private void doHealthCheckBrokerAsyncWithRetries(String 
brokerId, int retry, Com
         }
     }
 
-    private synchronized void doCleanup(String broker, boolean gracefully) {
+    private void tryWaitForOverrides(List<CompletableFuture<Void>> 
overrideFutures, boolean force) {

Review Comment:
   It seems that this solution is fine for this use case, but it's worth 
knowing that there's a generic solution for limiting concurrency for async 
operations which is used in Pulsar Admin client (introduced by PR 
https://github.com/apache/pulsar/pull/22541). It's the 
[com.spotify.futures.ConcurrencyReducer 
class](https://github.com/spotify/completable-futures/blob/master/src/main/java/com/spotify/futures/ConcurrencyReducer.java)
 from Spotify completable-futures library.



##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/channel/ServiceUnitStateChannelImpl.java:
##########
@@ -106,13 +106,15 @@ public class ServiceUnitStateChannelImpl implements 
ServiceUnitStateChannel {
 
     private static final int OWNERSHIP_CLEAN_UP_MAX_WAIT_TIME_IN_MILLIS = 5000;
     private static final int OWNERSHIP_CLEAN_UP_WAIT_RETRY_DELAY_IN_MILLIS = 
100;
+    private static final int MAX_CONCURRENT_OWNERSHIP_OVERRIDE = 500;

Review Comment:
   500 seems like very high concurrency. Would it make sense to make this 
setting configurable?



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