szetszwo commented on code in PR #8420:
URL: https://github.com/apache/ozone/pull/8420#discussion_r2083241007
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/protocol/commands/CommandForDatanode.java:
##########
@@ -28,22 +29,26 @@
public class CommandForDatanode<T extends Message> implements
IdentifiableEventPayload {
- private final UUID datanodeId;
-
+ private final DatanodeID datanodeId;
private final SCMCommand<T> command;
public CommandForDatanode(DatanodeDetails datanode, SCMCommand<T> command) {
- this(datanode.getUuid(), command);
+ this(datanode.getID(), command);
}
// TODO: Command for datanode should take DatanodeDetails as parameter.
public CommandForDatanode(UUID datanodeId, SCMCommand<T> command) {
+ this(DatanodeID.of(datanodeId), command);
+ }
Review Comment:
This constructor can be removed since all callers have `DatanodeID` or
`DatanodeDetails`.
```diff
diff --git
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/protocol/commands/CommandForDatanode.java
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/protocol/commands/CommandForDatanode.java
index 77b418a3ba..0326df66a9 100644
---
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/protocol/commands/CommandForDatanode.java
+++
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/protocol/commands/CommandForDatanode.java
@@ -36,11 +36,6 @@ public CommandForDatanode(DatanodeDetails datanode,
SCMCommand<T> command) {
this(datanode.getID(), command);
}
- // TODO: Command for datanode should take DatanodeDetails as parameter.
- public CommandForDatanode(UUID datanodeId, SCMCommand<T> command) {
- this(DatanodeID.of(datanodeId), command);
- }
-
public CommandForDatanode(DatanodeID datanodeId, SCMCommand<T> command) {
this.datanodeId = datanodeId;
this.command = command;
diff --git
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/CloseContainerEventHandler.java
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/CloseContainerEventHandler.java
index 716762ba73..f5adc42538 100644
---
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/CloseContainerEventHandler.java
+++
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/CloseContainerEventHandler.java
@@ -157,8 +157,7 @@ private Void triggerCloseCallback(
EventPublisher publisher, ContainerInfo container, SCMCommand<?>
command)
throws ContainerNotFoundException {
getNodes(container).forEach(node ->
- publisher.fireEvent(DATANODE_COMMAND,
- new CommandForDatanode<>(node.getUuid(), command)));
+ publisher.fireEvent(DATANODE_COMMAND, new
CommandForDatanode<>(node, command)));
return null;
}
diff --git
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java
index c269cf0144..0c6667fa78 100644
---
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java
+++
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java
@@ -781,8 +781,7 @@ protected void
sendFinalizeToDatanodeIfNeeded(DatanodeDetails datanodeDetails,
// Send Finalize command to the data node. Its OK to
// send Finalize command multiple times.
scmNodeEventPublisher.fireEvent(SCMEvents.DATANODE_COMMAND,
- new CommandForDatanode<>(datanodeDetails.getUuid(),
- finalizeCmd));
+ new CommandForDatanode<>(datanodeDetails, finalizeCmd));
} catch (NotLeaderException ex) {
LOG.warn("Skip sending finalize upgrade command since current
SCM" +
" is not leader.", ex);
diff --git
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineActionHandler.java
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineActionHandler.java
index 94a17d5989..c5ee6b9090 100644
---
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineActionHandler.java
+++
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineActionHandler.java
@@ -118,8 +118,7 @@ private void closeUnknownPipeline(final EventPublisher
publisher,
"firing close pipeline event.", pid);
SCMCommand<?> command = new ClosePipelineCommand(pid);
command.setTerm(scmContext.getTermOfLeader());
- publisher.fireEvent(SCMEvents.DATANODE_COMMAND,
- new CommandForDatanode<>(datanode.getUuid(), command));
+ publisher.fireEvent(SCMEvents.DATANODE_COMMAND, new
CommandForDatanode<>(datanode, command));
} catch (NotLeaderException nle) {
LOG.info("Cannot process Pipeline Action for pipeline {} as " +
"current SCM is not leader anymore.", pid);
diff --git
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineReportHandler.java
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineReportHandler.java
index dc3b016823..edc60e468f 100644
---
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineReportHandler.java
+++
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineReportHandler.java
@@ -100,8 +100,7 @@ private void handlePipelineNotFoundException(final
PipelineReport report,
try {
final SCMCommand<?> command = new ClosePipelineCommand(pipelineID);
command.setTerm(scmContext.getTermOfLeader());
- publisher.fireEvent(SCMEvents.DATANODE_COMMAND,
- new CommandForDatanode<>(dn.getUuid(), command));
+ publisher.fireEvent(SCMEvents.DATANODE_COMMAND, new
CommandForDatanode<>(dn, command));
} catch (NotLeaderException ex) {
// Do nothing if the leader has changed.
}
diff --git
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/RatisPipelineProvider.java
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/RatisPipelineProvider.java
index a7c09238b4..e01a53ccda 100644
---
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/RatisPipelineProvider.java
+++
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/RatisPipelineProvider.java
@@ -208,7 +208,7 @@ public synchronized Pipeline
create(RatisReplicationConfig replicationConfig,
LOG.info("Sending CreatePipelineCommand for pipeline:{} to
datanode:{}",
pipeline.getId(), node);
eventPublisher.fireEvent(SCMEvents.DATANODE_COMMAND,
- new CommandForDatanode<>(node.getUuid(), createCommand));
+ new CommandForDatanode<>(node, createCommand));
});
return pipeline;
@@ -268,8 +268,7 @@ public void close(Pipeline pipeline) throws
NotLeaderException {
new ClosePipelineCommand(pipeline.getId());
closeCommand.setTerm(scmContext.getTermOfLeader());
pipeline.getNodes().forEach(node -> {
- final CommandForDatanode<?> datanodeCommand =
- new CommandForDatanode<>(node.getUuid(), closeCommand);
+ final CommandForDatanode<?> datanodeCommand = new
CommandForDatanode<>(node, closeCommand);
LOG.info("Send pipeline:{} close command to datanode {}",
pipeline.getId(), datanodeCommand.getDatanodeId());
eventPublisher.fireEvent(SCMEvents.DATANODE_COMMAND, datanodeCommand);
```
--
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: issues-unsubscr...@ozone.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@ozone.apache.org
For additional commands, e-mail: issues-h...@ozone.apache.org
