jiajunwang commented on a change in pull request #519: Refine the rebalance
scope calculating logic in the WAGED rebalancer.
URL: https://github.com/apache/helix/pull/519#discussion_r338393426
##########
File path:
helix-core/src/main/java/org/apache/helix/controller/changedetector/ResourceChangeDetector.java
##########
@@ -50,8 +51,13 @@
private Map<HelixConstants.ChangeType, Collection<String>> _addedItems = new
HashMap<>();
private Map<HelixConstants.ChangeType, Collection<String>> _removedItems =
new HashMap<>();
- public ResourceChangeDetector() {
+ public ResourceChangeDetector(boolean ignoreHelixSourceChange) {
_newSnapshot = new ResourceChangeSnapshot();
+ _ignoreHelixSourceChange = ignoreHelixSourceChange;
+ }
Review comment:
I tried. You know I always try if there is a plan B before sending PR.
So here let's call what I did plan A. Then plan B would be creating a class
called something like ControllerResourceChangeDetector, as we discussed.
However, when there are 2 classes exist in the project, the users might be
confused, they will need to read the description of these classes then decide
which one to use. Since the option in plan A explains itself good enough, I
feel this way is much easier for those who may use this class in the future.
Moreover, it is not straightforward to describe what is a
"ControllerResourceChangeDetector" and explain why we need an additional class.
I'm not a fan of increasing documentation while we can let the code explain
for itself.
Finally, logically, I feel Helix controller should not have a special
requirement to the change detector. Or our design might be not good enough in
some way.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
