amaliujia commented on code in PR #11384:
URL: https://github.com/apache/ozone/pull/11384#discussion_r4456285617
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockDataStreamOutput.java:
##########
@@ -433,19 +413,36 @@ public void watchForCommit(boolean bufferFull) throws
IOException {
* @param close whether putBlock is happening as part of closing the stream
*/
public void executePutBlock(boolean sync, boolean close) throws IOException {
+ if (!pendingFlush.hasBuffersForPutBlock()) {
+ if (!close && !(sync && putBlockWithoutRaft)) {
+ return;
+ }
+ executePutBlock(null, sync, close);
+ return;
+ }
+ executePutBlock(pendingFlush.seal(), sync, close);
+ }
Review Comment:
I really had a hard time to understand this code and didn't find an
equivalence in the head branch. From the readability perspective, this piece is
not good because the code becomes hard to read than before.
Meanwhile it seems that we need this 2-arguments executePutBlock because of
`KeyDataStreamOutput`.
I am wondering if the following would help to eliminate the need for the
3-arguments executePutBlock:
1. We separate parameters as `block level` and `flush level`.
buffersForPutBlock, chunk futures are flush level. writtenDataLength,
totalDataFlushedLength are block level.
2. Flush level parameters move to PendingFlush, block level parameters still
stay.
3. Then PendingFlush is a true per flush object, we no longer need to make
it a `Final`, and we do not need `seal()`.
4. We still just keep the previous 2-arguments executePutBlock as
```
public void executePutBlock(boolean sync, boolean close) throws IOException {
checkOpen();
final PendingFlush flush = pendingFlush;
pendingFlush = new PendingFlush();
final List<StreamBuffer> byteBufferList = flush.getBuffers();
flush.awaitChunks();
// ... rest unchanged, using byteBufferList
}
```
Basically now because PendingFlush only controls per flush state, we can
consume it and then drop it, which replace `seal()` and FlushBatch.
PendingFlush would be the `FlushBatch`.
It is also easy to check if we are mirroring the head branch behavior when
only keep the 2-arguments executeBlock.
--
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]
