[
https://issues.apache.org/jira/browse/HDDS-16808?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16808:
------------------------------
Attachment:
MC-8-refresh-container-data-checksum-when-block-deletion-changes-checksum-tree.patch
> Data checksum reported to SCM and stored in RocksDB is not refreshed when
> block deletion changes the container checksum tree
> ----------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-16808
> URL: https://issues.apache.org/jira/browse/HDDS-16808
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Minor
> Attachments:
> MC-8-refresh-container-data-checksum-when-block-deletion-changes-checksum-tree.patch,
> TestBugMC8BlockDeleteStaleDataChecksum.java
>
>
> h3. Mechanism
> A container's data checksum is kept in three places: the root of the checksum
> tree file, the in-memory value in {{ContainerData}} that container reports
> carry, and a RocksDB key that is read at startup. The scanner and
> reconciliation update all three together in
> {{KeyValueHandler.updateAndGetContainerChecksum}}. {{BlockDeletingTask}}
> calls {{ContainerChecksumTreeManager.addDeletedBlocks}}, which rewrites the
> tree file only.
> A deleted block is entered in the tree with the checksum computed from the
> chunk checksums in its {{BlockData}}. If the tree already lists the block
> with those checksums, the root does not change and nothing is wrong. If the
> tree does not list the block, or lists it with other checksums, the root
> changes while the in-memory value and the RocksDB key keep the old value.
> {{KeyValueContainerUtil.loadAndSetContainerDataChecksum}} takes the RocksDB
> key at startup when it is present, so a restart keeps the old value too.
> {{ContainerMerkleTreeTestUtils.verifyAllDataChecksumsMatch}} states the
> expectation that the three values are equal.
> h3. Trigger
> Three ways were run:
> # Interrupted reconciliation. dn1 misses a block, pulls it from a peer during
> reconciliation and commits it, and stops before the reconcile round writes
> its tree. After the restart the block is in RocksDB but not in the tree, and
> all three checksums agree. SCM then deletes the block.
> # No reconciliation and no restart, for a container written with checksum
> type NONE. The scanner does not add blocks without checksums to the tree
> (HDDS-16806), so each block deletion after a scan adds a block that the tree
> did not list. This route exists as long as the scanner leaves such blocks out.
> # No reconciliation and no restart, at default settings, for a block without
> chunks. The tree written when the container is closed lists every block,
> while the scanner adds a block to its tree only while it walks the block's
> chunks, so a scan drops a block without chunks from the tree and its deletion
> adds it back. Run with a block committed without chunks through
> {{BlockManager.putBlock}}. EC clients send such put block requests for
> padding stripes (read in {{ECBlockOutputStreamEntry}} and
> {{ECKeyOutputStream}}, not run).
> Two more follow from the code and were not run: deleting a block after the
> scanner and reconciliation race of HDDS-14908 left a tree without that block,
> and a deleted block entry taken from a peer during reconciliation that block
> deletion later overwrites with a different locally computed checksum.
> h3. Impact
> The datanode reports a data checksum that differs from the tree it serves to
> its peers. {{ozone admin container reconcile --status}} ({{replicasMatch}})
> and the REPLICA_MISMATCH check in Recon compare the reported values (read in
> the code, not run), so they show a mismatch between replicas whose trees are
> identical. In the first test both trees have root 57f7e8c3 and dn1 keeps
> reporting 4d1ae7e2. For EC containers this is not visible today: {{reconcile
> --status}} accepts only RATIS containers and Recon compares EC replicas only
> within the same replica index (read, not run). SCM only stores and returns
> the value today. HDDS-16012 proposes that SCM flag replicas with equal BCSID
> and different data checksums, which a stale value would trigger. No data is
> affected, and reconciliation itself compares the trees, not the reported
> values.
> The old value stays until the next data scan of the container (at least 7
> days apart by default) or its next reconciliation. A data scan correcting it
> was run separately and is not part of the attached test.
> {{BlockDeletingTask}} calls {{addDeletedBlocks}} without refreshing the other
> two values in the releases 2.1.0, 2.2.0 and 2.2.1 as well (read in the
> release tags).
> h3. Reproduction
> PASS, deterministic, unmodified source.
> [^TestBugMC8BlockDeleteStaleDataChecksum.java]
> ({{hadoop-hdds/container-service}}, about 3 seconds, no SCM) has one test for
> each of the three triggers above. Each datanode is a real {{KeyValueHandler}}
> over a real volume, RocksDB and tree file, with the real on-demand scanner,
> {{DeleteBlocksCommandHandler}}, {{BlockDeletingService}} and, for restarts,
> {{ContainerReader}}. Only the datanode to datanode transport is routed in
> process. In the first test the reconcile thread is parked on entry to
> {{ContainerChecksumTreeManager.updateTree}} and never resumed, which stands
> in for the datanode stopping at that point. All three tests show the tree
> root changed by the delete while the container report and RocksDB keep the
> previous value, and the first shows that a restart keeps it. A passing test
> means the defect is present.
> h3. Patch
> [^MC-8-refresh-container-data-checksum-when-block-deletion-changes-checksum-tree.patch],
> against 1e528b97aed3eef1be6846f1f0767c1ef98a883a. It also applies to master
> at 8ca44914771.
> {{ContainerChecksumTreeManager.addDeletedBlocks}} returns the checksum info
> it wrote. {{BlockDeletingTask}} compares its data checksum with the in-memory
> value at both call sites and, when they differ, writes the RocksDB key and
> sets the in-memory value. Nothing is done while the tree has no data
> checksum, so a container that was never scanned does not get one from a
> delete. No log line is written, because on this path the change is expected.
> Cost: one comparison per deletion pass of a container. The RocksDB write
> happens when the root changed, which for a container whose scanner tree
> leaves blocks out (triggers 2 and 3) is every pass that deletes such a block.
> The RocksDB key is written before the in-memory value. A failed write fails
> the deletion pass before the block metadata and the delete transactions are
> removed from RocksDB (the chunk files are already gone at that point, as with
> any failure of the batch), the in-memory value is unchanged and the pass is
> repeated. The scanner and reconciliation set the in-memory value first and
> only log a failed write.
> The refresh runs after the tree file lock is released, like the existing
> update by the scanner and reconciliation, so a tree write by one of them that
> lands between the delete's tree write and its refresh leaves the reported
> value and the RocksDB key at the delete's root while the file has the later
> root, until the next scan, reconciliation or block deletion pass (run in a
> separate probe with an injected tree update; HDDS-14908 covers the same kind
> of interleaving between the scanner and reconciliation).
> No ICR is sent: block deletion sends none for its other effects either. The
> value reaches SCM with the next full container report
> ({{hdds.container.report.interval}}, 60 minutes by default).
> A datanode that stops between the tree write and the RocksDB write restarts
> with the old value from RocksDB. For block deletion this closes by itself,
> because the delete transaction is still pending and the pass is repeated (run
> in a separate probe). For the scanner and reconciliation the same window
> exists today and is not changed here, except that a later block deletion pass
> in the container now corrects the value (argued, not run).
> Covered by {{testDataChecksumUpdatedWhenDeleteChangesChecksumFile}} in the
> existing {{TestBlockDeletingService}}, for all layout and schema versions. It
> also checks that a delete does not give an unscanned container a data
> checksum. Without the change it fails with "In-memory data checksum should
> match the one in the checksum file". With it the block deleting, delete
> command handler, checksum tree, container reader, key value container and
> handler, reconciliation with mock datanodes and disk balancer suites of
> {{hadoop-hdds/container-service}} (605 tests) and {{TestBlockDeletion}} on a
> mini cluster (6 tests) pass, checkstyle is clean. With the patch all three
> tests of the reproduction fail, because the container report follows the tree
> after the delete.
> 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]
