[ 
https://issues.apache.org/jira/browse/HDDS-16887?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Siyao Meng updated HDDS-16887:
------------------------------
    Attachment: TestBugCR1ScrubPassAbortedByRemovedPipeline.java

> Pipeline scrubber pass stops at a pipeline removed by another thread and 
> skips the remaining pipelines until the next pass
> --------------------------------------------------------------------------------------------------------------------------
>
>                 Key: HDDS-16887
>                 URL: https://issues.apache.org/jira/browse/HDDS-16887
>             Project: Apache Ozone
>          Issue Type: Bug
>            Reporter: Siyao Meng
>            Priority: Minor
>         Attachments: CR-1-scrubber-skip-pipeline-removed-during-pass.patch, 
> TestBugCR1ScrubPassAbortedByRemovedPipeline.java
>
>
> h3. Mechanism
> {{PipelineManagerImpl.scrubPipelines}} loops over the pipeline list read at 
> the start of the pass with no exception handling per pipeline. If another 
> thread removes a listed pipeline before the scrubber reaches it, 
> {{closePipeline}} (through {{closeContainersForPipeline}}) or 
> {{deletePipeline}} throws {{PipelineNotFoundException}}, the loop ends, and 
> {{scrubAndClosePipelinesMissingDataStreamPort}} logs "Unexpected error during 
> pipeline scrubbing" at ERROR with the stack trace. Pipelines are removed 
> outside the scrubber by {{DeadNodeHandler.destroyPipelines}} and by 
> {{closeStalePipelines}} (from {{NewNodeHandler}} and 
> {{NodeAddressUpdateHandler}}). {{DeadNodeHandler}} already ignores 
> {{PipelineNotFoundException}} for the same {{closePipeline}} and 
> {{deletePipeline}} calls, and {{closePipelinesMissingDataStreamPort}} catches 
> errors per pipeline.
> h3. Trigger
> # A scrub pass has several candidates (ALLOCATED past 
> {{ozone.scm.pipeline.allocated.timeout}}, CLOSED past 
> {{ozone.scm.pipeline.destroy.timeout}}, or OPEN with unregistered nodes).
> # While the scrubber works on one of them, another thread removes a later 
> one, for example on a dead node event.
> # The scrubber reaches the removed pipeline and the pass ends.
> The reproduction drives the removal from the scrubber's own thread at a fixed 
> point; a removal racing a pass on a cluster was not exercised.
> h3. Impact
> * The candidates after the removed pipeline wait for the next pass, one 
> {{ozone.scm.pipeline.scrub.interval}} later (default 5m). The next pass reads 
> the list again and handles them (run).
> * An ERROR log with stack trace for a benign race.
> * No data impact.
> h3. Reproduction
> PASS, deterministic, unmodified source. 
> [^TestBugCR1ScrubPassAbortedByRemovedPipeline.java] 
> ({{hadoop-hdds/server-scm}}) uses a real {{PipelineManagerImpl}} on RocksDB 
> with the single node HA stub, a mock node manager and a mock clock. Four 
> pipelines are past the ALLOCATED timeout. A Mockito spy on {{closePipeline}} 
> closes and deletes the second pipeline through the public API when the 
> scrubber starts closing the first one, as {{DeadNodeHandler}} would. The pass 
> runs through {{scrubAndClosePipelinesMissingDataStreamPort}}, the method the 
> background scrubber calls. After the first pass the error is logged and the 
> third and fourth pipelines are still there; after the second pass none are 
> left. A passing test means the defect is present.
> h3. Patch
> [^CR-1-scrubber-skip-pipeline-removed-during-pass.patch], against 
> b0aa6475b7856d2c7f467dd3db12b9a1dd116f63. It also applies to master at 
> 6818707698c.
> The loop body of {{scrubPipelines}} is wrapped in a catch of 
> {{PipelineNotFoundException}} that logs at DEBUG and continues with the next 
> pipeline, as {{DeadNodeHandler}} does. Other exceptions, for example losing 
> SCM leadership during the pass, still end the pass and are logged once by the 
> caller as before. Nothing changes when no pipeline is removed concurrently. 
> The change reindents the loop body; {{git diff -w}} shows five added lines.
> Covered by {{testScrubPipelinesSkipsPipelineRemovedDuringScrub}} in the 
> existing {{TestPipelineManagerImpl}}: without the change {{scrubPipelines}} 
> throws ({{Unexpected exception thrown: ... PipelineNotFoundException: 
> Pipeline not found: ...}}); with it the pipeline after the removed one is 
> scrubbed in the same pass. 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: no error is logged and all 
> pipelines are removed in the first pass.
> Related: a separate issue for the same loop, where the scrubber closes and 
> deletes a pipeline that was opened during the pass after it was read as a 
> timed out ALLOCATED pipeline. Its proposed patch conflicts textually with 
> this one and will be rebased if this one lands first.
> Found by code review of the SCM pipeline lifecycle paths, as part of the TLA+ 
> verification effort 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]

Reply via email to