[
https://issues.apache.org/jira/browse/HDDS-16888?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16888:
------------------------------
Attachment: MC-1-scrubber-skip-pipeline-opened-after-allocated-timeout.patch
> Pipeline scrubber closes and deletes a pipeline that was opened after the
> scrubber read it as a timed out ALLOCATED pipeline
> ----------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-16888
> URL: https://issues.apache.org/jira/browse/HDDS-16888
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Minor
> Attachments:
> MC-1-scrubber-skip-pipeline-opened-after-allocated-timeout.patch,
> TestBugMC1ScrubberClosesOpenedPipeline.java
>
>
> h3. Mechanism
> {{PipelineManagerImpl.scrubPipelines}} reads the pipeline list once per pass
> and decides on those {{Pipeline}} objects, which do not change when the
> pipeline changes state. For an entry that has been ALLOCATED for longer than
> {{ozone.scm.pipeline.allocated.timeout}}, it calls {{closePipeline}} and
> {{deletePipeline}} without reading the current state again and without a lock
> held across the check and the action. {{PipelineReportHandler}} opens a
> pipeline ({{openPipeline}}, under the pipeline manager write lock) once all
> its datanodes have reported it and a leader is known. If that happens after
> the scrubber read the list and before it reaches the pipeline, the scrubber
> closes and deletes a pipeline that is OPEN and may already have containers.
> The branch is meant for "pipelines who stay ALLOCATED for too long", as its
> comment says.
> h3. Trigger
> # A Ratis THREE pipeline stays ALLOCATED longer than the ALLOCATED timeout
> (default 5m), for example because one datanode was slow to create the Ratis
> group.
> # A scrub pass (default every 5m) reads the pipeline list. The pipeline is a
> timed out ALLOCATED candidate.
> # While the scrubber closes and deletes the candidates before it in the same
> pass, the last datanode report arrives and {{PipelineReportHandler}} opens
> the pipeline. A writer can now get a container on it;
> {{WritableRatisContainerProvider}} even waits for such an ALLOCATED pipeline
> to open when it cannot create a new one.
> # The scrubber reaches the pipeline and closes and deletes it.
> The window is the time the pass spends on the candidates before this
> pipeline, and the pipeline must become healthy only after the timeout, so
> this is a narrow race. It was not observed on a cluster.
> h3. Impact
> * A healthy, newly opened pipeline is destroyed: SCM sends
> {{ClosePipelineCommand}} to its three datanodes (run).
> * Containers already allocated on it are moved to CLOSING (run). Their
> writers fail and, as for any pipeline closed while in use (dead node, admin
> close, datanode close action), the client excludes the pipeline and continues
> on a new block (argued from code, not run). No data loss was shown.
> * The end state is the same as if the scrubber had removed the pipeline just
> before it opened, but writers that already started on it have to fail over.
> h3. Reproduction
> PASS, deterministic, unmodified source.
> [^TestBugMC1ScrubberClosesOpenedPipeline.java] ({{hadoop-hdds/server-scm}})
> uses a real {{PipelineManagerImpl}}, {{ContainerManagerImpl}} and
> {{PipelineReportHandler}} on RocksDB with the single node HA stub, a mock
> node manager, a mock clock and a mock event publisher. Two pipelines are past
> the ALLOCATED timeout when the scrub starts. A Mockito spy on
> {{closePipeline}} provides the timing: when the scrubber starts closing the
> first pipeline, the report handler receives reports from the three datanodes
> of the other one and opens it, and a writer gets a container on it through
> {{ContainerManagerImpl.getMatchingContainer}}. Output:
> {noformat}
> pipeline Pipeline-38f825a4-4d56-4bb4-bf21-eb14a3461121 after its reports:
> OPEN, container #1 handed to a writer
> after scrub: pipeline Pipeline-38f825a4-4d56-4bb4-bf21-eb14a3461121 present =
> false, container #1 = CLOSING, ClosePipelineCommand sent to 3 datanodes
> {noformat}
> A passing test means the defect is present.
> h3. Patch
> [^MC-1-scrubber-skip-pipeline-opened-after-allocated-timeout.patch], against
> b0aa6475b7856d2c7f467dd3db12b9a1dd116f63. It also applies to master at
> 6818707698c.
> In the ALLOCATED branch the scrubber first moves the pipeline to CLOSED under
> the pipeline manager write lock, only if it is still ALLOCATED (new private
> {{closeIfAllocated}}). {{openPipeline}} takes the same lock and already
> refuses a CLOSED pipeline, so either the report opens the pipeline first and
> the scrubber leaves it alone (with an INFO log), or the scrubber closes it
> first and the report no longer opens it (a report handler that read the
> pipeline as ALLOCATED just before gets the existing "Closed pipeline can not
> be opened" refusal, as today). The unchanged {{closePipeline}} (which closes
> any containers) and {{deletePipeline}} then run. For a pipeline that is still
> ALLOCATED the result is the same as before; only the CLOSED transition now
> comes before the container close. {{closePipeline}} itself is not changed.
> Covered by {{testScrubPipelinesKeepsPipelineOpenedDuringScrub}} in the
> existing {{TestPipelineManagerImpl}}: without the change the opened pipeline
> is deleted ({{Expecting actual: [] to contain exactly ...}}); with it all
> test classes of the SCM pipeline package, {{TestBackgroundSCMService}},
> {{TestSCMSafeModeManager}} and {{TestDeadNodeHandler}} pass, and checkstyle
> is clean. With the patch the reproduction fails at its first assertion: the
> pipeline stays OPEN, the container stays OPEN and no {{ClosePipelineCommand}}
> is sent.
> Related: HDDS-16887 (the scrub pass stops at a pipeline removed by another
> thread). Both patches change the loop in {{scrubPipelines}} and are made
> against the same commit, so they conflict textually; if HDDS-16887 lands
> first, this patch needs a trivial rebase (the two changed lines of the
> ALLOCATED branch move inside the new {{try}} block).
> Found by TLA+ model checking and code review of the SCM pipeline lifecycle
> paths under HDDS-15926, on commit b0aa6475b7856d2c7f467dd3db12b9a1dd116f63.
> Checked against HDDS issues and apache/ozone pull requests for duplicates
> before filing. The attached patch is a proposal for review. Generated with
> Specula (Claude Opus 5.5).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
