[
https://issues.apache.org/jira/browse/HDDS-16892?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16892:
------------------------------
Attachment: MC-9-keep-pending-allocations-on-datanode-reregistration.patch
> SCM drops the pending container allocations of a datanode that re-registers
> with a new version or port set, so its last slot can be granted twice
> -------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-16892
> URL: https://issues.apache.org/jira/browse/HDDS-16892
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Minor
> Attachments:
> MC-9-keep-pending-allocations-on-datanode-reregistration.patch,
> TestBugMC9ReRegisterDropsPendingAllocations.java
>
>
> h3. Mechanism
> When a registered datanode registers again with the same ID but a changed
> version string or port set, {{SCMNodeManager.register}} calls
> {{updateNodeAndTopology}}, and {{NodeStateManager.updateNode}} replaces the
> stored {{DatanodeInfo}} with a new object. The {{DatanodeInfo}} constructor
> creates a new, empty {{TwoWindowBucket}}, and nothing copies the entries of
> the old one. Pending allocations of containers that SCM has allocated on the
> datanode but that the datanode has not created yet (containers are created on
> the first write) are therefore lost. Once the next heartbeat brings a node
> report, {{hasAvailableSpace}} and {{checkSpaceAndRecordAllocation}} see the
> slot as free and grant it again. The old bucket stays reachable only through
> stale references, and later container reports update the new one.
> An address change takes the same path, but there {{NODE_ADDRESS_UPDATE}}
> makes {{PipelineManagerImpl.closeStalePipelines}} close the pipelines with
> the old address, so the dropped entries belong to containers that can no
> longer be created there.
> h3. Trigger
> # A datanode has room for one more container, and SCM allocates container C1
> on it.
> # Before C1 is written, the datanode restarts and re-registers with a new
> version (for example in a rolling upgrade), within the stale node interval so
> its pipeline stays open. A re-registration with a changed port set takes the
> same code path.
> # The next node report arrives, and SCM allocates or places C2 on the same
> datanode.
> h3. Impact
> The datanode is promised more containers than it has room for, which is what
> {{PendingContainerTracker}} exists to prevent. Only reservations that are
> younger than two roll intervals
> ({{ozone.scm.pending.container.roll.interval}}, default 5m) and not yet
> confirmed by a container report are affected, and only datanodes that are
> down to their last free slots can be over granted. The expected effect is
> that the datanode refuses one of the containers for lack of space when it is
> created. On a Ratis pipeline the failed create marks the pipeline's
> {{ContainerStateMachine}} unhealthy, so the pipeline is closed and the in
> flight writes on it are retried elsewhere. This is argued from the code and
> was not run.
> h3. Reproduction
> [^TestBugMC9ReRegisterDropsPendingAllocations.java]
> ({{hadoop-hdds/server-scm}}) uses a real SCM from {{HddsTestUtils.getScm}}
> with a 1GB container size and one datanode with 1.5GB usable, so exactly one
> slot. It records C1 through
> {{PipelineManagerImpl.checkSpaceAndRecordAllocation}} (the call
> {{ContainerManagerImpl.allocateContainer}} makes), checks that the slot is
> taken, re-registers the datanode with a new version or a new port set,
> processes the next node report and asserts that the slot is still taken. On
> an unmodified checkout both cases fail at that check ({{slot held by C1 must
> not look free}}), so the second allocation would be granted. With the patch
> both pass.
> h3. Patch
> [^MC-9-keep-pending-allocations-on-datanode-reregistration.patch], against
> b0aa6475b7856d2c7f467dd3db12b9a1dd116f63. It also applies to master at
> 6818707698c.
> {{DatanodeInfo}} gets a package private constructor that takes an existing
> {{TwoWindowBucket}}, and {{NodeStateManager.updateNode}} passes the bucket of
> the info it replaces. The same instance is shared, so a thread that still
> holds the old {{DatanodeInfo}} records into the live bucket, and the report
> that confirms the container removes the entry as usual. Everything else in
> the replacement is reset as before. For an address change the entries are now
> kept as well, although their pipelines are closed. That is the conservative
> direction: it can only under grant on that datanode, for at most two roll
> intervals, after which the entries age out like any unconfirmed entry.
> Covered by
> {{TestSCMNodeManager#testRegisterKeepsPendingAllocationsOnVersionOrPortChange}},
> which fails without the change ({{expected: <1> but was: <0>}} for the
> pending count after the version change). With the change 118 tests in the
> node package suites pass, and checkstyle is clean.
> Found by TLA+ model checking and code review of the SCM pending container
> allocation tracking 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]
