sodonnel commented on a change in pull request #521: HDDS-2592 Add Datanode
command to allow the datanode to persist its admin state
URL: https://github.com/apache/hadoop-ozone/pull/521#discussion_r374945844
##########
File path:
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/helpers/DatanodeIdYaml.java
##########
@@ -82,6 +83,12 @@ public static DatanodeDetails readDatanodeIdFile(File path)
.setIpAddress(datanodeDetailsYaml.getIpAddress())
.setHostName(datanodeDetailsYaml.getHostName())
.setCertSerialId(datanodeDetailsYaml.getCertSerialId());
+ if (datanodeDetailsYaml.getPersistedOpState() != null) {
+ builder.setPersistedOpState(HddsProtos.NodeOperationalState.valueOf(
+ datanodeDetailsYaml.getPersistedOpState()));
+ }
+ builder.setPersistedOpStateExpiry(
Review comment:
If I recall (I wrote this code a month or so ago and forgot to create the
PR) the reason the OpState is wrapped in a IF block is because I need take the
string read from the YAML file and turn it into the enum. The enum didn't like
me calling valueOf(null), so I only make that call if its non-null. All the
other parameters in the builder are strings except for the two new ones (enum
and long), so they never really had to handle the null case before.
You are correct, in that if there is no data, the
"getPersistedOpStateExpiryEpochSec" will end up as zero. That is OK, as SCM
treats zero as "no expiry" right now.
I've got similar logic at the end of that class too to handle the turning
the enum into a string:
```
String persistedOpString = null;
if (datanodeDetails.getPersistedOpState() != null) {
persistedOpString = datanodeDetails.getPersistedOpState().name();
}
return new DatanodeDetailsYaml(
datanodeDetails.getUuid().toString(),
datanodeDetails.getIpAddress(),
datanodeDetails.getHostName(),
datanodeDetails.getCertSerialId(),
persistedOpString,
datanodeDetails.getPersistedOpStateExpiryEpochSec(),
portDetails);
}
```
I think the shorter summary is that the enums needs special handling around
the nulls while the long and Strings behave OK without wrapping them in IF
blocks.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
