Skip to content

HDDS-17093. Sync block data and PutBlock before acknowledging a stream-committed PutBlock - #11767

Open
amaliujia wants to merge 8 commits into
apache:masterfrom
amaliujia:raftless_1
Open

amaliujia wants to merge 8 commits into
apache:masterfrom
amaliujia:raftless_1

Conversation

@amaliujia

@amaliujia amaliujia commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

A PutBlock committed through the data stream (the mid-stream STREAM_COMMAND or the close-time STREAM_LINK) has no Raft log entry to replay. Today neither the block file nor the RocksDB write-ahead log is synced before the datanode replies. When the data is written into OS page cache, it is already able to send back the acknowledgement to the client.

This might be fine for the default mode, but we can also make sure data stream writes work the hsync too: we can send back the acknowledgement after the data becomes durable when the client wants it.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-17093

How was this patch tested?

Unit test

@szetszwo szetszwo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amaliujia , thanks for working on this!

Please see the comments inlined and also https://issues.apache.org/jira/secure/attachment/13085970/11767_review.patch

"without sync. It takes effect only when hsync is enabled (ozone.fs.hsync.enabled) and the PutBlock " +
"is committed through the data stream (ozone.client.datastream.putblock.without.raft.enabled).",
tags = ConfigTag.CLIENT)
private boolean datastreamHsyncEnabled = false;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should not be configurable -- hsync should always wait.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure I guess we only need the sync parameter and let the code path set it in the hsync mode.

* @param sync whether the datanodes sync the data and the PutBlock to disk before replying;
* it applies only to a PutBlock committed in the middle of the stream without Raft
*/
private void executePutBlock(boolean close, boolean force, boolean sync) throws IOException {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With sync, it seems that force is not longer needed.

@amaliujia amaliujia Oct 29, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per my understanding, force means there is no new data since last PutBlock, while sync means a different thing.

I ended up with ussing buffersForPutBlock != null for the places that used to be !force.

+ ", expected " + Type.PutBlock, ContainerProtos.Result.MALFORMED_REQUEST);
}
if (request.getPutBlock().getSync()) {
channel.force(false);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be channel.force(request.getPutBlock().getSync())?

@amaliujia

Copy link
Copy Markdown
Contributor Author

@szetszwo can you take another look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants