smengcl commented on code in PR #11370:
URL: https://github.com/apache/ozone/pull/11370#discussion_r4301830191
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ScmClient.java:
##########
@@ -146,8 +146,11 @@ static Pipeline newPipelineWithDNCache(Pipeline pipeline,
for (DatanodeDetails node : pipeline.getNodes()) {
DatanodeDetails datanodeDetails =
datanodeDetailsCache.getIfPresent(node.getID());
- // Call compareNodeValues to handle IP / hostname changes
- if (datanodeDetails != null && node.compareNodeValues(datanodeDetails)) {
+ // Use the cached entry only when it matches on both network identity
+ // (compareNodeValues: hostname / IP / ports) and wire-protocol
capability
+ // version (compareNodeVersions: currentVersion).
+ if (datanodeDetails != null && node.compareNodeValues(datanodeDetails)
+ && node.compareNodeVersions(datanodeDetails)) {
Review Comment:
`currentVersion` is now an `HDDSVersion` enum, and any version this OM does
not know deserializes to `UNKNOWN_VERSION`. With `==` on the enum, two
different newer versions look equal and OM keeps forwarding the old raw value.
`getSerializedCurrentVersion()` keeps the wire value, so this inline check is
enough and `DatanodeDetails#compareNodeVersions` can be dropped.
```suggestion
// Use the cached entry only when it matches on both network identity
// (compareNodeValues: hostname / IP / ports) and the serialized
currentVersion.
if (datanodeDetails != null && node.compareNodeValues(datanodeDetails)
&& node.getSerializedCurrentVersion() ==
datanodeDetails.getSerializedCurrentVersion()) {
```
--
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]
