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]