You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "len548 (via GitHub)" <gi...@apache.org> on 2026/10/02 08:48:29 UTC
[PR] HDDS-16191. Extract PendingFlush state object from BlockDataStreamOutput [ozone]
len548 opened a new pull request, #11384:
URL: https://github.com/apache/ozone/pull/11384
## What changes were proposed in this pull request?
Move in-flight flush state out of `BlockDataStreamOutput` into a single-owner `PendingFlush` object.
- Add package-private `PendingFlush` holding buffers pending putBlock, chunk `CompletableFuture`s, and flushed/written length counters.
- Replace scattered fields (`buffersForPutBlock`, class-level chunk `futures`, `totalDataFlushedLength`, `writtenDataLength`) with one `PendingFlush` instance.
- At flush boundaries, `seal()` produces a `FlushBatch` passed to `executePutBlock(FlushBatch, close, force)` so chunk futures and buffer lists stay paired through putBlock.
- Keep the public 2-arg `executePutBlock(boolean, boolean)` for `KeyDataStreamOutput.handleException`; return early when there is no pending batch (after a prior seal), matching master when `buffersForPutBlock` was already null.
- `handleFlush` with unflushed data uses `seal()` + 3-arg directly (same as `doFlushIfNeeded`), avoiding a second seal via the 2-arg overload.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-16191
## How was this patch tested?
Current `TestBlockDataStreamOutput` and `TestKeyDataStreamOutput` are enough as testing.
CI pass: https://github.com/len548/ozone/actions/runs/36838851713
Generated-by: Cursor (Auto)
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16191. Extract PendingFlush state object from BlockDataStreamOutput [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #11384:
URL: https://github.com/apache/ozone/pull/11384#issuecomment-6250987632
Converted to draft only to "stack" CI runs: resolving conflicts will first trigger CI only in fork, then after successful run it can be marked "ready for review".
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16191. Extract PendingFlush state object from BlockDataStreamOutput [ozone]
Posted by "len548 (via GitHub)" <gi...@apache.org>.
len548 commented on code in PR #11384:
URL: https://github.com/apache/ozone/pull/11384#discussion_r4271398890
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockDataStreamOutput.java:
##########
@@ -438,7 +435,7 @@ public void executePutBlock(boolean close,
if (e != null || reply == null || !reply.isSuccess()) {
LOG.warn("Failed executePutBlockClose, reply=" + reply, e);
try {
- executePutBlock(true, false);
+ executePutBlock(batch, true, false);
Review Comment:
Thank you for the review. Updated.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16191. Extract PendingFlush state object from BlockDataStreamOutput [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #11384:
URL: https://github.com/apache/ozone/pull/11384#discussion_r4268052241
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockDataStreamOutput.java:
##########
@@ -438,7 +435,7 @@ public void executePutBlock(boolean close,
if (e != null || reply == null || !reply.isSuccess()) {
LOG.warn("Failed executePutBlockClose, reply=" + reply, e);
try {
- executePutBlock(true, false);
+ executePutBlock(batch, true, false);
Review Comment:
Please keep the two-argument call here instead of replaying `batch`. Ratis 3.3.1 closes the stream when CLOSE is submitted, so retrying this close against the same `out` immediately returns `AlreadyClosedException` and invokes this callback recursively. Each unwinding invocation also sends PutBlock and registers the same buffers at another commit index.
With one 100-byte chunk and one close timeout, the head reproduced hundreds of PutBlocks, 44,000 acknowledged bytes, and `key.close()` failing with `IllegalArgumentException`. `handleException` uses the inflated acknowledgment count as the block position, then `commitKey` rejects its mismatch with the key offset. The base and this one-line change both acknowledge 100 bytes and commit the key once.
```diff
--- a/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockDataStreamOutput.java
+++ b/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockDataStreamOutput.java
@@ -435,7 +435,7 @@
if (e != null || reply == null || !reply.isSuccess()) {
LOG.warn("Failed executePutBlockClose, reply=" + reply, e);
try {
- executePutBlock(batch, true, false);
+ executePutBlock(true, false);
} catch (IOException ex) {
throw new CompletionException(ex);
}
```
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16191. Extract PendingFlush state object from BlockDataStreamOutput [ozone]
Posted by "len548 (via GitHub)" <gi...@apache.org>.
len548 commented on PR #11384:
URL: https://github.com/apache/ozone/pull/11384#issuecomment-6172347374
@yandrey321 no, this is a structural refactor, not a behavioral change so I didn't run any perf benchmarks.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16191. Extract PendingFlush state object from BlockDataStreamOutput [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on PR #11384:
URL: https://github.com/apache/ozone/pull/11384#issuecomment-6170650450
Did you run any perf bechmarks with these changes?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16191. Extract PendingFlush state object from BlockDataStreamOutput [ozone]
Posted by "len548 (via GitHub)" <gi...@apache.org>.
len548 commented on PR #11384:
URL: https://github.com/apache/ozone/pull/11384#issuecomment-6270071915
@peterxcli can you review this please?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org