[
https://issues.apache.org/jira/browse/HDDS-16830?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16830:
------------------------------
Attachment: TestBugMC6RecoveryFailsOnNeverWrittenLastBlock.java
> Lease recovery matches open key and fileTable blocks by count, so it can drop
> blocks, repeat the last block, never complete or commit a hole
> --------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-16830
> URL: https://issues.apache.org/jira/browse/HDDS-16830
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Critical
> Attachments:
> MC-2-lease-recovery-match-open-key-blocks-by-identity.patch,
> TestBugCR3ForceRecoverySplicesBlockAfterStaleBlock.java,
> TestBugMC2RecoveryDropsBlocksAheadOfFileTable.java,
> TestBugMC3RecoveryRepeatsLastBlock.java,
> TestBugMC6RecoveryFailsOnNeverWrittenLastBlock.java
>
>
> h3. Mechanism
> {{LeaseRecoveryClientDNHandler.getOmKeyLocationInfos}} builds the block list
> of the recovery commit from the fileTable entry (the blocks as of the last
> hsync that reached OM) and the open key (every allocated block). It compares
> the two by block count and looks only at the open key's last two blocks: it
> appends the open key's last block when the open key has more blocks, then
> refreshes fileTable's last block only when that is the open key's penultimate
> block or the counts are equal. Only there is {{NO_SUCH_BLOCK}} or
> {{CONTAINER_NOT_FOUND}} accepted as "allocated but never written". With the
> force property any other failure is logged and the list is committed as it is.
> {{BlockOutputStreamEntryPool.hsyncKey}} calls OM only when the last block ID
> changed, so a writer without hsync gets several blocks ahead of fileTable. A
> never written block is left out of the hsync list but stays in the open key,
> possibly in front of fileTable's last block.
> OM does not catch the result: the recovery commit
> ({{OMKeyCommitRequestWithFSO}}) accepts any subset of the open key's blocks
> and moves the others to {{deletedTable}}, drops a repeated block ID and
> stores the client's {{dataSize}} as is.
> h3. Trigger
> Only with hsync enabled, which is not the default: {{ozone.fs.hsync.enabled}}
> and {{ozone.hbase.enhancements.allowed}}
> ({{ozone.client.hbase.enhancements.allowed}} on the client) are false. The
> writer of an hsynced file is gone and another client calls {{recoverLease}}.
> # *Open key two or more blocks ahead*, fileTable {{[b1]}}, open key {{[b1,
> b2, b3]}}: the writer wrote more than a full block after its last hsync that
> reached OM. No fault needed. A never written block counts, so {{[b1, b2 never
> written, b3]}} also hits a writer that hsyncs often (read from the code, not
> run). The force property ({{OZONE.CLIENT.RECOVER.LEASE.FORCE}}, a client
> system property) makes no difference here or in shape 2 (read, not run with
> it set).
> # *Never written block in front of the last block*, fileTable {{[b2]}}, open
> key {{[b1, b2]}}: the first write to b1 failed (for example its container was
> closed in between), the client moved on to b2 and called hsync there.
> # *As shape 1, but the open key's last block was never written*: the writer
> stopped right after that block was allocated. No fault needed.
> # *Open key one block ahead, the earlier block unreachable, force set*,
> fileTable {{[b1]}}, open key {{[b1, b2]}}: the writer wrote more into b1
> after its hsync, and no replica of b1 can return its length while b2 answers.
> The handler logic is the same in the release tags ozone-2.0.0 to ozone-2.2.1
> (compared in the tagged source, nothing run on a release).
> h3. Impact
> Reproduced on unmodified source (FSO bucket, 128 KB blocks, 16 KB chunks).
> * *Shape 1, blocks dropped and hsynced bytes lost.* Two hsyncs acknowledged
> 32768 bytes in b1, then the writer flushed on to 262244 bytes.
> {{recoverLease}} returns true and the file is closed as {{[b1:16384,
> b3:100]}}: the second hsync is not covered, b2 is in {{deletedTable}}, and
> the content is not a prefix of what was written.
> [^TestBugMC2RecoveryDropsBlocksAheadOfFileTable.java]
> * *Shape 2, wrong file length.* A file of 100 bytes is closed with length 200
> and block list {{[b2:100]}}. {{readFully}} of 200 bytes throws
> {{EOFException}} and the bucket's {{usedBytes}} goes from 300 to 600. The 100
> bytes stay readable. [^TestBugMC3RecoveryRepeatsLastBlock.java]
> * *Shape 3, recovery cannot complete.* Every {{recoverLease}} throws
> {{StorageContainerException}}. The file stays open with the
> {{LEASE_RECOVERY}} mark, {{OmMetadataManagerImpl.getExpiredOpenKeys}} no
> longer selects it for cleanup and an hsync of the writer is rejected. Only a
> commit removes the mark (read), which leaves delete, overwrite or force.
> Force closes the file at the fileTable length, 16384 of 262144 bytes (seen in
> a run with the property set), dropping later hsyncs in that block too (read).
> [^TestBugMC6RecoveryFailsOnNeverWrittenLastBlock.java]
> * *Shape 4, hole after a forced recovery.* After an hsync at 100 bytes the
> writer flushed 1000 more to b1 and 100 to b2. Forced recovery closes the file
> with 200 bytes, bytes 0 to 99 followed by bytes 1100 to 1199. Least severe of
> the four: it needs the force property, set by hand after a normal recovery
> failed, and every replica of b1 down while b2 answers.
> [^TestBugCR3ForceRecoverySplicesBlockAfterStaleBlock.java]
> The wrong list and length are committed to the closed key, and the patch does
> not repair files already recovered this way.
> h3. Reproduction
> The four attached classes are self contained and pass on unmodified source
> while the bug is present. Each drives the file through the FileSystem API on
> a mini cluster in {{hadoop-ozone/integration-test}}. The fault setup of
> shapes 2 and 4, the target path and the Maven command are in each class
> comment.
> h3. Patch
> [^MC-2-lease-recovery-match-open-key-blocks-by-identity.patch], against
> ea69b4a7d9abd040e5259dbcbb7c6ce52eb5d199, also applies to master at
> bdff9801bb2. Client only.
> The handler now finds fileTable's last block in the open key by container and
> local ID, finalizes it, then finalizes every open key block listed after it,
> in order, and appends it. A block for which the datanodes answer
> {{NO_SUCH_BLOCK}} or {{CONTAINER_NOT_FOUND}} was never written and is left
> out. Blocks listed before fileTable's last block are never appended.
> With the force property the result is the longest prefix whose lengths are
> known: fileTable's blocks, the last one at its datanode length if that could
> be read, then the following blocks up to the first whose length could not be
> read. If fileTable's last block cannot be read nothing is appended after it,
> even if its fileTable length was already final; the blocks left out hold no
> byte acknowledged by an hsync and OM moves them to {{deletedTable}} as
> uncommitted (both read).
> Not covered by the patch:
> * *Open key three or more blocks ahead.* {{OMRecoverLeaseRequest}} returns a
> pipeline only for fileTable's last block and the open key's last two blocks,
> so the client has no datanode to ask for a block in between. The patched
> client then fails before it finalizes anything, with an {{IOException}}
> naming the block and the force property, and drops nothing (unit test, not
> run on a cluster). The file stays open and marked as in shape 3; with force
> it is closed with the blocks before that block, fileTable's last block at its
> datanode length. The follow up is for OM to return the pipelines of all
> blocks after fileTable's last one in one batched SCM call, once that call no
> longer runs while the request is applied, see HDDS-16823, which moves the SCM
> call of {{RecoverLease}} out of the apply path. It is not part of this patch
> because it would add one SCM call per block to the apply path.
> * *A block for which SCM knows no datanode.* Recovery still fails, now with
> an {{IOException}} that names the block in place of the
> {{IllegalArgumentException}} of {{XceiverClientManager.acquireClient}} (unit
> test, cluster state not reproduced).
> * *A block the writer abandoned.* Like the existing code, the patch takes a
> datanode length as one the writer accepted.
> {{KeyOutputStream.handleExceptionInternal}} rewrites the unacknowledged data
> of a failed block into a new block while the datanodes may have applied the
> failed write, so such a block is appended as the datanodes have it and the
> next block repeats those bytes (read, not run).
> * *An empty fileTable block list* (an hsync at offset 0, then flushed
> writes). The file is still closed empty. No acknowledged byte is involved
> (read, not run).
> Tests: a new unit class {{TestLeaseRecoveryClientDNHandler}} (six tests, mock
> adapter) and two parameterized tests in {{TestLeaseRecovery}},
> {{testRecoveryWithOpenKeyTwoBlocksAheadOfFileTable}} and
> {{testForceRecoveryKeepsHsyncedPrefixWhenEarlierBlockIsNotFinalized}}, all
> failing without the change. With it they pass, as do the whole
> {{TestLeaseRecovery}} class (19 tests) and the lease recovery cases of
> {{TestSecureOzoneRpcClient}}, checkstyle is clean, and none of the four
> reproductions passes any more.
> Found by TLA+ model checking and code review of the lease recovery paths for
> hsynced files under HDDS-15926, on commit
> ea69b4a7d9abd040e5259dbcbb7c6ce52eb5d199. Checked against HDDS issues and
> apache/ozone pull requests for duplicates before filing. Related but not
> duplicates: HDDS-11550 (open, lease recovery fails with "Unable to find the
> block" for one file, no cause given, possibly the third shape), HDDS-10242
> (resolved, the open key one block ahead with a never written last block),
> HDDS-10632 (resolved, introduced the three cases), HDDS-11126 (open, asks for
> test coverage of this handler) and HDDS-16829 (the datanode side of a
> FinalizeBlock for a never written block). 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]
