abstractdog commented on code in PR #5742:
URL: https://github.com/apache/hive/pull/5742#discussion_r2148693137
##########
llap-client/src/java/org/apache/hadoop/hive/llap/registry/impl/LlapFixedRegistryImpl.java:
##########
@@ -257,7 +257,7 @@ public List<LlapServiceInstance>
getAllInstancesOrdered(boolean consistentIndexe
Collections.sort(list, new Comparator<LlapServiceInstance>() {
@Override
public int compare(LlapServiceInstance o1, LlapServiceInstance o2) {
- return o2.getWorkerIdentity().compareTo(o2.getWorkerIdentity());
+ return o1.getWorkerIdentity().compareTo(o2.getWorkerIdentity());
Review Comment:
Considering that the comparison was incorrect and FixedServiceInstanceSet
failed to comply with any sorting requirements (unlike
DynamicServiceInstanceSet, which uses a TreeMap to maintain order), it doesn't
really matter whether we fix the sorting or remove it entirely.
However, since the interface implies that some ordering is expected, it's
reasonable to fix it now using a simple ascending order, which aligns better
with intuitive expectations and brings consistency to the implementation.
--
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: gitbox-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
