[
https://issues.apache.org/jira/browse/HDDS-16889?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16889:
------------------------------
Attachment: MC-4-reject-activate-deactivate-of-closed-pipeline.patch
> Admin pipeline activate reopens a CLOSED pipeline, and deactivate moves a
> CLOSED or ALLOCATED pipeline to DORMANT
> -----------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-16889
> URL: https://issues.apache.org/jira/browse/HDDS-16889
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Minor
> Attachments:
> MC-4-reject-activate-deactivate-of-closed-pipeline.patch,
> TestBugMC4ActivateClosedPipeline.java
>
>
> h3. Mechanism
> {{PipelineManagerImpl.activatePipeline}} and
> {{PipelineManagerImpl.deactivatePipeline}}, called by
> {{SCMClientProtocolServer}} for {{ozone admin pipeline activate|deactivate}}
> after the admin check, take the pipeline manager write lock and call
> {{PipelineStateManager.updatePipelineState}} without looking at the current
> state. {{PipelineStateMap.updatePipelineState}} has no transition check
> either, so any state can become OPEN or DORMANT, and a move to OPEN puts the
> pipeline back into the list used for container allocation.
> Everywhere else CLOSED is terminal: {{openPipeline}} refuses a CLOSED
> pipeline ("Closed pipeline can not be opened"), the scrubber only deletes a
> CLOSED pipeline, and the Javadoc of the two methods says "Activates a dormant
> pipeline" and "Deactivates an active pipeline". By the time a pipeline is
> CLOSED its containers have been finalized, and the way back to a writable
> pipeline is a new one from the background pipeline creator.
> h3. Trigger
> # A Ratis THREE or EC pipeline is OPEN and has a container.
> # An admin deactivates it (for example while debugging it).
> # SCM closes the pipeline on its own: a datanode of it goes stale and
> {{StaleNodeHandler}} closes its pipelines (other SCM side closes are
> {{DeadNodeHandler}}, an admin {{ozone admin pipeline close}} and the
> scrubber's unregistered node rule). The container goes to CLOSING and the
> pipeline to CLOSED. It stays listed until the scrubber deletes it after
> {{ozone.scm.pipeline.destroy.timeout}} (default 300s).
> # Within that window the admin runs {{ozone admin pipeline activate}} to undo
> step 2. The CLOSED pipeline becomes OPEN.
> Variants: {{deactivate}} on a CLOSED pipeline makes it DORMANT;
> {{deactivate}} on an ALLOCATED pipeline makes it DORMANT, after which neither
> the datanode report ({{openPipeline}} only opens ALLOCATED pipelines) nor the
> ALLOCATED timeout of the scrubber acts on it.
> h3. Impact
> * A reopened CLOSED pipeline is offered to
> {{ContainerManagerImpl.allocateContainer}} again, which creates a new OPEN
> container on it (run). The scrubber skips it because it is no longer CLOSED,
> so {{removePipeline}} never sends the {{ClosePipelineCommand}} to its
> datanodes (run, past the destroy timeout).
> * A CLOSED pipeline made DORMANT is skipped by the scrubber and
> {{deletePipeline}} refuses it with "is not yet closed" (run).
> * How long this lasts depends on who closed the pipeline. When the pipeline
> was closed because of a datanode close action, the datanodes resend that
> action with every heartbeat until the pipeline is removed from them
> (HDDS-9735), so SCM closes it again within one heartbeat interval and the
> container allocated in between goes to CLOSING. It stays OPEN, or DORMANT,
> only when SCM closed the pipeline itself (stale or dead node, admin close,
> scrubber), for example an EC pipeline closed by {{StaleNodeHandler}}; then it
> lasts until an admin closes it. These points are argued from code; the
> reproduction does not model datanodes.
> * Only an admin can trigger it, and no data loss was shown.
> h3. Reproduction
> PASS, deterministic, unmodified source.
> [^TestBugMC4ActivateClosedPipeline.java] ({{hadoop-hdds/server-scm}}) uses a
> real {{PipelineManagerImpl}} and {{ContainerManagerImpl}} on RocksDB with a
> mock node manager, the single node HA stub and a mock clock, and makes the
> calls SCM makes for a stale node close, the admin commands, container
> allocation and the scrubber. The three tests show a CLOSED pipeline reopened
> and given a new OPEN container and still OPEN after the destroy timeout
> scrub, a CLOSED pipeline made DORMANT that the scrubber and
> {{deletePipeline}} do not remove, and an ALLOCATED pipeline left DORMANT
> after a datanode report and the ALLOCATED timeout scrub. A passing test means
> the defect is present.
> h3. Patch
> [^MC-4-reject-activate-deactivate-of-closed-pipeline.patch], against
> b0aa6475b7856d2c7f467dd3db12b9a1dd116f63. It also applies to master at
> 6818707698c.
> {{activatePipeline}} throws {{InvalidPipelineStateException}} for a CLOSED
> pipeline, like {{openPipeline}}. Activating an ALLOCATED pipeline stays
> allowed, because it is used to force open pipelines (the freon
> {{SCMThroughputBenchmark}} and {{TestContainerCommandsEC}}).
> {{SCMThroughputBenchmark}} activates every listed pipeline, so a CLOSED
> pipeline in that list now makes it fail; that is unlikely because its fake
> datanodes never report the pipelines, which therefore stay ALLOCATED (not
> run). {{deactivatePipeline}} accepts only OPEN (DORMANT stays a no op) and
> throws for ALLOCATED and CLOSED. The ALLOCATED rejection is separate from the
> CLOSED one and can be dropped for the narrowest patch. The check runs on the
> leader under the same write lock as the state update, before the update is
> submitted, so the replicated apply path, the Ratis log and the RPC messages
> are unchanged. Operators now get an error for these transitions instead of a
> silent state change. An unknown pipeline ID now fails with
> {{PipelineNotFoundException}}, as for {{pipeline close}}, instead of
> succeeding silently.
> {{TestContainerCommandsEC#testCreateRecoveryContainerAfterDNRestart}}
> activated its EC pipeline again after a datanode restart, when
> {{StaleNodeHandler}} had already closed it. The write chunk that follows goes
> straight to the datanode, which does not check the SCM pipeline state, so the
> patch drops that activate call.
> Covered by {{testActivateAndDeactivateRejectClosedPipeline}} and
> {{testDeactivateRejectsAllocatedPipeline}} (which also asserts the unknown ID
> case) in the existing {{TestPipelineManagerImpl}};
> {{testDeactivatePipelineShouldFailOnFollower}} now opens its pipeline first
> so that it still tests the follower path. Without the change both new tests
> fail because nothing is thrown. With it all test classes of the SCM pipeline
> package and {{TestSCMClientProtocolServer}} (269 tests) and the whole
> {{TestContainerCommandsEC}} class (54 tests) pass, and checkstyle reports no
> violations. With the patch the three reproduction tests fail at the admin
> call with {{InvalidPipelineStateException}}.
> 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]
