Siyao Meng created HDDS-16830:
---------------------------------
Summary: 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
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]
