jiajunwang commented on a change in pull request #1256:
URL: https://github.com/apache/helix/pull/1256#discussion_r469070998
##########
File path:
helix-core/src/main/java/org/apache/helix/controller/changedetector/trimmer/HelixPropertyTrimmer.java
##########
@@ -57,38 +57,49 @@
* @param originalProperty
*/
protected ZNRecord doTrim(T originalProperty) {
+ ZNRecord originalZNRecord = originalProperty.getRecord();
Review comment:
Good point. So this is also my first try. FYI, I also tried to extend
the field type to include LIST_FIELD_KEY_ONLY. But then I find these 2
solutions are too complicated.
It is correct that putting this logic into the IdealStateTrimmer will
immediately fix the problem without too much change. The complex is that in
this way, it would be hard to reason the overall logic of all trimmers. Some of
them care about values, the others do not. Of course, we can add comments. But
gradually, it will become harder to understand and maintain.
So I go back and think about why we need these trimmers. Do we need to trim
soo much information? I think we don't. The rebalancer will perform well even
we detect all keys change (no value, just key's change) for all the properties.
And the logic is very clean and concentrate:
1. Keys are not ignored
2. Values are ignored accordingly and this is specified by the
getNonTrimmableFields() method.
Then I firstly tried to not trim all keys (including the simple fields). But
then I realize it is not optimal, since the simple fields' keys are tightly
bounded with the values. And finally, I have the current proposal.
Overall, the main reason for this trade-off is to make the trim logic less
diverse and easier to maintain.
----------------------------------------------------------------
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
