peterxcli opened a new pull request, #11414: URL: https://github.com/apache/ozone/pull/11414
## What changes were proposed in this pull request? [HDDS-11014](https://issues.apache.org/jira/browse/HDDS-11014) made `BlockManagerImpl#finalizeBlock` move the last chunk of a block written with an incremental chunk list from the `last_chunk_info` table into the `block_data` table. But the merge is added to the batch after the batch has already been committed. Nothing commits the batch again, so `RDBBatchOperation#close` throws the merge away. Every such finalize logs: ``` WARN db.RDBBatchOperation: discarding changes Batch-2: block_data batchSize=94 B, discarded: 0 (0 B), #put=1, #del=0 WARN db.RDBBatchOperation: discarding changes Batch-2: last_chunk_info batchSize=1 B, discarded: 0 (0 B), #put=0, #del=1 ``` Reads still return the whole block, because `getBlock` merges the `last_chunk_info` row when it reads the block. But code that reads `block_data` rows directly never sees the last chunk. The container scanner does not check it, and the container merkle tree leaves it out. Reconciliation can also break the block: when it repairs a chunk of such a block, it writes the full chunk list back to `block_data` and leaves the `last_chunk_info` row in place. After that, reads of the block on that datanode fail with `IllegalStateException: chunk offset does not match`. Only clients with `ozone.client.incremental.chunk.list=true` (off by default) are affected. This PR adds the merge to the batch before the commit, so the finalize marker and the merge are written together. ## What is the link to the Apache JIRA https://issues.apache.org/jira/browse/HDDS-16730 ## How was this patch tested? - New `TestBlockManagerImpl#testFinalizeBlockMergesLastChunk`. It writes a block with an incremental chunk list (two hsyncs), finalizes it, and checks the RocksDB rows directly. Without the fix it fails because the `last_chunk_info` row is still there. - Ran `TestBlockManagerImpl`, `TestKeyValueHandler`, `TestKeyValueHandlerWithUnhealthyContainer`, `TestContainerReconciliationWithMockDatanodes`, `TestReconcileChunksPerBlockHoleBcsId`, `TestDatanodeUpgradeToHBaseSupport` and `TestFinalizeBlock` locally. - Ran `TestLeaseRecovery` locally with `ozone.client.incremental.chunk.list=true` added for the run (not part of this PR). Its 42 finalize calls logged 84 `discarding changes` warnings without the fix and none with it. The test results were the same with and without the fix (`testGetCommittedBlockLengthTimeout` fails on my machine either way). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
