HDDS-12991 recreate pipelines after enabling ratis write streaming by yandrey321 · Pull Request #10842 · apache/ozone

yandrey321 · 2026-07-22T16:46:34Z

What changes were proposed in this pull request?

SCM refreshes a datanode's ports on re-registration (SCMNodeManager)

When a datanode re-registers with a changed port set — e.g. it restarted with DataStream enabled and now advertises the RATIS_DATASTREAM port — SCM updates its stored NodeManager record instead of keeping the stale one. Previously a port-only change (same IP/hostname/version) was ignored, so SCM never learned the new port and every new pipeline stayed port-less.
New PipelineManager.closeNonStreamablePipelines(): closes+deletes OPEN pipelines whose persisted node snapshot lacks a port the currently-registered nodes now expose BackgroundPipelineCreator then builds fresh, streaming-capable replacements. Runs automatically as part of the pipeline scrubber: scrubAndCloseNonStreamablePipelines() (scrubs, then closes non-streamable) is wired into the BackgroundPipelineScrubber.

Why close-and-recreate rather than heal in place: a pipeline created before DataStream persists a stale dataStreamAddress (the gRPC port) in its Raft group configuration. ServerState.setRaftConf() only re-adds peers to the gRPC RPC, not the datastream RPC, so the group can never stream in place — the group must be recreated.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-12991

How was this patch tested?

TestSCMNodeManager: testRegisterRefreshesPortsOnPortChange (ports changed → node record updated) and testRegisterKeepsPortsWhenUnchanged (unchanged → no update).
TestPipelineManagerImpl: coverage for closeNonStreamablePipelines() — closes a portless OPEN pipeline once nodes expose the new port, and is a no-op when port sets already match.
Integration — TestOzoneFileSystemDataStreamEnablement (MiniOzoneCluster, datanodes start with DataStream disabled, client with streaming enabled):

testDataStreamFallbackAndPortRefresh: writes fall back gracefully while a pipeline is port-less; SCM refreshes the datanodes' ports on re-registration (validates piece 1).
testCloseNonStreamablePipelineThenStream: a portless legacy pipeline survives a rolling restart + SCM restart; closeNonStreamablePipelines() retires it; a fresh streaming-capable pipeline forms and a streaming write succeeds end-to-end (validates piece 2).
testBatchWritesAcrossStreamingEnablement: full lifecycle over a batch of files — several files written while disabled (all succeed via fallback), enable + close legacy pipeline, then several more files that all stream over a RATIS_DATASTREAM-capable pipeline; asserts no write fails and the post-enablement writes take the streaming path.

