errose28 commented on code in PR #10570:
URL: https://github.com/apache/ozone/pull/10570#discussion_r3641566911
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/protocol/commands/ReplicateContainerCommand.java:
##########
@@ -36,10 +39,23 @@ public final class ReplicateContainerCommand
private int replicaIndex = 0;
private ReplicationCommandPriority priority =
ReplicationCommandPriority.NORMAL;
+ private ComponentVersion apparentVersion = HDDSVersion.DEFAULT_VERSION;
public static ReplicateContainerCommand toTarget(long containerID,
+ DatanodeDetails target, ComponentVersion apparentVersion) {
+ ReplicateContainerCommand cmd =
+ new ReplicateContainerCommand(containerID, target);
+ cmd.apparentVersion = apparentVersion;
+ return cmd;
+ }
+
+ /**
+ * Test-only factory that fills in the latest apparent version, so callers
+ * that do not exercise mixed-version handling don't need to specify it.
+ */
+ public static ReplicateContainerCommand forTest(long containerID,
Review Comment:
This can go in `ContainerTestUtils` so we don't need test-only code in the
prod class.
##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestReplicationManager.java:
##########
@@ -1739,4 +1751,85 @@ private void mockReplicationCommandCounts(
});
}
+ /**
+ * Captures the ReplicateContainerCommand SCM sends to a datanode.
+ */
+ private ReplicateContainerCommand captureSentReplicateCommand() {
+ ArgumentCaptor<SCMCommand<?>> command =
+ ArgumentCaptor.forClass(SCMCommand.class);
+ verify(nodeManager).addDatanodeCommand(any(DatanodeID.class),
+ command.capture());
+ return (ReplicateContainerCommand) command.getValue();
+ }
+
+ private DatanodeInfo mockDatanodeWithApparentVersion(
+ DatanodeDetails dn, HDDSVersion version) {
+ DatanodeInfo info = mock(DatanodeInfo.class);
+ when(info.getLastKnownApparentVersion()).thenReturn(version);
+ when(nodeManager.getNode(dn.getID())).thenReturn(info);
+ return info;
+ }
+
+ /**
+ * Regardless of which datanode is newer, and regardless of the command
+ * sending path, the replicate command must carry the lowest apparent version
+ * among the source and target datanodes.
+ */
+ @ParameterizedTest
+ @CsvSource({
+ "true, true",
+ "true, false",
+ "false, true",
+ "false, false",
+ })
+ public void testApparentVersionIsLowestOfSourceAndTarget(
+ boolean throttled, boolean sourceNewer)
+ throws CommandTargetOverloadedException, NotLeaderException,
+ NodeNotFoundException {
+ ContainerInfo containerInfo =
+ ReplicationTestUtil.createContainerInfo(repConfig, 1,
+ HddsProtos.LifeCycleState.CLOSED, 10, 20);
+ DatanodeDetails target = MockDatanodeDetails.randomDatanodeDetails();
+ DatanodeDetails source = MockDatanodeDetails.randomDatanodeDetails();
+
+ HDDSVersion lower = HDDSVersion.SEPARATE_RATIS_PORTS_AVAILABLE;
+ HDDSVersion higher = HDDSVersion.STREAM_BLOCK_SUPPORT;
Review Comment:
These are original component versions which will never be used as apparent
versions in the new framework. We should use ZDU as the new version and some
HDDSLayoutFeature as the old version.
Claude's more verbose explanation:
> TestReplicationManager.testApparentVersionIsLowestOfSourceAndTarget uses
HDDSVersion.SEPARATE_RATIS_PORTS_AVAILABLE (serialize=1) and
STREAM_BLOCK_SUPPORT (3) as apparent versions. These can
never be real apparent versions — deserializeHDDSVersionOrLayoutVersion
maps any value < ZDU (100) to an HDDSLayoutFeature, so a value of 1 would come
back as
HDDSLayoutFeature.DATANODE_SCHEMA_V2, not the HDDSVersion used here. The
test passes only because it captures the command before proto serialization
(via addDatanodeCommand). Using realistic
values (layout features and/or ZDU+) would keep the SCM-side test aligned
with production semantics. Not a production bug — the real values in
getLastKnownApparentVersion() are always
layout features or ZDU+ — purely a test-realism point.
--
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]
