errose28 commented on code in PR #10991:
URL: https://github.com/apache/ozone/pull/10991#discussion_r3929246846
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/utils/ClientCommandsUtils.java:
##########
@@ -46,4 +51,30 @@ public static ContainerProtos.ReadChunkVersion
getReadChunkVersion(
return ContainerProtos.ReadChunkVersion.V0;
}
}
+
+ /**
+ * Returns the write pipeline (component) version the datanode should
execute the write at,
+ * derived from the value the client forwarded from the SCM-provided
pipeline version.
+ * {@link HDDSVersion#ZDU} is the lowest version write-path versioning can
use, so it is the
+ * floor: any version below ZDU (including an absent field from a client
predating zero downtime
+ * upgrade support, or a value this datanode cannot deserialize) is rounded
up to ZDU. ZDU is the
+ * first value that is unambiguous across both the component-version and the
(legacy)
+ * layout-feature domains, so any {@code isAllowed} comparison on a datanode
that has not
+ * finalized ZDU is safe. No write-path versioning feature predates ZDU, so
this loses no
+ * behavior. New clients may still forward a pre-ZDU current version (e.g.
when HDDS is not yet
+ * finalized for ZDU); the datanode rounds it up here so there is no issue.
+ */
+ public static HDDSVersion
getWritePipelineVersion(ContainerProtos.ContainerCommandRequestProto request) {
+ // Absent, unrecognized (deserializes to UNKNOWN_VERSION == -1), and
pre-ZDU versions all fall
+ // below the ZDU floor, so a single comparison rounds every one of them up
to ZDU.
+ int serializedVersion = request.hasWritePipelineVersion()
+ ? request.getWritePipelineVersion() : HDDSVersion.ZDU.serialize();
+ HDDSVersion writeVersion = HDDSVersion.deserialize(serializedVersion);
+ if (writeVersion == HDDSVersion.UNKNOWN_VERSION) {
+ // Should not normally happen: the version originates from SCM's view of
the datanodes.
+ LOG.error("Datanode was given an unrecognized write pipeline version {};
using {} instead.",
+ serializedVersion, HDDSVersion.ZDU);
+ }
+ return writeVersion.serialize() < HDDSVersion.ZDU.serialize() ?
HDDSVersion.ZDU : writeVersion;
Review Comment:
Here's a different way to write this with more documentation, no redundant
serialization on the ZDU version, and without comparing the serialized values
directly. I free handed this though so double check it.
```suggestion
final HDDSVersion defaultWriteVersion = HDDSVersion.ZDU;
// If no write version is provided, fall back to the default.
if (!request.hasWritePipelineVersion()) {
return defaultWriteVersion;
}
// If the write version does not deserialize to any known version, fall
back to the default.
int serializedVersion = request.getWritePipelineVersion();
HDDSVersion writeVersion = HDDSVersion.deserialize(serializedVersion);
if (writeVersion == HDDSVersion.UNKNOWN_VERSION) {
// Should not normally happen: the version originates from SCM's view
of the datanodes.
LOG.error("Datanode was given an unrecognized write pipeline version
{}; using {} instead.",
serializedVersion, defaultWriteVersion);
return defaultWriteVersion;
}
// ZDU is the first version that supports write path versioning.
// A lower version should be replaced with the default.
if (!defaultWriteVersion.isSupportedBy(writeVersion)) {
return defaultWriteVersion;
}
return writeVersion;
--
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]
