Skip to content

HDDS-10875. XceiverRatisServer#getRaftPeersInPipeline should be called before XceiverRatisServer#removeGroup #6696

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ivandika3 merged 8 commits into from
May 21, 2024
Merged

HDDS-10875. XceiverRatisServer#getRaftPeersInPipeline should be called before XceiverRatisServer#removeGroup #6696

Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9331834
HDDS-10875. GroupMismatchException in ClosePipelineCommandHandler sho…
ivandika3 May 18, 2024
9366da7
Add pipeline ID to the log
ivandika3 May 18, 2024
661fb10
Catch GroupMismatchException in the existing try catch
ivandika3 May 19, 2024
39844e3
Improve log message
ivandika3 May 19, 2024
0de1765
Detected nested GroupMismatchException
ivandika3 May 19, 2024
c885d9f
Checkstyle fix
ivandika3 May 19, 2024
a40d62c
NoRetry policy for raft client in ClosePipelineCommandHandler
ivandika3 May 19, 2024
4c0f441
Remove unnecessary catch block
ivandika3 May 21, 2024
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
24 changes: 19 additions & 5 deletions ...adoop/ozone/container/common/statemachine/commandhandler/ClosePipelineCommandHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,14 +105,16 @@ public void handle(SCMCommand command, OzoneContainer ozoneContainer,
try {
XceiverServerSpi server = ozoneContainer.getWriteChannel();
if (server.isExist(pipelineIdProto)) {
server.removeGroup(pipelineIdProto);
if (server instanceof XceiverServerRatis) {
// TODO: Refactor Ratis logic to XceiverServerRatis
// Propagate the group remove to the other Raft peers in the pipeline
XceiverServerRatis ratisServer = (XceiverServerRatis) server;
final RaftGroupId raftGroupId = RaftGroupId.valueOf(pipelineID.getId());
final Collection<RaftPeer> peers = ratisServer.getRaftPeersInPipeline(pipelineID);
final boolean shouldDeleteRatisLogDirectory = ratisServer.getShouldDeleteRatisLogDirectory();
// This might throw GroupMismatchException if the Ratis group has been closed by other datanodes
final Collection<RaftPeer> peers = ratisServer.getRaftPeersInPipeline(pipelineID);
// Try to send remove group for the other datanodes first, ignoring GroupMismatchException
// if the Ratis group has been closed in the other datanodes
peers.stream()
.filter(peer -> !peer.getId().equals(ratisServer.getServer().getId()))
.forEach(peer -> {
Expand All @@ -122,17 +124,29 @@ public void handle(SCMCommand command, OzoneContainer ozoneContainer,
} catch (GroupMismatchException ae) {
// ignore silently since this means that the group has been closed by earlier close pipeline
// command in another datanode
LOG.debug("Failed to remove group {} for pipeline {} on peer {} since the group has " +
"been removed by earlier close pipeline command handled in another datanode", raftGroupId,
pipelineID, peer.getId());
} catch (IOException ioe) {
LOG.warn("Failed to remove group {} for peer {}", raftGroupId, peer.getId(), ioe);
LOG.warn("Failed to remove group {} of pipeline {} on peer {}",
raftGroupId, pipelineID, peer.getId(), ioe);
}
});
}
// Remove the Ratis group from the current datanode pipeline, might throw GroupMismatchException as
// well. It is a no-op for XceiverServerSpi implementations (e.g. XceiverServerGrpc)
server.removeGroup(pipelineIdProto);
LOG.info("Close Pipeline {} command on datanode {}.", pipelineID,
dn.getUuidString());
} else {
LOG.debug("Ignoring close pipeline command for pipeline {} " +
"as it does not exist", pipelineID);
LOG.debug("Ignoring close pipeline command for pipeline {} on datanode {} " +
"as it does not exist", pipelineID, dn.getUuidString());
}
} catch (GroupMismatchException gme) {
// ignore silently since this means that the group has been closed by earlier close pipeline
// command in another datanode
LOG.debug("The group for pipeline {} on datanode {} has been removed by earlier close " +
"pipeline command handled in another datanode", pipelineID, dn.getUuidString());
Copy link
Contributor

@szetszwo szetszwo May 20, 2024

Choose a reason for hiding this comment

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

@ivandika3 , Since HddsClientUtils.containsException below will cover this case, let's remove catch (GroupMismatchException gme) {...}?

Copy link
Contributor Author

@ivandika3 ivandika3 May 21, 2024

Choose a reason for hiding this comment

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

Thank you for the review. Updated.

} catch (IOException e) {
LOG.error("Can't close pipeline {}", pipelineID, e);
} finally {
Expand Down