peterxcli opened a new pull request, #11678: URL: https://github.com/apache/ozone/pull/11678
## What changes were proposed in this pull request? With Ratis streaming, the datanode keeps the last 1 MiB or more of each stream in memory (the `Buffers` of `KeyValueStreamDataChannel`). It needs these bytes at close, because the end of the stream may be the PutBlock request that the client appends. By default (`ozone.client.datastream.putblock.without.raft.enabled=false`), `hsync()` commits its PutBlock through Raft, and nothing writes the buffered bytes to the block file first. HDDS-16746 fixed this only for a PutBlock sent as a stream command. So when `hsync()` returns, every replica has committed a block length whose last 1 MiB or more is not in its block file. For example, after writing 3 MiB + 1000 bytes and calling `hsync()`, each block file has 2 MiB: - A reader that opens the file fails with `Unexpected read size. expected: 262144, actual: -1`. - If the writer fails before it closes the stream, the bytes never reach the block file. Lease recovery then closes the file at the hsync'ed length, and the file stays unreadable. With this change: - `KeyValueHandler.handlePutBlock` asks the chunk manager to write the buffered data of the block's open stream, up to the length that the PutBlock commits, before it commits the PutBlock. - `FilePerBlockStrategy` keeps the open stream channels by container and local ID. A channel removes itself when it is closed or cleaned up. - `KeyValueStreamDataChannel.drainBuffers(long)` writes whole buffers until the block file has the given length, and stops there. At close the client sends the PutBlock request at the end of the stream and also a PutBlock through Raft, so writing all the buffers could put that request into the block file. The client sends the request in its own packets after the data, so writing whole buffers up to the committed length is safe. - The Raft PutBlock is applied on a different thread than the stream writes, so the methods that use the buffers are synchronized. The PutBlock at the end of the stream is committed without holding the lock. The drain is a buffered write without fsync, which is what an ordinary hsync gets with the default `hdds.container.chunk.write.sync=false`. ## What is the link to the Apache JIRA https://issues.apache.org/jira/browse/HDDS-16822 ## How was this patch tested? - `TestContainerStateMachineStream#testReadAfterHsync` is the existing `testReadAfterHsyncWithPutBlockWithoutRaft`, now run in both PutBlock modes and with 1000 bytes and 3 MiB + 1000 bytes. It writes, calls `hsync()` and reads the key from a second client. The Raft cases failed before this change. - New `TestLeaseRecovery#testRecoveryAfterStreamWriterFailure` calls `hsync()`, closes the writer's connection to the datanodes without closing the stream, recovers the lease from a second client and reads the file. The Raft case failed before this change. - New unit tests `TestKeyValueStreamDataChannel#testDrainBuffersUpToLength`, `#testDrainBuffersAfterCleanUp` and `TestFilePerBlockStrategy#testDrainStreamBuffers`. - Locally, these also pass: `TestBlockDataStreamOutput` (integration and unit), `TestHSync`, `TestOzoneFileSystemWithStreaming`, and the container-service unit tests for the stream channel, the chunk managers, `KeyValueHandler` and `ContainerStateMachine`. Checkstyle is clean. -- 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]
