neils-dev commented on code in PR #3409:
URL: https://github.com/apache/ozone/pull/3409#discussion_r1014922846
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerHAWithFailover.java:
##########
@@ -62,4 +70,48 @@ public void testIncrementalWaitTimeWithSameNodeFailover()
throws Exception {
Assert.assertEquals((numTimesTriedToSameNode + 1) * waitBetweenRetries,
omFailoverProxyProvider.getWaitTime());
}
+
Review Comment:
The javadoc for the test class, `TestOzoneManagerHAWithFailover` comments
that tests should not be added to this class as it has a cleanup problem that
leaves the cluster in a unusable states (not reusable). Are we affected by
this?
> * NOTE: Do not add new tests to this class since
https://github.com/apache/ozone/blob/40203afe63d40fa1340b64fd67ddfd5a2a6b0c99/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerHAWithFailover.java#L36
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/exceptions/OMNotLeaderException.java:
##########
@@ -34,24 +34,33 @@ public class OMNotLeaderException extends IOException {
private final String currentPeerId;
private final String leaderPeerId;
+ private final String leaderAddress;
private static final Pattern CURRENT_PEER_ID_PATTERN =
Pattern.compile("OM:(.*) is not the leader[.]+.*", Pattern.DOTALL);
private static final Pattern SUGGESTED_LEADER_PATTERN =
- Pattern.compile(".*Suggested leader is OM:([^.]*).*", Pattern.DOTALL);
+ Pattern.compile(".*Suggested leader is OM:([^.]*)\\/(.*)\\.",
+ Pattern.DOTALL);
public OMNotLeaderException(RaftPeerId currentPeerId) {
super("OM:" + currentPeerId + " is not the leader. Could not " +
"determine the leader node.");
this.currentPeerId = currentPeerId.toString();
this.leaderPeerId = null;
+ this.leaderAddress = null;
}
public OMNotLeaderException(RaftPeerId currentPeerId,
RaftPeerId suggestedLeaderPeerId) {
+ this(currentPeerId, suggestedLeaderPeerId, null);
+ }
+
+ public OMNotLeaderException(RaftPeerId currentPeerId,
+ RaftPeerId suggestedLeaderPeerId, String suggestedLeaderAddress) {
super("OM:" + currentPeerId + " is not the leader. Suggested leader is" +
- " OM:" + suggestedLeaderPeerId + ".");
+ " OM:" + suggestedLeaderPeerId + "/" + suggestedLeaderAddress + ".");
Review Comment:
Thanks @symious for this important patch to fix suggesting the leader om
when the client submits a request to a follower.
Q. Will we also run into the client nodeid list misconfiguration problem
brought up should the `OMNotALeaderException `return a `null` for the
`suggestedLeaderAddress`?
> In https://github.com/apache/ozone/pull/2765#issuecomment-952091699, the
concern I think is the misconfig of client side might trigger some dead loops,
--
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]
