[
https://issues.apache.org/jira/browse/HDDS-17678?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17678:
------------------------------
Attachment: MC-1-update-bcsid-map-for-piggybacked-putblock.patch
> PutBlock piggybacked on WriteChunk does not update the BCSID saved in the
> Ratis snapshot, so the restart check misses its loss
> ------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17678
> URL: https://issues.apache.org/jira/browse/HDDS-17678
> Project: Apache Ozone
> Issue Type: Bug
> Affects Versions: 2.0.0, 2.1.0, 2.2.0, 2.1.1, 2.1.2, 2.2.1, 2.2.2
> Reporter: Siyao Meng
> Priority: Major
> Attachments: MC-1-update-bcsid-map-for-piggybacked-putblock.patch,
> TestBugMC1PiggybackedHsyncBcsidRestart.java
>
>
> h3. Mechanism
> With {{ozone.client.stream.putblock.piggybacking}}, the client sends the
> block metadata inside the WriteChunk request for the last partial chunk of a
> flush, hsync or close, and {{KeyValueHandler.handleWriteChunk}} commits the
> block when the entry is applied (COMMIT_DATA stage). The commit goes through
> {{BlockManagerImpl.persistPutBlock}} like a plain PutBlock and advances the
> container BCSID through an unsynced RocksDB batch.
> {{HddsDispatcher.updateBCSID}} only updates {{container2BCSIDMap}} for
> {{PutBlock}} and {{PutSmallFile}}. The piggybacked commit is a
> {{WriteChunk}}, so the map keeps the BCSID of the last plain PutBlock (or 0).
> {{ContainerStateMachine.takeSnapshot}} persists only that map, and on restart
> {{ContainerSet.buildMissingContainerSetAndValidate}} marks a container
> UNHEALTHY only if its DB BCSID is below the map value. The check added by
> HDDS-1843 therefore cannot detect that a piggybacked commit covered by the
> snapshot was lost.
> h3. Trigger
> # A client sets {{ozone.client.hbase.enhancements.allowed}} and
> {{ozone.client.stream.putblock.piggybacking}} (both default false). The
> datanode accepts a piggybacked WriteChunk regardless of server configuration.
> # The client writes a key on a Ratis pipeline and flushes, hsyncs or closes
> it with a partial last chunk, so the PutBlock is piggybacked. The piggybacked
> commit is applied on a datanode; no plain PutBlock follows on that container
> on that datanode.
> # A Ratis snapshot is taken on that datanode after the apply.
> # The datanode loses the unsynced RocksDB WAL tail (power loss or OS crash)
> while the snapshot file, which is forced to disk, survives.
> # On restart the DB BCSID is not below the snapshot value, the check passes,
> and log replay starts after the snapshot index, so the commit is not
> reapplied.
> h3. Impact
> The replica stays OPEN (and later CLOSED) without the block data the client
> was told is durable after hsync or close, and it is not marked UNHEALTHY. The
> same loss with piggybacking off marks the replica UNHEALTHY at restart, as
> intended by HDDS-1843. The other replicas still hold the data, so the visible
> effect is lost redundancy that nothing reports. Possible downstream effects,
> not verified: SCM counting the CLOSED replica as healthy, and a GetBlock with
> an older BCSID returning the shorter block from this replica.
> h3. Reproduction
> Fails on the unfixed code, deterministic, unmodified source.
> [^TestBugMC1PiggybackedHsyncBcsidRestart.java]
> ({{hadoop-ozone/integration-test}}) starts 3 datanodes, writes and hsyncs a
> key on RATIS/THREE, resets the container BCSID in one replica's container DB
> to 0 (the lost write injection {{TestValidateBCSIDOnRestart}} uses), and
> restarts that datanode, whose stop takes a snapshot at the last applied
> index. It runs with piggybacking on and off and expects the replica to be
> UNHEALTHY after the restart. With piggybacking off it passes. With
> piggybacking on it fails with {{expected: UNHEALTHY but was: OPEN}}. The same
> test with the key closed instead of hsynced (not attached) fails the same
> way, which confirms the close path. A real power loss was not run.
> h3. Patch
> [^MC-1-update-bcsid-map-for-piggybacked-putblock.patch], against
> ec5ed5f7ea0e4b406aee53aae4906616819cba2b. It also applies to master at
> 7294fc2da18.
> {{HddsDispatcher.updateBCSID}} also updates {{container2BCSIDMap}} for a
> successful WriteChunk that carries a block in the COMMIT_DATA stage, which is
> where the piggybacked block is committed. The map then holds the same value
> as after a plain PutBlock and the existing restart check applies. WriteChunk
> without a block and the WRITE_DATA stage are unchanged.
> Covered by {{testPiggybackedPutBlockUpdatesContainer2BCSIDMap}} in the
> existing {{TestHddsDispatcher}}, which fails without the change: the map
> still holds 0 instead of 2 ({{[1L=0L (expected: 2L)]}}). With it
> {{TestHddsDispatcher}}, {{TestKeyValueHandler}},
> {{TestContainerStateMachineLeader}}, {{TestContainerStateMachineFollower}},
> {{TestBlockManagerImpl}} (156 tests), {{TestValidateBCSIDOnRestart}} and
> {{TestHSync}} (43 tests) pass, and both cases of the reproduction pass.
> Found by TLA+ model checking and code review of the datanode container state
> machine apply and block commit paths 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]
