[
https://issues.apache.org/jira/browse/HDDS-16811?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16811:
------------------------------
Attachment: MC-2-do-not-restore-deleted-blocks-in-reconciliation.patch
> Reconciliation can restore a deleted block, or commit it without its file
> when block deletion runs during the repair
> --------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-16811
> URL: https://issues.apache.org/jira/browse/HDDS-16811
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Major
> Attachments:
> MC-2-do-not-restore-deleted-blocks-in-reconciliation.patch,
> TestBugMC2DeletedBlockRestoredByReconciliation.java,
> TestBugMC3BlockDeletedDuringReconciliation.java
>
>
> h3. Mechanism
> {{BlockDeletingTask.deleteTransactions}} handles a delete transaction for a
> block that has no metadata in the container by calling
> {{KeyValueHandler.deleteUnreferenced}}, which removes the block file if there
> is one, and then removes the transaction like any other. A block that has
> metadata is also recorded as deleted in the container merkle tree
> ({{ContainerChecksumTreeManager.addDeletedBlocks}}). A block without metadata
> is not, so the tree keeps no trace of the deletion.
> Reconciliation copies every block that is live in the peer's tree and has no
> entry in the local tree. A deleted entry in the local tree is the only thing
> that makes it skip a block. {{KeyValueHandler.reconcileChunksPerBlock}} then
> pulls the chunks, writes them and commits the block metadata with
> {{putBlockForClosedContainer}} without holding the container lock, while the
> block deleting service works under the container write lock. Nothing orders a
> deletion against the copy of the same block.
> Two results, depending on when the block deleting service handles the
> transaction:
> # Before reconciliation reads the local tree: the block is copied back from a
> peer that has not deleted it yet, and no transaction is left to delete it.
> # While reconciliation copies the block: the block file is deleted and
> reconciliation still commits the block metadata, so the replica has block
> metadata without a block file.
> The same code is in the tags ozone-2.1.0, ozone-2.2.0 and ozone-2.2.1 (read
> with git grep on the tags, not run there).
> h3. Trigger
> # A QUASI_CLOSED container has a replica that lacks a block the other
> replicas have, for example a replica that was quasi closed behind the others.
> This is the state reconciliation exists to repair.
> # The key is deleted. SCM sends the block delete transaction to every
> replica, including the one without the block.
> {{DeletedBlockLogImpl.getTransactions}} sends it when the replication health
> of the container is HEALTHY, and a QUASI_CLOSED container whose replicas are
> all QUASI_CLOSED is HEALTHY whatever their sequence IDs are
> ({{RatisReplicationCheckHandler}} run in a unit test, {{DeletedBlockLogImpl}}
> read, not run).
> # {{ozone admin container reconcile}} is run for the container.
> The state of steps 1 and 2 is short lived by default (read in the code, not
> run). With replicas of all three origins present, the next replication
> manager pass ({{hdds.scm.replication.thread.interval}}, 300s) closes the
> replicas with the highest sequence ID ({{QuasiClosedContainerHandler}}).
> After that the lagging replica counts as unhealthy, the container is under
> replicated and its delete transactions are held back. A container that is
> stuck in QUASI_CLOSED with replicas of two origins is kept at five replicas
> by default ({{hdds.scm.replication.quasi.closed.stuck.best.origin.copies}} 3,
> {{other.origin.copies}} 2), which the same health check rates as over
> replicated. So the transaction has to be sent between the quasi close and the
> next replication manager pass, or while a stuck container has exactly three
> replicas. A transaction that a datanode has already stored stays pending
> there and is executed as long as its replica is CLOSED or QUASI_CLOSED.
> For the first result the lagging replica's block deleting service handles the
> transaction before a peer's does, and reconciliation with that peer runs in
> between. Each datanode runs the service on its own schedule. For the second
> result the transaction is still pending on the lagging replica and its block
> deleting service reaches it between the first chunk write and the commit of
> that block, which is the time it takes to pull the block's chunks from the
> peer. No fault is needed. How often either ordering occurs on a cluster was
> not measured.
> h3. Impact
> * Restored block: data of a deleted key stays on one datanode. SCM completes
> the transaction once every replica has acknowledged the command, which a
> datanode does when it stores the transaction, so it is not sent again (read,
> not run). A later reconciliation does not remove the block: once the peers
> have deleted it, their deleted entry carries the same checksum as the live
> block, all replicas report the same data checksum, and reconciling with both
> peers leaves the block in place (run). So no existing check shows the block,
> and a replication that uses this replica as its source copies it along,
> because the block files, the metadata and the tree file are all part of the
> container archive (read in {{TarContainerPacker}}, not run). The replica
> never becomes empty, and SCM deletes an empty container only when all its
> replicas report empty (read in {{EmptyContainerHandler}}, not run), so the
> container stays on all datanodes.
> * Metadata without file: the scan that reconciliation starts reports
> {{MISSING_DATA_FILE}} and marks the replica UNHEALTHY (run). If the replica
> had the block with only its first chunk, the same ordering ends in
> {{MISSING_CHUNK}} and UNHEALTHY (run). SCM is then expected to replace the
> replica from a healthy one (argued, not run). No data of a live key is lost
> in either case.
> h3. Reproduction
> PASS for both on unmodified source at
> 1e528b97aed3eef1be6846f1f0767c1ef98a883a, deterministic, module
> {{hadoop-hdds/container-service}}. Each datanode is a real
> {{KeyValueHandler}} on its own volume with the real
> {{DeleteBlocksCommandHandler}}, {{BlockDeletingService}} and on demand
> scanner. There is no SCM and no network: the delete transaction is handed to
> the command handler, and the datanode to datanode calls of reconciliation are
> answered in process by the peer's handler. A passing test means the defect is
> present.
> * [^TestBugMC2DeletedBlockRestoredByReconciliation.java]: three QUASI_CLOSED
> replicas, dn1 without block 1, the transaction stored on all three. After
> dn1's block deleting service ran: no metadata, no tree entry, no pending
> deletion. After dn1 reconciled with dn2, which has not deleted yet: metadata
> and block file present, tree entry live, no pending deletion. After dn2 and
> dn3 deleted the block and dn1 reconciled with both, ran its block deleting
> service and was scanned again: unchanged, and all three replicas report the
> same data checksum. After the remaining block is deleted, dn2 and dn3 are
> marked empty and dn1 is not.
> * [^TestBugMC3BlockDeletedDuringReconciliation.java]: two replicas, the
> transaction pending on dn1. One pass of dn1's block deleting service runs at
> the moment the peer is asked for the second chunk of the block. It runs in
> the reconciliation thread, which is possible because reconciliation holds no
> container lock there. On a datanode it is a separate thread. Result: block
> metadata present, block file absent, {{MISSING_DATA_FILE}}, replica UNHEALTHY.
> h3. Patch
> [^MC-2-do-not-restore-deleted-blocks-in-reconciliation.patch]. It applies on
> top of the patch attached to HDDS-16808 (the data checksum reported to SCM
> and stored in RocksDB is not refreshed when block deletion changes the
> container checksum tree), not to the tree without it. Both patches change the
> same call in {{BlockDeletingTask.deleteTransactions}}, and the entries added
> here move the tree root on ordinary deletes, so the refresh from HDDS-16808
> is needed for the reported data checksum to follow. Developed on
> 1e528b97aed3eef1be6846f1f0767c1ef98a883a plus that patch. The two patches in
> that order also apply to master at 8ca44914771. Below, with the patch means
> with both patches.
> Two parts, and the second relies on the first:
> # {{BlockDeletingTask.deleteTransactions}} passes the IDs of the blocks it
> found no metadata for to {{ContainerChecksumTreeManager.addDeletedBlocks}},
> which marks them as deleted in the tree write the pass already does. An entry
> that exists keeps its checksum, a new entry gets the checksum of a block
> without chunks. The call goes through the refresh that HDDS-16808 adds, so
> the data checksum in memory and in RocksDB follows the tree file. This is
> enough for the first result when the deletion is complete before
> reconciliation reads the local tree.
> # {{KeyValueHandler.reconcileChunksPerBlock}} closes the block file handle
> before the commit instead of after it, as the client put block paths do, and
> then commits under the container read lock only if the tree on disk does not
> mark the block as deleted. The read lock orders the check and the commit
> against the block deleting service, which holds the write lock for its whole
> pass.
> A block that is deleted during its repair leaves the patched code by one of
> two exits (both run):
> * The block file was removed while its handle was cached and later chunks
> were still written, which is the ordering of the second reproduction.
> {{finishWriteChunks}} then fails with "Chunk file not found" before any
> metadata is written. Moving that call in front of the commit is what stops
> this ordering. As for every caller of {{finishWriteChunks}}, the failure also
> requests an asynchronous volume check, which is throttled
> ({{hdds.datanode.disk.check.min.gap}}, 10m) and finds a healthy volume. The
> unpatched code makes the same request in this ordering, after the commit.
> * A block file exists at commit time, because the deletion ran before the
> first chunk write or the block was partly present. The tree check finds the
> deleted entry, the pulled chunks are removed and the block fails.
> Cost. The delete path gets no new lock and no additional file write, but the
> tree write it already does now changes the data checksum, see the side effect
> below. Reconciliation holds the read lock only for the commit of each
> repaired block, not while it pulls chunks. It reads and parses the tree file
> once per repaired block: measured 0.5 ms for a tree of 1,280 blocks, 1 ms for
> 10,000 and 10 ms for 100,000 (3 MB file), one chunk per block, warm page
> cache. That is noise for the usual repair of a few blocks and quadratic for a
> replica that misses most blocks of a container with very many small blocks
> (about 16 minutes in total for 100,000). Skipping the read while the tree is
> unchanged, for example with a write counter in
> {{ContainerChecksumTreeManager}}, is a possible follow up. Holding the read
> lock for the whole repair of a block would also work, but it would block
> block deletion and delete commands for the container across network reads
> from the peer.
> Side effect of the first part. Every delete of a block without metadata now
> adds a deleted entry, also for a block that no replica ever had. That is
> routine: the unused blocks of a committed key are deleted this way
> ({{OMKeyRequest.wrapUncommittedBlocksAsPseudoKey}}, read). Each such entry
> changes the data checksum in the tree of the replica that executes the
> delete, and the value the replica reports follows it in the same pass (run),
> where the unpatched code leaves the tree alone. Replicas on the patched
> version are equal again once all have executed the transaction. A replica on
> an older version, or one that executed the transaction before the upgrade,
> has no such entry and differs until it is reconciled, which only {{ozone
> admin container reconcile}} starts. SCM does not act on the value, and Recon
> lists the container as REPLICA_MISMATCH while the reported values differ
> (read). Trees written before the patch are not backfilled, so a delete
> executed before the upgrade is not protected by the patch. For a block that
> only some replicas had, the entries differ in checksum until the replicas
> reconcile, as the missing entry does today. A stricter alternative, not taken
> here, is to write the new entries only once the datanode layout is finalized
> on a version that has them, which needs a new layout feature.
> Covered by two new tests in the existing
> {{TestContainerReconciliationWithMockDatanodes}} (one per result, the second
> with three interleavings), an added assertion in
> {{TestBlockDeletingService.testWithUnrecordedBlocks}} and
> {{TestContainerMerkleTreeWriter.testMarkBlocksDeleted}}. One of the new tests
> also asserts that after a delete pass that records an entry for a block the
> replica never had, the data checksum in memory, in RocksDB and in the tree
> file agree. On HDDS-16808 alone the tests in the first two classes fail (8
> failures). With both patches 534 tests in 19 suites of
> {{hadoop-hdds/container-service}} and {{TestBlockDeletion}} on a mini cluster
> pass, checkstyle is clean. {{TestContainerCommandReconciliation}} was not run
> because its secure mini cluster did not start in the test environment. It is
> the existing suite most likely to notice the change to what block deletion
> writes to the tree, so it should pass in CI before the patch goes in. With
> both patches the two reproductions fail: the block is not restored, and in
> the second no metadata is committed and the replica stays QUASI_CLOSED. With
> HDDS-16808 alone they still pass.
> Not covered by the patch: a replica that already holds a restored block is
> not cleaned up.
> 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. Related but
> different: HDDS-11765 (apply block deletions that a replica missed, open),
> HDDS-13712 (more tests for deleted block handling in reconciliation, open)
> and HDDS-10929 (coordinate reconciliation with container deletion and
> replication, open). 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]
