anandheritage commented on code in PR #15895:
URL: https://github.com/apache/pinot/pull/15895#discussion_r2105660969
##########
pinot-server/src/main/java/org/apache/pinot/server/starter/helix/SendStatsPredicate.java:
##########
@@ -67,87 +74,155 @@ public static SendStatsPredicate create(PinotConfiguration
configuration) {
throw new IllegalArgumentException("Invalid value " + modeStr + " for "
+ CommonConstants.MultiStageQueryRunner.KEY_OF_SEND_STATS_MODE, e);
}
- return mode.create();
+ return mode.create(helixManager);
}
public enum Mode {
SAFE {
@Override
- public SendStatsPredicate create() {
- return new Safe();
+ public SendStatsPredicate create(HelixManager helixManager) {
+ return new Safe(helixManager);
}
},
ALWAYS {
@Override
- public SendStatsPredicate create() {
+ public SendStatsPredicate create(HelixManager helixManager) {
return new SendStatsPredicate() {
@Override
- public boolean getSendStats() {
+ public boolean isSendStats() {
return true;
}
+ @Override
+ public boolean needWatchForInstanceConfigChange() {
+ return false;
+ }
+
@Override
public void onInstanceConfigChange(List<InstanceConfig>
instanceConfigs, NotificationContext context) {
- // Nothing to do
+ throw new UnsupportedOperationException("Should not be invoked");
}
};
}
},
NEVER {
@Override
- public SendStatsPredicate create() {
+ public SendStatsPredicate create(HelixManager helixManager) {
return new SendStatsPredicate() {
@Override
- public boolean getSendStats() {
+ public boolean isSendStats() {
+ return false;
+ }
+
+ @Override
+ public boolean needWatchForInstanceConfigChange() {
return false;
}
@Override
public void onInstanceConfigChange(List<InstanceConfig>
instanceConfigs, NotificationContext context) {
- // Nothing to do
+ throw new UnsupportedOperationException("Should not be invoked");
}
};
}
};
- public abstract SendStatsPredicate create();
+ public abstract SendStatsPredicate create(HelixManager helixManager);
}
+ @BatchMode(enabled = false)
+ @PreFetch(enabled = false)
private static class Safe extends SendStatsPredicate {
- private final AtomicBoolean _sendStats = new AtomicBoolean(true);
+ private final HelixManager _helixManager;
+ private final String _clusterName;
+ private final Map<String, String> _problematicVersionsById = new
HashMap<>();
+
+ private HelixAdmin _helixAdmin;
+ private volatile boolean _sendStats = true;
+
+ public Safe(HelixManager helixManager) {
+ _helixManager = helixManager;
+ _clusterName = helixManager.getClusterName();
+ }
+
+ @Override
+ public boolean isSendStats() {
+ return _sendStats;
+ }
@Override
- public boolean getSendStats() {
- return _sendStats.get();
+ public boolean needWatchForInstanceConfigChange() {
+ return true;
}
@Override
- public void onInstanceConfigChange(List<InstanceConfig> instanceConfigs,
NotificationContext context) {
- Map<String, String> problematicVersionsById = new HashMap<>();
- for (InstanceConfig instanceConfig : instanceConfigs) {
- switch
(InstanceTypeUtils.getInstanceType(instanceConfig.getInstanceName())) {
- case BROKER:
- case SERVER:
- String otherVersion = instanceConfig.getRecord()
-
.getStringField(CommonConstants.Helix.Instance.PINOT_VERSION_KEY, null);
- if (isProblematicVersion(otherVersion)) {
- problematicVersionsById.put(instanceConfig.getInstanceName(),
otherVersion);
+ public synchronized void onInstanceConfigChange(List<InstanceConfig>
instanceConfigs, NotificationContext context) {
+ if (_helixAdmin == null) {
+ _helixAdmin = _helixManager.getClusterManagmentTool();
Review Comment:
getClusterManagmentTool - spelling is incorrect
It should be getClusterManagementTool
--
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-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
