9aman commented on code in PR #15241:
URL: https://github.com/apache/pinot/pull/15241#discussion_r1990708214
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/SegmentCompletionFSMFactory.java:
##########
@@ -48,8 +49,12 @@ private SegmentCompletionFSMFactory() {
public static void register(String scheme, Class<? extends
SegmentCompletionFSM> fsmClass) {
Preconditions.checkNotNull(scheme, "Scheme cannot be null");
Preconditions.checkNotNull(fsmClass, "FSM Class cannot be null");
- Preconditions.checkState(FSM_CLASS_MAP.put(scheme, fsmClass) == null,
- "FSM class already registered for scheme: " + scheme);
+
+ Class<? extends SegmentCompletionFSM> previousFsmClass =
FSM_CLASS_MAP.put(scheme, fsmClass);
+ if (previousFsmClass != null) {
+ LOGGER.warn("Replacing existing FSM: {} for scheme: {} with: {}",
Review Comment:
This has been changed to tackle the following scenario.
- User adds a new class for the "default" scheme, that has already been
registered by the static block, by using the `swagger UI` and updating the
cluster config in `ZK`
- The controller restart keeps on failing.
- The ZK cannot be updated easily as the controller won't respond. (in case
we can single controller and no other controller is available to update ZK)
This change just logs the warning, meanwhile allowing the user to update the
defaults as well.
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/SegmentCompletionConfig.java:
##########
@@ -43,16 +45,17 @@ public SegmentCompletionConfig(PinotConfiguration
configuration) {
_fsmSchemes.put(scheme, className);
}
}
-
- // Get the default FSM scheme
- _defaultFsmScheme = configuration.getProperty(DEFAULT_FSM_SCHEME_KEY,
DEFAULT_FSM_SCHEME);
Review Comment:
The `getProperty()` should return the class name for the key.
The default value returned here is the scheme name which is wrong.
cc: @KKcorps
I have added tests to ensure that these things are tested fine and we get
the right FSM
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
