omalley commented on code in PR #4127:
URL: https://github.com/apache/hadoop/pull/4127#discussion_r969956527
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcClient.java:
##########
@@ -478,8 +498,13 @@ public Object invokeMethod(
rpcMonitor.proxyOp();
}
boolean failover = false;
+ boolean tryActive = false;
Review Comment:
Do we need tryActive? The observers will be sorted ahead of the active ones,
so we should only use them if we have gone through all of the observers.
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MembershipNamenodeResolver.java:
##########
@@ -371,24 +405,37 @@ private List<MembershipState>
getRecentRegistrationForQuery(
membershipStore.getNamenodeRegistrations(request);
List<MembershipState> memberships = response.getNamenodeMemberships();
- if (!addExpired || !addUnavailable) {
- Iterator<MembershipState> iterator = memberships.iterator();
- while (iterator.hasNext()) {
- MembershipState membership = iterator.next();
- if (membership.getState() == EXPIRED && !addExpired) {
- iterator.remove();
- } else if (membership.getState() == UNAVAILABLE && !addUnavailable) {
- iterator.remove();
- }
+ List<MembershipState> observerMemberships = new ArrayList<>();
+ Iterator<MembershipState> iterator = memberships.iterator();
+ while (iterator.hasNext()) {
+ MembershipState membership = iterator.next();
+ if (membership.getState() == EXPIRED && !addExpired) {
+ iterator.remove();
+ } else if (membership.getState() == UNAVAILABLE && !addUnavailable) {
+ iterator.remove();
+ } else if (membership.getState() == OBSERVER && observerRead) {
+ iterator.remove();
+ observerMemberships.add(membership);
}
}
- List<MembershipState> priorityList = new ArrayList<>();
- priorityList.addAll(memberships);
- Collections.sort(priorityList, new NamenodePriorityComparator());
+ if(!observerRead) {
+ Collections.sort(memberships, new NamenodePriorityComparator());
Review Comment:
This sort should be pulled out of the branch and you just need the else
branch.
--
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]
