jiajunwang commented on PR #2478: URL: https://github.com/apache/helix/pull/2478#issuecomment-1536624692
I had to download the code to remind myself what happened exactly... Since we have added an index to cover searching functionality, I agree that it seems the Set here can be changed to List. I guess it was carried over from the performance enhancement attempts I tried as I mentioned earlier in this thread. Given that saying, I think we should consider removing the equals() override instead of adding new hashcode override. At a high level, AssignableReplica objects are different event their resource/partition/state keys are the same. So I'm actually more concerned of adding hashCode which makes 2 different replica object identical. I tried to search for all AssignableReplica usages to confirm that if equals is still needed. Seems OK to remove, but please double check : ) Also, please run some benchmark to ensure that we don't miss anything. And this is a good catch. Thanks for the change! -- 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]
