HDDS-15723. Ignore writes after PutBlock with end-of-block flag - #10967
HDDS-15723. Ignore writes after PutBlock with end-of-block flag#10967prathmesh12-coder wants to merge 4 commits into
Conversation
# Conflicts: # hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/BlockManagerImpl.java
chihsuan
left a comment
chihsuan
left a comment
There was a problem hiding this comment.
Thanks for working on this! @prathmesh12-coder
I noticed that KeyValueContainerData.finalizedBlockSet seems to represent a similar state, but persists it and checks it earlier.
I wonder if we could reuse that mechanism for explicit EOF PutBlocks instead of maintaining a separate in-memory cache? What do you think?
I've also added a few inline comments. thanks!
| + "by a PutBlock with the end-of-block flag set. PutBlock is not " | ||
| + "idempotent", | ||
| data.getBlockID()); | ||
| return data.getSize(); |
There was a problem hiding this comment.
This skips the PutBlock but still reports success with its new BCS ID. The client may then record a BCS ID that the datanode never persisted, causing later reads to fail. Would it make sense to change this to throw exception with BLOCK_ALREADY_FINALIZED instead?
| // writes to the container. So pendingPutBlockCache is not needed. | ||
| // writes to the container. So pendingPutBlockCache and eofBlockCache are not needed. | ||
| this.pendingPutBlockCache = new HashSet<>(); | ||
| this.eofBlockCache = new HashSet<>(); |
There was a problem hiding this comment.
This state is lost when a replica restarts. A later Raft entry could then be skipped by replicas that retained the cache but persisted by the restarted replica. Could the EOF state be persisted and restored with the container?
| long localID = data.getLocalID(); | ||
|
|
||
| // PutBlock is not idempotent; ignore duplicate eof writes (HDDS-12007). | ||
| if (container.isBlockFinalizedByEof(localID)) { |
There was a problem hiding this comment.
This check may be too late for a piggybacked WriteChunk, sinceKeyValueHandler writes the chunk before calling putBlock.
The data may therefore already be changed when this check skips the metadata update. Could the finalized-state check happen before chunk processing as well?
2 participants
What changes were proposed in this pull request?
Currently, the datanode will finalize a block if it encounters PutBlock with eof flag set. There should only be one PutBlock with eof flag since PutBlock is not idempotent. Currently this is not caught which can cause issue like HDDS-12007 to not be detected.
Therefore, we need to assert that once we encounter block EOF, there will not be any more writes on that block.
We should throw exception (or ignore) if there are some writes after EOF. This change ignores them and logs a warning.
Added
eofBlockCacheon open/closing containers to track blocks finalized by an EOF PutBlock.In
BlockManagerImpl#persistPutBlock, ignored subsequent writes on those blocks.What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-15723
How was this patch tested?
Added unit tests for simple and incremental writes.
CI : https://github.com/prathmesh12-coder/ozone/actions/runs/31194497621