LuciferYang commented on code in PR #12379:
URL: https://github.com/apache/gluten/pull/12379#discussion_r3479744817
##########
gluten-core/src/main/java/org/apache/gluten/hash/ConsistentHash.java:
##########
@@ -162,7 +162,10 @@ public Set<T> getNodes() {
public Set<Partition<T>> getPartition(T node) {
lock.readLock().lock();
try {
- return nodes.get(node);
+ // Return a defensive copy: the map value is internal mutable state, and
getNodes() copies
+ // for the same reason. Callers get a snapshot they can't use to mutate
the ring.
+ Set<Partition<T>> partitions = nodes.get(node);
+ return partitions == null ? null : new HashSet<>(partitions);
Review Comment:
Good point — addressed in 4d1048310. `setSlot()` is now private (only
`add()` assigns slots during construction, accessible as a nestmate), so
callers can no longer mutate ring state through the partitions returned by
`getPartition()`. I kept it to the minimal non-public change rather than
restructuring construction for full `Partition` immutability, since that would
touch `add()`/`getPartitionKey()` — which #12351 is already changing — and I'd
rather not overlap the two PRs.
--
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]
