HDDS-14159. Have an option to read only Key in ManagedRawSSTFileIterator#9485
Merged
swamirishi merged 8 commits intoapache:masterfrom Dec 13, 2025
Merged
HDDS-14159. Have an option to read only Key in ManagedRawSSTFileIterator#9485swamirishi merged 8 commits intoapache:masterfrom
swamirishi merged 8 commits intoapache:masterfrom
Conversation
Change-Id: Iaaf2919c4e8a044ad0f3fc8714758e794e093480
Change-Id: I173a8753d6e2dc7a039bbc6c8872f8026bd5ac6a
Change-Id: I26e22a57c52159431e401174f5bdffb195af3a00
Change-Id: Ifb66a66064e5a5efb62c1dde87208d2c0c0ddbad
Comment on lines
27
to
47
| NEITHER, | ||
| /** | ||
| * Read key only. | ||
| */ | ||
| KEY_ONLY, | ||
| /** | ||
| * Read value only. | ||
| */ | ||
| VALUE_ONLY, | ||
| /** | ||
| * Read both key and value. | ||
| */ | ||
| KEY_AND_VALUE; | ||
|
|
||
| public boolean readKey() { | ||
| return (this.ordinal() & KEY_ONLY.ordinal()) != 0; | ||
| } | ||
|
|
||
| public boolean readValue() { | ||
| return (this.ordinal() & VALUE_ONLY.ordinal()) != 0; | ||
| } |
Contributor
There was a problem hiding this comment.
Thanks for the patch @swamirishi
nit: set the bit mask explicitly instead of relying on the ordinal just to avoid any issues if the enum constants are reordered/added in the future.
Suggested change
| NEITHER, | |
| /** | |
| * Read key only. | |
| */ | |
| KEY_ONLY, | |
| /** | |
| * Read value only. | |
| */ | |
| VALUE_ONLY, | |
| /** | |
| * Read both key and value. | |
| */ | |
| KEY_AND_VALUE; | |
| public boolean readKey() { | |
| return (this.ordinal() & KEY_ONLY.ordinal()) != 0; | |
| } | |
| public boolean readValue() { | |
| return (this.ordinal() & VALUE_ONLY.ordinal()) != 0; | |
| } | |
| NEITHER(0), | |
| /** | |
| * Read key only. | |
| */ | |
| KEY_ONLY(1), | |
| /** | |
| * Read value only. | |
| */ | |
| VALUE_ONLY(2), | |
| /** | |
| * Read both key and value. | |
| */ | |
| KEY_AND_VALUE(3); | |
| public final int mask; | |
| IteratorType(int mask) { | |
| this.mask = mask; | |
| } | |
| public boolean readKey() { | |
| return (this.mask & KEY_ONLY.mask) != 0; | |
| } | |
| public boolean readValue() { | |
| return (this.mask & VALUE_ONLY.mask) != 0; | |
| } |
Contributor
Author
There was a problem hiding this comment.
nice thanks for pointing out. I just moved this code from internal TableIterator. Let me make this change
jojochuang
approved these changes
Dec 12, 2025
| protected ClosableIterator<String> getKeyIteratorForFile(String file) { | ||
| return new ManagedRawSstFileIterator(file, options, lowerBoundSlice, upperBoundSlice, | ||
| keyValue -> StringUtils.bytes2String(keyValue.getKey())); | ||
| keyValue -> StringCodec.get().fromPersistedFormat(keyValue.getKey()), KEY_ONLY); |
Contributor
There was a problem hiding this comment.
SstFileSetReader reads just the key.
…instead of ordinal Change-Id: I4bdfdc6da9990a1518dc900f8297d92700de0022
Change-Id: Ic7033f081410d196409cf054b1777a1e78901cd2
Change-Id: I2eb955c8fc0a06219465b424bffd5e426e0b70ab # Conflicts: # hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdb/util/SstFileSetReader.java
Change-Id: Ia60bc037c0fb29f9fd92b44a486977e48b7d19d9
Contributor
Author
|
thank you @jojochuang and @SaketaChalamchala for reviewing the the patch |
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
3 participants
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 for SnapshotDiff and defrag we only rely on Key and never on the value of the sst file. We should make the value read optional to avoid unnecessary memory allocation.
This is to enable SSTFileSetReader to just read the key where the value is not required. This is to avoid unnecessary IO while reading the delta SSTFiles which only requires key to compute diff b/w snapshots both for defrag and snapshot diff.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-14159
How was this patch tested?
Added unit tests