Siyao Meng created HDDS-17676:
---------------------------------
Summary: FinalizeBlock lowers the stored block bcsId to the
request's when the block has no last_chunk_info row
Key: HDDS-17676
URL: https://issues.apache.org/jira/browse/HDDS-17676
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments: CR-8-finalize-block-keeps-stored-bcsid.patch,
TestBugCR8FinalizeBlockBcsId.java
h3. Mechanism
With the incremental chunk list, {{BlockManagerImpl.finalizeBlock}} merges the
last chunk into the block through {{mergeLastChunkForBlockFinalization}}. It
passes {{new BlockData(blockId)}}, built from the request, to {{putBlockByID}}
with end of block set, which ends in
{{DatanodeStoreWithIncrementalChunkList.moveLastChunkToBlockData}}:
* If a {{last_chunk_info}} row exists, {{reconcilePartialChunks}} sets the
bcsId from the row, which is correct.
* If there is no row, case 2.2 copies the bcsId of the request onto the stored
{{block_data}} row.
There is no row after a PutBlock whose last chunk is full. Lease recovery sends
FinalizeBlock with the {{BlockID}} that OM holds, which can carry an older
bcsId than the last PutBlock (the {{KeyValueHandler.handleFinalizeBlock}} check
only rejects a higher one). FinalizeBlock goes through Ratis, so the stored
block bcsId goes down on every replica. The container BCSID is not changed.
Introduced on master by HDDS-16730, which made the merge persistent. Before it,
{{finalizeBlock}} committed its batch before the merge, so no release is
affected.
h3. Trigger
# HBase enhancements, hsync and {{ozone.client.incremental.chunk.list}} are
enabled (all default to false).
# A writer hsyncs, then writes until the current chunk is full and flushes, so
the last PutBlock leaves no {{last_chunk_info}} row.
# The writer stops and {{recoverLease}} finalizes the block with the bcsId from
OM.
h3. Impact
No observable effect found: readers and lease recovery use the bcsId that OM
holds, which after the recovery equals the lowered stored value, and
reconciliation takes the larger bcsId of two replicas. A {{BCSID_MISMATCH}}
would need a GetBlock with the pre finalize bcsId, which no client path sends
today. This is filed as a follow up to HDDS-16730, which is not released yet:
fixing it before that change ships means no release ever saves a lowered block
bcsId on disk.
h3. Reproduction
Fails on the unfixed code, deterministic, unmodified source.
[^TestBugCR8FinalizeBlockBcsId.java] ({{hadoop-ozone/integration-test}}) starts
3 datanodes, writes through ofs with the incremental chunk list, hsyncs at one
chunk + 1024 bytes, fills the second chunk and flushes, recovers the lease,
reads the file back and then expects each replica's stored block bcsId to be
unchanged by the recovery. It fails on all 3 replicas: the stored bcsId goes
from 8 to 0, the value OM held for the block. The read back before that
assertion passes. With the patch it passes, and {{TestLeaseRecovery}} passes as
well.
h3. Patch
[^CR-8-finalize-block-keeps-stored-bcsid.patch], against
ec5ed5f7ea0e4b406aee53aae4906616819cba2b. It also applies to master at
7294fc2da18.
{{mergeLastChunkForBlockFinalization}} already reads the stored block; the
merge input now takes that block's bcsId instead of the request's. When a
{{last_chunk_info}} row exists the row's bcsId still wins, so only the case
without a row changes. {{moveLastChunkToBlockData}} is left as is because it
also serves every incremental PutBlock.
Covered by {{testFinalizeBlockKeepsBcsIdWhenLastChunkIsFull}} in the existing
{{TestBlockManagerImpl}}, next to {{testFinalizeBlockMergesLastChunk}} which
covers the case with a row. Without the change it fails with {{expected: <2>
but was: <0>}}. With it {{TestBlockManagerImpl}}, {{TestKeyValueHandler}},
{{TestKeyValueContainerCheck}} and
{{TestContainerReconciliationWithMockDatanodes}} (196 tests) pass.
Found by code review of the datanode container state machine apply and block
commit paths, as part of the TLA+ verification effort under HDDS-15926, on
commit ec5ed5f7ea0e4b406aee53aae4906616819cba2b. 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
(v10.3.26#10030026)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
