[GitHub] [pulsar] Demogorgon314 commented on a diff in pull request #19945: [improve][broker] PIP-192 updated metrics and cleanup broker selector

["Demogorgon314 (via GitHub)" <gi...@apache.org>]

Demogorgon314 commented on code in PR #19945:
URL: https://github.com/apache/pulsar/pull/19945#discussion_r1154060387


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/channel/ServiceUnitStateChannelImpl.java:
##########
@@ -113,14 +113,15 @@ public class ServiceUnitStateChannelImpl implements ServiceUnitStateChannel {
     private static final long MIN_CLEAN_UP_DELAY_TIME_IN_SECS = 0; // 0 secs to clean immediately
     private static final long MAX_CHANNEL_OWNER_ELECTION_WAITING_TIME_IN_SECS = 10;
     private static final int MAX_OUTSTANDING_PUB_MESSAGES = 500;
+    private static final long MAX_OWNED_BUNDLE_COUNT_DELAY_TIME_IN_MILLIS = 10 * 60 * 1000;
     private final PulsarService pulsar;
     private final ServiceConfiguration config;
     private final Schema<ServiceUnitStateData> schema;
     private final ConcurrentOpenHashMap<String, CompletableFuture<String>> getOwnerRequests;
     private final String lookupServiceAddress;
     private final ConcurrentOpenHashMap<String, CompletableFuture<Void>> cleanupJobs;
     private final StateChangeListeners stateChangeListeners;
-    private BrokerSelectionStrategy brokerSelector;
+    private ExtensibleLoadManagerImpl brokerSelector;

Review Comment:
   We should change the field name.



##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/strategy/DefaultNamespaceBundleSplitStrategyImpl.java:
##########
@@ -98,9 +98,8 @@ public Set<SplitDecision> findBundlesToSplit(LoadManagerContext context, PulsarS
 
             if (!channel.isOwner(bundle)) {
                 if (debug) {
-                    log.error(String.format(CANNOT_SPLIT_BUNDLE_MSG
+                    log.warn(String.format(CANNOT_SPLIT_BUNDLE_MSG
                             + " This broker is not the owner.", bundle));
-                    counter.update(Failure, Unknown);

Review Comment:
   Do we need update counter?



##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/channel/ServiceUnitStateChannelImpl.java:
##########
@@ -329,9 +337,9 @@ protected LoadManagerContext getContext() {
     }
 
     @VisibleForTesting
-    protected BrokerSelectionStrategy getBrokerSelector() {
+    protected ExtensibleLoadManagerImpl getBrokerSelector() {

Review Comment:
   Same as the previous comment.



-- 
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: commits-unsubscribe@pulsar.apache.org

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