HBASE-29890 WAL tailing reader should resume partial cell reads instead of resetting compression#7741
Open
sidkhillon wants to merge 4 commits intoapache:masterfrom
Open
HBASE-29890 WAL tailing reader should resume partial cell reads instead of resetting compression#7741sidkhillon wants to merge 4 commits intoapache:masterfrom
sidkhillon wants to merge 4 commits intoapache:masterfrom
Conversation
Contributor
Author
|
cc @Apache9, I would really appreciate any feedback you have on this PR. Thank you! |
added 3 commits
February 13, 2026 11:33
9d57c06 to
874e4ff
Compare
Apache9
reviewed
Feb 14, 2026
|
|
||
| private byte[] getDeferredOrDictEntry(short dictIdx) { | ||
| if (deferAdditions) { | ||
| int deferredIdx = dictIdx - deferredBaseIndex; |
Contributor
There was a problem hiding this comment.
Is this algorithm always right?
The Dictionary interface does not guarantee that the newly added entry will have the current size as its index, and in the implementation, LRUDictionary may move entries when it is being accessed...
Contributor
Author
There was a problem hiding this comment.
That's a good point. I have created an UndoableLRUDictionary that will let us rollback changes to the dictionary, and I've added many scenarios to test it out as unit tests. Please let me know how that looks.
Contributor
Author
|
@Apache9 I've addressed the PR feedback you left, and I would really appreciate another review when you have the chance. Thank you! |
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
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.
When the WAL tailing reader hits EOF mid-cell during WAL compression, it currently returns EOF_AND_RESET_COMPRESSION, which forces the reader to re-read the entire WAL file from the beginning to rebuild dictionary state. This is an O(n) operation that becomes increasingly expensive as the WAL grows.
The root cause is that the CompressedKvDecoder eagerly adds entries to the compression dictionaries (ROW, FAMILY, QUALIFIER, and tag dictionaries) as it reads each field of a cell. If an IOException occurs partway through reading a cell, the dictionaries are left in a partially-updated state that no longer matches the actual stream position. The reader has no choice but to throw away the entire compression context and start over.
Proposed Fix is to defer dictionary additions until a cell is fully parsed:
With deferred additions, hitting EOF mid-cell leaves the dictionaries in the state they were after the last fully-read cell. This means the reader can return EOF_AND_RESET (a cheap seek to the saved position) instead of EOF_AND_RESET_COMPRESSION, and resume reading from where it left off once the file grows.