fhan688 commented on code in PR #18897:
URL: https://github.com/apache/hudi/pull/18897#discussion_r3346668496
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/embedded/EmbeddedTimelineService.java:
##########
@@ -280,17 +283,16 @@ public boolean equals(Object o) {
return false;
}
TimelineServiceIdentifier that = (TimelineServiceIdentifier) o;
- if (this.hostAddr != null && that.hostAddr != null) {
- return isMetadataEnabled == that.isMetadataEnabled &&
isEarlyConflictDetectionEnable == that.isEarlyConflictDetectionEnable
- && hostAddr.equals(that.hostAddr) && markerType == that.markerType;
- } else {
Review Comment:
Thanks for pointing this out. This change is not only for the new remote
partitioner flag, but also fixes a latent equality/hashCode bug in
`TimelineServiceIdentifier`.
Previously, when both `hostAddr` values were null, `equals` returned true
without comparing the other fields. That means two identifiers with different
marker type, metadata table config, early conflict detection config, or remote
partitioner config could be treated as equal, while `hashCode` still included
some of those fields. This could break the equals/hashCode contract and also
cause the embedded timeline service to be incorrectly reused.
I removed the special if/else branch and switched to comparing all
identifier fields consistently with `Objects.equals(hostAddr, that.hostAddr)`.
I also added a unit test to cover the null-host case and verify all fields
participate in equality.
--
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]
