errose28 commented on code in PR #11023:
URL: https://github.com/apache/ozone/pull/11023#discussion_r3873596249
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java:
##########
@@ -527,17 +531,10 @@ public void sendThrottledReplicationCommand(ContainerInfo
containerInfo,
DatanodeDetails source = selectAndOptionallyExcludeDatanode(
1, sourceWithCmds);
- try {
- ReplicateContainerCommand cmd = ReplicateContainerCommand.toTarget(
- containerID, target,
- nodeManager.getLowestApparentVersion(source, target));
- cmd.setReplicaIndex(replicaIndex);
- sendDatanodeCommand(cmd, containerInfo, source);
- } catch (NodeNotFoundException e) {
- throw new IllegalArgumentException("Datanode not found in NodeManager
while sending replication "
- + "command for container " + containerID + " from source " + source
+ " to target " + target
- + ". Should not happen", e);
- }
+ ReplicateContainerCommand cmd = ReplicateContainerCommand.toTarget(
Review Comment:
Right, it is now scoped to just the node lookup where the exception actually
occurs. `NodeNotFoundException` is checked but it looks like the previous code
wrapped it in `IllegalArgumentException` since it indicates a bug in the system
we don't have a good recovery path for.
It does mean that block no longer has context on which container was being
operated on. We could pass the container ID through the calls as well for
logging purposes if you think that would help. IMO it's not necessary since the
issue should be within the node manager independent of the container being
operated on.
--
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]
