HDDS-14246. Change fsync boundary for FilePerBlockStrategy to block level #9570
+51
−4
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Currently, datanode has an option to flush the write on chunk boundary (hdds.container.chunk.write.sync) which is disabled by default since it might affect the DN write throughput and latency. However, disabling this means that if the datanode machine is suddenly down (e.g. power failure, reaped by OOM killer), this might cause the file to have incomplete data even if PutBlock (write commit) is successful which violates our durability guarantee. Although PutBlock triggers FilePerBlockStrategy#finishWriteChunks which will trigger close (RandomAccessFile#close), the buffer cache might not be flushed yet since closing a file does not imply that the buffer cache for the file is flushed (see https://man7.org/linux/man-pages/man2/close.2.html). So there might be a chance where the user's key is committed, but the data do not exist in datanodes.
However, flushing for every WriteChunk might cause unnecessary overhead. We might need to consider calling FileChannel#force on PutBlock instead of WriteChunk since the data is only visible for users when PutBlock returns successfully (the data is committed) and for failure the client will try to replace the block (allocate another block). Therefore, we can guarantee that the after user successfully uploaded the key, the data has been persistently stored in the leader and at least one follower promise to flush the data (MAJORITY_COMMITTED).
This might still affect the write throughput and latency due to waiting for the buffer cached to be flushed to persistent storage (ssd or disk), but will increase our data durability guarantee (which should be our priority). Flushing the buffer cache might also reduce the memory usage of datanode.
In the future, we should consider enabling hdds.container.chunk.write.sync by default.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-14246
How was this patch tested?
CI when sync is enabled (https://github.com/ivandika3/ozone/actions/runs/20535392231)