[
https://issues.apache.org/jira/browse/HDDS-16810?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16810:
------------------------------
Attachment: TestBugMC1ContainerBcsIdFromIncompletePeer.java
> Reconciliation raises the container BCSID to a block BCSID taken from a peer
> whose own container BCSID is lower
> ---------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-16810
> URL: https://issues.apache.org/jira/browse/HDDS-16810
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Major
> Attachments:
> MC-1-cap-container-bcsid-advance-at-peer-container-bcsid.patch,
> TestBugMC1ContainerBcsIdFromIncompletePeer.java
>
>
> h3. Mechanism
> After each peer round {{KeyValueHandler.reconcileContainerInternal}} calls
> {{advanceContainerBcsIdForFullyCoveredRound}}. When every block repaired in
> the round ended complete and the diff dropped no chunk as unhealthy, the
> container BCSID is raised to the highest block BCSID adopted in the round.
> The Javadoc of the method reasons that such a round inherits the peer's own
> container level claim, but the peer's container BCSID is never read, and a
> block BCSID on the peer says nothing about the peer's other blocks:
> * A peer that was itself only partially repaired holds complete blocks above
> its own container BCSID. It correctly did not raise its container BCSID, but
> it serves those blocks.
> * A block that both replicas hold in the same incomplete form is not in the
> diff, so the round counts as clean.
> A replica that reconciles with such a peer ends with a container BCSID that
> covers a committed chunk it does not hold. Nothing on the datanode corrects
> this afterwards: the scanner checks a block against its own metadata, which
> lists only the chunks the replica has (run).
> The logic came with HDDS-15542 and is on master only. Releases 2.1 and 2.2
> raise the container BCSID after every fully pulled block, which HDDS-15542
> replaced (read from the release code, not run). HDDS-16011 states the rule
> that is broken: a replica that advertises BCSID N holds every chunk committed
> through N.
> h3. Trigger
> In the reproduced schedules the container is QUASI_CLOSED in SCM and its
> replicas are QUASI_CLOSED or UNHEALTHY, and reconciliation is started with
> {{ozone admin container reconcile}}. Layout of the reproduction: two blocks
> of two chunks, block 0 committed at BCSID 1 and 2, block 1 at BCSID 3 and 4.
> d1 applied every commit.
> Schedule A, no RPC failure:
> # d1 has a corrupt second chunk in block 0, its scanner has found it and
> marked d1 UNHEALTHY. d2 and d3 are QUASI_CLOSED and empty.
> # d2 reconciles. It gets block 1 and the first chunk of block 0, and keeps
> container BCSID 0.
> # d3 reconciles and its first peer is d2. The round is clean, d3 adopts block
> 1 at BCSID 4 and raises its container BCSID to 4. The round with d1 cannot
> add the corrupt chunk.
> SCM sends the peers as a set, so their order is not controlled. SCM also
> sends the command to every replica at the same time, so whether d2 has
> finished its round with d1 before d3 starts its round with d2 depends on
> which datanode runs its command first. With d1 first in d3's list, d3 stays
> at 0 (the control in the reproduction).
> Schedule B, found by model checking, all three replicas QUASI_CLOSED and d1
> healthy:
> # d2 is empty, d3 applied only the first commit.
> # d2 reconciles and its GetBlock for block 0 to d1 fails. d2 gets block 1
> from d1 and the short block 0 from d3, and ends at container BCSID 1.
> # d3 reconciles while d1 is unreachable. It gets block 1 from d2, the round
> is clean, and its container BCSID goes from 1 to 4.
> h3. Impact
> * d3 is QUASI_CLOSED with container BCSID 4, the same as d1, and holds one of
> the two chunks of block 0 (run, both schedules).
> * SCM treats the two as equal. {{QuasiClosedContainerHandler}} force closes
> every QUASI_CLOSED replica with the highest sequence ID, and the datanode
> accepts the close for d3 (the datanode side is run in schedule B, the SCM
> decision is read from code). {{RatisUnderReplicationHandler}} keeps only
> replicas with the highest sequence ID as sources, so d3 can be the copy
> source when d2 is replaced (read from code, not run). The same value on a
> QUASI_CLOSED replica of a CLOSED container makes
> {{MismatchedReplicasHandler}} force close it (read from code, not run).
> * In schedule A the chunk is already damaged on d1, the only replica that had
> it. What the defect changes there is visibility:
> {{QuasiClosedContainerHandler}} keeps a container QUASI_CLOSED while an
> UNHEALTHY replica has a higher sequence ID than every QUASI_CLOSED one, and
> d3 at BCSID 4 lifts that hold (read from code, not run).
> * A client read of block 0 at its committed BCSID is still rejected on d3 by
> the block level check in {{BlockManagerImpl.getBlock}}, so the client fails
> over (read from code, not run). The container level check is the one that no
> longer protects.
> * A later reconcile of d3 against a complete healthy replica would repair the
> block (read from code, not run), but only an administrator starts one.
> h3. Reproduction
> PASS, deterministic, unmodified source.
> [^TestBugMC1ContainerBcsIdFromIncompletePeer.java]
> ({{hadoop-hdds/container-service}}) uses the setup of
> {{TestContainerReconciliationWithMockDatanodes}}: three datanodes, each a
> real {{KeyValueHandler}} with a real container on disk and a real on demand
> scanner, the real {{BlockInputStream}}, and only the static
> {{ContainerProtocolCalls}} transport routed to the peer, which answers the
> request for its checksum info through its own {{KeyValueHandler}}. There is
> no SCM. Each replica is created as the prefix of the commit log that it
> applied and is then quasi closed. Schedule B uses two switches in that
> routing (one GetBlock fails, one datanode is unreachable). The tests assert
> the data and the BCSID of d2 and d3 after each step, the control order for
> schedule A, and for schedule B that {{closeContainer}} leaves d3 CLOSED at
> BCSID 4 with the chunk missing. A passing test means the defect is present.
> h3. Patch
> [^MC-1-cap-container-bcsid-advance-at-peer-container-bcsid.patch], against
> 1e528b97aed3eef1be6846f1f0767c1ef98a883a. It also applies to master at
> 8ca44914771.
> {{GetContainerChecksumInfoResponseProto}} gets an optional field with the
> container BCSID of the replica that the checksum info comes from. The serving
> datanode reads it before it reads the tree. The reconciling datanode keeps
> the conditions from HDDS-15542 for a clean round and raises its container
> BCSID to the highest adopted block BCSID as before, but never above the value
> the peer reported. The field is on a datanode to datanode read response,
> nothing is persisted, and the proto compatibility check of the build passes
> without a change to {{proto.lock}}. The same field is the input for the rule
> proposed in HDDS-16011, raising the container BCSID to the higher of the two
> when the trees are equal.
> A peer that does not send the field is handled as today. During a rolling
> upgrade a round against a datanode without the patch can therefore still
> raise the BCSID too far, and the defect is closed once both datanodes run the
> patch. The other choice, never raising against such a peer, would leave
> replicas that were repaired completely below the sequence ID of the container
> for good, because a later reconcile finds an empty diff and adopts nothing.
> Limits:
> * When a partially repaired peer precedes a complete one in the list, the
> replica ends complete but with the BCSID of the blocks adopted from the
> complete peer only, which can be lower than that peer's container BCSID. Such
> a replica rejects reads of the blocks above its container BCSID, is not force
> closed, and after the container closes it is replaced and deleted as a stale
> copy (read from code; the resulting BCSID is run). A later reconcile does not
> raise it because the diff is then empty. HDDS-16011 already describes this
> case, a replica with complete content that claims a lower BCSID, and today's
> code already produces it for a replica whose own pull failed once (run).
> Adopting the peer's BCSID when the trees are equal is left to HDDS-16011.
> * If the only replica at the full BCSID is UNHEALTHY, that limit can leave
> every other replica complete and below it, and the container then stays
> QUASI_CLOSED (the hold in SCM is read from code). This needs no failed call.
> It needs a peer that holds blocks above its own container BCSID, which is
> also what the defect needs. Run at the datanodes: d1 is UNHEALTHY at 4 with a
> corrupt second chunk in block 0, d3 is at 2 and holds block 0 intact, d2 is
> empty. d2 reconciles first and ends complete at 2, with block 1 at BCSID 4.
> d3 then reconciles with d2 before d1 and ends complete at 2, and further
> reconciles of all three change no BCSID (d1 repairs its chunk and stays
> UNHEALTHY at 4). Without the patch d3 reaches 4 there, through the round that
> is the defect described here, and SCM can then close the container on d3
> (read from code). With the patch the container stays QUASI_CLOSED until a
> replica can adopt the container BCSID of a peer whose tree equals its own, as
> HDDS-16011 proposes. When d3 reconciles before d2 has pulled block 1, it
> reaches 4 from d1 with and without the patch.
> * The patch only ever lowers the value compared with today, so it adds no way
> to claim too much. It does not close the case where the peer's tree is older
> than the BCSID sent with it: a reconciling peer raises its container BCSID
> before it writes its tree (argued, not run). HDDS-12986 is the open issue for
> a new BCSID paired with an older tree.
> Covered in the existing {{TestContainerReconciliationWithMockDatanodes}} by
> {{testContainerBcsIdNotAdoptedAbovePeerContainerBcsId}} (a complete peer,
> whose BCSID is still adopted, and schedule A),
> {{testContainerBcsIdNotAdoptedAfterPeerFailureAndOutage}} (schedule B) and
> {{testContainerBcsIdAdoptedFromPeerThatDoesNotReportContainerBcsId}}. The
> first two use the layout of that suite, not that of the reproduction:
> replicas that start CLOSED, and three blocks that are each committed once, at
> BCSID 0, 1 and 2. In schedule A the corrupt chunk is the first one of block
> 0, so the partially repaired peer holds none of block 0. The mock datanodes
> of that class now answer the checksum info request through the real handler.
> With the cap taken out and the rest of the patch in place, two of the four
> invocations fail, the schedule A case of the first test and the second test,
> because the reconciling replica reports BCSID 2 while its peer is at 0. With
> the patch the unit suites of the {{keyvalue}}, {{ozoneimpl}} and {{checksum}}
> packages in {{hadoop-hdds/container-service}} pass (1127 tests, 18 skipped by
> the suites), checkstyle is clean on the three touched modules, and both
> schedules of the reproduction fail because d3 keeps its own container BCSID.
> Not run on a mini cluster, and not run against a datanode of an older version.
> Found by TLA+ model checking and code review of the container reconciliation
> paths in the datanode and SCM under HDDS-15926, on commit
> 1e528b97aed3eef1be6846f1f0767c1ef98a883a. 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]
