Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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<>();
Comment thread
goiri marked this conversation as resolved.
Outdated
for (Set<MembershipState> matchingSet : occurenceMap.values()) {
if (largestSet.size() < matchingSet.size()) {
largestSet = matchingSet;
}
}

// If quorum, use the newest element here
if (largestSet.size() > records.size() / 2) {
return largestSet.first();
TreeSet<MembershipState> sortedList = new TreeSet<>(largestSet);
return sortedList.first();
// Otherwise, return most recent by class comparator
} else if (records.size() > 0) {
TreeSet<MembershipState> sortedList = new TreeSet<>(records);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tweak the spacing to fit checkstyle.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

report.setDateModified(dateModified);
assertTrue(namenodeHeartbeat(report));

// Active - 2nd oldest
report = createRegistration(
ns, nn, ROUTERS[2], FederationNamenodeServiceState.ACTIVE);
report.setDateModified(dateModified);
assertTrue(namenodeHeartbeat(report));

// Active - 3rd oldest
report = createRegistration(
ns, nn, ROUTERS[3], FederationNamenodeServiceState.ACTIVE);
report.setDateModified(dateModified);
assertTrue(namenodeHeartbeat(report));

// standby - newest overall
report = createRegistration(
ns, nn, ROUTERS[0], FederationNamenodeServiceState.STANDBY);
assertTrue(namenodeHeartbeat(report));

// Load and calculate quorum
assertTrue(getStateStore().loadCache(MembershipStore.class, true));

// Verify quorum entry
MembershipState quorumEntry = getNamenodeRegistration(
report.getNameserviceId(), report.getNamenodeId());
assertNotNull(quorumEntry);
assertEquals(quorumEntry.getState(), FederationNamenodeServiceState.ACTIVE);
}

@Test
public void testRegistrationQuorumExcludesExpired()
throws InterruptedException, IOException {
Expand Down