Introduce raw fwd index version V5 containing implicit num doc length, improving space efficiency #14105
Conversation
This file contains 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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #14105 +/- ##
============================================
+ Coverage 61.75% 63.76% +2.01%
- Complexity 207 1535 +1328
============================================
Files 2436 2626 +190
Lines 133233 144646 +11413
Branches 20636 22136 +1500
============================================
+ Hits 82274 92239 +9965
- Misses 44911 45596 +685
- Partials 6048 6811 +763
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
…wIndexCreatorV2Test.java
…tiValueFixedByteRawIndexCreatorV2Test
jackluo923
changed the title
Jackie-Jiang
added
enhancement
feature
release-notes
Jackie-Jiang
reviewed
Oct 8, 2024
| /** | ||
| Same as MultiValueFixedByteRawIndexCreator, but without storing the number of elements for each row. | ||
| */ | ||
| public class MultiValueFixedByteRawIndexCreatorV2 extends MultiValueFixedByteRawIndexCreator { |
There was a problem hiding this comment.
We don't want to add a new creator because creator is used to handle creation of different version of forward index. Instead, we want to add a new raw index version v5 for this new format
| /** | ||
| Same as FixedByteChunkMVForwardIndexReader, but the number of elements for each row is inferred | ||
| */ | ||
| public final class FixedByteChunkMVForwardIndexReaderV2 extends FixedByteChunkMVForwardIndexReader { |
There was a problem hiding this comment.
Let's call it V5 to be consistent with index version
.../pinot/segment/local/segment/index/readers/forward/FixedByteChunkMVForwardIndexReaderV2.java
Outdated
Show resolved
Hide resolved
…orward index creator version.
jackluo923
changed the title
jackluo923
changed the title
Jackie-Jiang
approved these changes
Oct 15, 2024
|
|
||
|
|
||
| /** | ||
| * Forward index writer that extends {@link VarByteChunkForwardIndexWriterV4} with the only difference being the |
There was a problem hiding this comment.
This is not the only difference. Let's also document the value format difference
...ache/pinot/segment/local/segment/index/readers/forward/VarByteChunkForwardIndexReaderV4.java
Show resolved
Hide resolved
…wIndexCreatorV2Test.java
…tiValueFixedByteRawIndexCreatorV2Test
…orward index creator version.
…dex' into master-improved-MV-fixed-byte-index
… reader to fetch version number.
| @@ -81,6 +80,12 @@ public VarByteChunkForwardIndexReaderV4(PinotDataBuffer dataBuffer, FieldSpec.Da | |||
| _isSingleValue = isSingleValue; | |||
| } | |||
|
|
|||
| public void validateIndexVersion(PinotDataBuffer dataBuffer) { | |||
There was a problem hiding this comment.
I meant we can add a getVersion() method into this class, and override it in v5 reader
Jackie-Jiang
approved these changes
Oct 16, 2024
| @@ -76,11 +76,13 @@ | |||
| public class VarByteChunkForwardIndexWriterV4 implements VarByteChunkWriter { | |||
| public static final int VERSION = 4; | |||
|
|
|||
| private static final Logger LOGGER = LoggerFactory.getLogger(VarByteChunkForwardIndexWriterV4.class); | |||
| // Use the run-time concrete class to retrieve the logger | |||
| protected final Logger _logger = LoggerFactory.getLogger(this.getClass()); | |||
There was a problem hiding this comment.
Suggested change
| protected final Logger _logger = LoggerFactory.getLogger(this.getClass()); | |
| protected final Logger _logger = LoggerFactory.getLogger(getClass()); |
...ain/java/org/apache/pinot/segment/local/io/writer/impl/VarByteChunkForwardIndexWriterV4.java
Outdated
Show resolved
Hide resolved
...ain/java/org/apache/pinot/segment/local/io/writer/impl/VarByteChunkForwardIndexWriterV4.java
Outdated
Show resolved
Hide resolved
...ache/pinot/segment/local/segment/index/readers/forward/VarByteChunkForwardIndexReaderV4.java
Show resolved
Hide resolved
deemoliu
approved these changes
Oct 17, 2024
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.
The data layout of the multi-value fixed byte raw forward index can be optimized to enhance storage efficiency.
Consider the following multi-value document as an example:
[int(1), int(2), int(3)]. The current binary data layout inMVFixedBytesRawFwdIndexis as follows:0x00000010 0x00000003 0x00000001 0x00000002 0x00000003.0x00000010is an integer representing the total payload length of the byte array containing the multi-value document content, which in this case is 16 bytes.0x00000003is an integer explicitly representing the number of elements in the multi-value document (i.e., 3).0x00000001 0x00000002 0x00000003are 3 integers representing the 3 values of the multi-value document: 1, 2, and 3.In Pinot, the fixed byte raw forward index can only contain one specific fixed-length data type:
int,long,float, ordouble. Rather than explicitly specifying the number of elements for each document using an integer, this value can be omitted and instead inferred implicitly using the following calculation:If the forward index uses the passthrough chunk compression type (i.e., no compression), we can save 4 bytes per document by omitting the explicit element count. This results in the following savings:
0elements, we save50%.1element, we save33%.2elements, we save25%.For forward indexes that leverage compression to reduce data size, the savings can be even more significant in some scenarios. This PR includes a unit test,
VarByteChunkV5Test#validateCompressionRatioIncrease, which demonstrates this. In particular, we usedZStandardas the chunk compressor and inserted 1 million short multi-value (MV) documents, where the length follows a Gaussian distribution. In this experiment, the values of each integer in the MV documents were also somewhat repetitive. Under these conditions, we observed 50%+ reduction in on-disk file size compared to V4 fwd index writer versionThis PR introduces the implicit length optimization via
VarByteChunkForwardIndexWriterV5on the write path, alongsideVarByteChunkForwardIndexReaderV5on the read path.MultiValueFixedByteRawIndexCreatorandForwardIndexReaderFactoryare also modified accordingly to use the new index writer and reader when the index version is set to 5 or greater. After this PR is merged, other composite forward indexes such as theCLPForwardIndexCreatorV1forward index can leverage these new classes to significantly improve the overall compression ratio.