KeeProMise commented on code in PR #6096:
URL: https://github.com/apache/hadoop/pull/6096#discussion_r1330001570
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/store/impl/MembershipStoreImpl.java:
##########
@@ -279,30 +279,30 @@ private MembershipState getRepresentativeQuorum(
Collection<MembershipState> records) {
// Collate objects by field value: field value -> order set of records
- Map<FederationNamenodeServiceState, TreeSet<MembershipState>> occurenceMap
=
+ Map<FederationNamenodeServiceState, Set<MembershipState>> occurenceMap =
new HashMap<>();
for (MembershipState record : records) {
FederationNamenodeServiceState state = record.getState();
- TreeSet<MembershipState> matchingSet = occurenceMap.get(state);
+ Set<MembershipState> matchingSet = occurenceMap.get(state);
if (matchingSet == null) {
- // TreeSet orders elements by descending date via comparators
- matchingSet = new TreeSet<>();
+ matchingSet = new HashSet<>();
occurenceMap.put(state, matchingSet);
}
matchingSet.add(record);
}
// Select largest group
- TreeSet<MembershipState> largestSet = new TreeSet<>();
- for (TreeSet<MembershipState> matchingSet : occurenceMap.values()) {
+ Set<MembershipState> largestSet = new HashSet<>();
Review Comment:
I reverted the changes to this code.
I modified the comparison logic of MembershipState. I think the essential
cause of the problem is not Treeset but the comparison rules of MembershipState.
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/TestStateStoreMembershipState.java:
##########
@@ -223,6 +223,57 @@ public void testRegistrationMajorityQuorum()
assertEquals(quorumEntry.getRouterId(), ROUTERS[3]);
}
+ /**
+ * Fix getRepresentativeQuorum when records have same date modified time.
+ */
+ @Test
+ public void testRegistrationMajorityQuorumEqDateModified()
+ throws InterruptedException, IOException {
+
+ // Populate the state store with a set of non-matching elements
+ // 1) ns0:nn0 - Standby (newest)
+ // 2) ns0:nn0 - Active
+ // 3) ns0:nn0 - Active
+ // 4) ns0:nn0 - Active
+ // (2), (3), (4) have the same date modified time
+ // Verify the selected entry is the newest majority opinion (4)
+ String ns = "ns0";
+ String nn = "nn0";
+
+ long dateModified = Time.now();
+ // Active - oldest
+ MembershipState report = createRegistration(
+ ns, nn, ROUTERS[1], FederationNamenodeServiceState.ACTIVE);
Review Comment:
done
--
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]