when part 1 (#10828) is missing:

mvn -pl hadoop-ozone/integration-test test \
  -Dtest='TestOzoneFileSystemDataStreamEnablement#testBatchWritesAcrossStreamingEnablement' \
  -DskipShade -DskipRecon \
  -Ddevelocity.cache.local.enabled=false -Ddevelocity.cache.remote.enabled=false -Dscan=false

Tests run: 1, Failures: 0, Errors: 1  <<< FAILURE!

java.io.IOException: RATIS_DATASTREAM port is missing for datanode <dn>
  in pipeline <id>; datastream is disabled for this pipeline
    at org.apache.hadoop.ozone.client.io.KeyDataStreamOutput.handleWrite(KeyDataStreamOutput.java:212)
    ...
Caused by: java.io.IOException: RATIS_DATASTREAM port is missing ...
    at org.apache.hadoop.hdds.scm.storage.BlockDataStreamOutput.setupStream(BlockDataStreamOutput.java:200)
    at org.apache.hadoop.hdds.scm.storage.BlockDataStreamOutput.<init>(BlockDataStreamOutput.java:173)

yandrey321 · 2026-07-23T20:14:55Z

@szetszwo

chihsuan

Thanks for working on this! I left a few comments around the pipeline retirement condition but the overall approach looks reasonable.

Also, the three integration tests each start their own MiniOzoneCluster, which probably would add noticeable CI cost. Would you consider combining these into one end-to-end lifecycle test?

chihsuan · 2026-07-24T10:35:28Z

+        .map(DatanodeDetails.Port::getName).collect(Collectors.toSet());
+    return current.getPorts().stream()
+        .map(DatanodeDetails.Port::getName)
+        .anyMatch(name -> !storedNames.contains(name));


SCMNodeManager.portsChanged handles name=value changes, while this check only compares names. Should we also compare the RATIS_DATASTREAM port value here?

chihsuan · 2026-07-24T10:40:47Z

+    return false;
+  }
+
+  private static boolean exposesNewPorts(DatanodeDetails stored,


I wonder if we could narrow this check to RATIS pipelines and the RATIS_DATASTREAM port specifically. Older pipeline snapshots may legitimately lack ports added by later versions, such as HTTP, HTTPS, or CLIENT_RPC. In that case, checking any new port could retire unrelated pipelines, including EC pipelines.

chihsuan · 2026-07-24T10:41:32Z

+  private boolean nodesExposeNewPorts(Pipeline pipeline) {
+    for (DatanodeDetails stored : pipeline.getNodes()) {
+      final DatanodeDetails current = nodeManager.getNode(stored.getID());
+      if (current != null && exposesNewPorts(stored, current)) {


One concern during a rolling restart: this may close the pipeline as soon as the first member advertises the new port. A replacement could still include datanodes that have not restarted yet, leading to another close-and-recreate cycle later. Would it make sense if we defer retirement until SCM can ensure the replacement uses only DataStream-capable nodes?

chihsuan · 2026-07-24T10:43:09Z

+   * succeeds over the new pipeline (HDDS-12991).
+   */
+  @Test
+  @Timeout(value = 70, unit = TimeUnit.SECONDS)


Would it make sense to rely on the repository’s five-minute default timeout here? This test waits up to 30 seconds for port registration both before and after the SCM restart, then up to 60 seconds for a replacement pipeline.

With cluster startup and restarts as well, the 70-second outer timeout may fire first and hide which operation was actually slow.

The same concern also applies to lines 225 and 318.

szetszwo

@yandrey321 , thanks for working on this!

Please review the AI generated code carefully before submitting.

The code is inconsistent -- streaming specific vs any new ports
The javadocs/comments are very long and keep repeating.

It is wasting the time of the reviewers of this PR.

szetszwo · 2026-07-24T17:52:36Z

+  private static boolean exposesNewPorts(DatanodeDetails stored,
+      DatanodeDetails current) {
+    final Set<DatanodeDetails.Port.Name> storedNames = stored.getPorts().stream()
+        .map(DatanodeDetails.Port::getName).collect(Collectors.toSet());
+    return current.getPorts().stream()
+        .map(DatanodeDetails.Port::getName)
+        .anyMatch(name -> !storedNames.contains(name));


Move it to DatanodeDetails and make it non-static.

szetszwo · 2026-07-24T17:54:11Z

+   * created in their place. A pre-datastream Raft group cannot be made
+   * streamable in place, so it must be recreated (HDDS-12991).
+   */
+  void closeNonStreamablePipelines();


The implementation is closing all the pipelines with new port but specific to Streaming. Please update the name and javadoc.

szetszwo · 2026-07-24T17:56:32Z

+  }
+
+  @Override
+  public void closeNonStreamablePipelines() {


The implementation is closing all the pipelines with new port but specific to Streaming. Please change the name to reflect it.

szetszwo · 2026-07-24T17:57:24Z

+        LOG.info("Closing non-streamable pipeline {} so a streaming-capable "
+            + "pipeline can replace it", id);


Again, the implementation is closing all the pipelines with new port. The log message is very misleading.

szetszwo · 2026-07-24T18:00:29Z

+  /**
+   * Close (and delete) OPEN pipelines that predate a datanode capability the
+   * registered node now advertises but the pipeline's stored node snapshot
+   * lacks — in practice the RATIS_DATASTREAM port after Ratis DataStream was
+   * enabled. Such pipelines cannot serve streaming even after the datanodes
+   * restart, because the Raft group's persisted configuration still carries the
+   * stale datastream address; only a freshly created pipeline is
+   * streaming-capable. BackgroundPipelineCreator recreates replacements from
+   * the now-capable nodes (HDDS-12991).
+   */


This kind of long javadoc repeated multiple times talking about the same thing through out this PR. It will be very maintain it for the future changes. Please review and revise the AI generated javadocs/comments before submitting a PR.

HDDS-12991 recreate pipelines after enabling ratis write streaming

917170c

yandrey321 mentioned this pull request Jul 22, 2026

HDDS-12991 failback to non-streaming write when pipeline does not have streaming port info #10828

Open

chihsuan reviewed Jul 24, 2026

View reviewed changes

szetszwo reviewed Jul 24, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

HDDS-12991 recreate pipelines after enabling ratis write streaming#10842

HDDS-12991 recreate pipelines after enabling ratis write streaming#10842
yandrey321 wants to merge 1 commit into
apache:masterfrom
yandrey321:HDDS-12991-part-2

yandrey321 commented Jul 22, 2026

Uh oh!

yandrey321 commented Jul 23, 2026

Uh oh!

chihsuan left a comment

Uh oh!

chihsuan Jul 24, 2026

Uh oh!

chihsuan Jul 24, 2026

Uh oh!

chihsuan Jul 24, 2026

Uh oh!

chihsuan Jul 24, 2026

Uh oh!

szetszwo left a comment

Uh oh!

szetszwo Jul 24, 2026

Uh oh!

szetszwo Jul 24, 2026

Uh oh!

szetszwo Jul 24, 2026

Uh oh!

szetszwo Jul 24, 2026

Uh oh!

szetszwo Jul 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		LOG.info("Closing non-streamable pipeline {} so a streaming-capable "
		+ "pipeline can replace it", id);

Uh oh!

Conversation

yandrey321 commented Jul 22, 2026

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

yandrey321 commented Jul 23, 2026

Uh oh!

chihsuan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

szetszwo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants