You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "xiangfu0 (via GitHub)" <gi...@apache.org> on 2026/04/03 11:33:35 UTC
[PR] Add parameterized forward index compression codecs [pinot]
xiangfu0 opened a new pull request, #18097:
URL: https://github.com/apache/pinot/pull/18097
## Summary
- add a structured `CompressionCodecSpec` parser/model for `compressionCodec`, including `CODEC` and `CODEC(level)` syntax plus `ZSTD -> ZSTANDARD` canonicalization
- thread optional compression levels through raw/forward index writer creation for codecs that support them, while preserving legacy behavior for plain codec names
- persist the explicit canonical forward-index compression spec in segment metadata so level-only config changes can trigger rewrite detection during reload
- add parser, config serde, compression factory, and forward-index handler tests covering aliases, validation errors, and level-aware rewrites
## Supported behavior
Valid examples now include:
- `ZSTANDARD`
- `ZSTD`
- `ZSTANDARD(3)`
- `ZSTD(3)`
- `GZIP(6)`
- `LZ4(12)`
Levels are validated in Pinot for codecs that meaningfully support them:
- `ZSTANDARD`: `1..22`
- `GZIP`: `0..9`
- `LZ4`: `1..17`
Unsupported parameterized forms such as `SNAPPY(3)` and malformed specs such as `ZSTD()` are rejected with validation errors.
## Backward compatibility
- existing plain codec configs continue to work unchanged
- read-path format is unchanged; readers still rely on codec identity only
- plain `LZ4` keeps the existing fast-compressor behavior; only explicit `LZ4(level)` selects HC compression
## Validation
- `./mvnw -pl pinot-segment-spi,pinot-segment-local -am -Dcheckstyle.skip=true -Dtest=ForwardIndexConfigTest,ForwardIndexTypeTest,IndexLoadingConfigTest,TestCompression,ForwardIndexHandlerTest -Dsurefire.failIfNoSpecifiedTests=false test`
- `./mvnw -pl pinot-spi,pinot-common -am -Dcheckstyle.skip=true -Dtest=CompressionCodecSpecTest,TableConfigSerDeUtilsTest -Dsurefire.failIfNoSpecifiedTests=false test`
- `./mvnw -pl pinot-spi,pinot-segment-spi,pinot-common,pinot-segment-local spotless:apply -DskipTests`
- `./mvnw -pl pinot-spi,pinot-segment-spi,pinot-common,pinot-segment-local checkstyle:check -DskipTests`
- `./mvnw -pl pinot-spi,pinot-segment-spi,pinot-common,pinot-segment-local license:format -DskipTests`
- `./mvnw -pl pinot-spi,pinot-segment-spi,pinot-common,pinot-segment-local license:check -DskipTests`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add parameterized forward index compression codecs [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on PR #18097:
URL: https://github.com/apache/pinot/pull/18097#issuecomment-4222640523
Addressed the legacy-segment rewrite gap called out in review.
Changes in this push:
- rebased the branch onto current `master`
- made `ForwardIndexHandler` treat missing `forwardIndexCompressionCodec` metadata as an unknown legacy state and force a rewrite when the table config now specifies an explicit compression level
- tightened `ForwardIndexHandlerTest.testChangeCompressionLevelForSingleColumn()` so it first asserts the old segment has no persisted codec-spec metadata, then verifies the rewrite persists `LZ4(12)`
Focused validation run:
- `./mvnw -pl pinot-segment-local -am -Dcheckstyle.skip=true -Dtest=ForwardIndexHandlerTest -Dsurefire.failIfNoSpecifiedTests=false test`
- `spotless`, `checkstyle`, and `license` checks on `pinot-segment-local`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add parameterized forward index compression codecs [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #18097:
URL: https://github.com/apache/pinot/pull/18097#discussion_r3072572555
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/fwd/MultiValueFixedByteRawIndexCreator.java:
##########
@@ -41,67 +40,36 @@ public class MultiValueFixedByteRawIndexCreator implements ForwardIndexCreator {
private final VarByteChunkWriter _indexWriter;
private final DataType _valueType;
- /**
- * Create a var-byte raw index creator for the given column
- *
- * @param baseIndexDir Index directory
- * @param compressionType Type of compression to use
- * @param column Name of column to index
- * @param totalDocs Total number of documents to index
- * @param valueType Type of the values
- * @param deriveNumDocsPerChunk true if writer should auto-derive the number of rows per chunk
- * @param writerVersion writer format version
- * @param targetMaxChunkSizeBytes target max chunk size in bytes, applicable only for V4 or when
- * deriveNumDocsPerChunk is true
- */
- public MultiValueFixedByteRawIndexCreator(File baseIndexDir, ChunkCompressionType compressionType, String column,
- int totalDocs, DataType valueType, int maxNumberOfMultiValueElements, boolean deriveNumDocsPerChunk,
- int writerVersion, int targetMaxChunkSizeBytes, int targetDocsPerChunk)
- throws IOException {
- this(new File(baseIndexDir, column + Indexes.RAW_MV_FORWARD_INDEX_FILE_EXTENSION), compressionType, totalDocs,
- valueType, maxNumberOfMultiValueElements, deriveNumDocsPerChunk, writerVersion, targetMaxChunkSizeBytes,
- targetDocsPerChunk);
- }
-
- public MultiValueFixedByteRawIndexCreator(File indexFile, ChunkCompressionType compressionType, int totalDocs,
- DataType valueType, int maxNumberOfMultiValueElements, boolean deriveNumDocsPerChunk, int writerVersion)
- throws IOException {
- this(indexFile, compressionType, totalDocs, valueType, maxNumberOfMultiValueElements, deriveNumDocsPerChunk,
- writerVersion, ForwardIndexConfig.getDefaultTargetMaxChunkSizeBytes(),
- ForwardIndexConfig.getDefaultTargetDocsPerChunk());
- }
-
- public MultiValueFixedByteRawIndexCreator(File indexFile, ChunkCompressionType compressionType, int totalDocs,
- DataType valueType, int maxNumberOfMultiValueElements, boolean deriveNumDocsPerChunk, int writerVersion,
- int targetMaxChunkSizeBytes, int targetDocsPerChunk)
+ public MultiValueFixedByteRawIndexCreator(File indexFile, ChunkCompressionType compressionType,
Review Comment:
add comment
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add parameterized forward index compression codecs [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #18097:
URL: https://github.com/apache/pinot/pull/18097#discussion_r3070978119
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/io/writer/impl/BaseChunkForwardIndexWriter.java:
##########
@@ -88,11 +89,18 @@ public abstract class BaseChunkForwardIndexWriter implements Closeable {
protected BaseChunkForwardIndexWriter(File file, ChunkCompressionType compressionType, int totalDocs,
Review Comment:
remove this constructor, just need one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add parameterized forward index compression codecs [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #18097:
URL: https://github.com/apache/pinot/pull/18097#issuecomment-4183364072
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/18097?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `56.84211%` with `123 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 56.11%. Comparing base ([`0c6521a`](https://app.codecov.io/gh/apache/pinot/commit/0c6521a5ccdb26443e042c4bc2a6f4ec6a5e1a5c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`39e9169`](https://app.codecov.io/gh/apache/pinot/commit/39e9169bac0170b914dfcc1b1037095ace36c9e2?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/pinot/pull/18097?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...he/pinot/segment/spi/index/ForwardIndexConfig.java](https://app.codecov.io/gh/apache/pinot/pull/18097?src=pr&el=tree&filepath=pinot-segment-spi%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Fspi%2Findex%2FForwardIndexConfig.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2luZGV4L0ZvcndhcmRJbmRleENvbmZpZy5qYXZh) | 25.64% | [28 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/pinot/pull/18097?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...t/local/io/compression/ChunkCompressorFactory.java](https://app.codecov.io/gh/apache/pinot/pull/18097?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Fio%2Fcompression%2FChunkCompressorFactory.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pby9jb21wcmVzc2lvbi9DaHVua0NvbXByZXNzb3JGYWN0b3J5LmphdmE=) | 17.64% | [10 Missing and 4 partials :warning: ](https://app.codecov.io/gh/apache/pinot/pull/18097?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...t/spi/config/table/CompressionCodecSpecParser.java](https://app.codecov.io/gh/apache/pinot/pull/18097?src=pr&el=tree&filepath=pinot-spi%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fspi%2Fconfig%2Ftable%2FCompressionCodecSpecParser.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL0NvbXByZXNzaW9uQ29kZWNTcGVjUGFyc2VyLmphdmE=) | 63.88% | [7 Missing and 6 partials :warning: ](https://app.codecov.io/gh/apache/pinot/pull/18097?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...r/impl/fwd/MultiValueFixedByteRawIndexCreator.java](https://app.codecov.io/gh/apache/pinot/pull/18097?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Fsegment%2Fcreator%2Fimpl%2Ffwd%2FMultiValueFixedByteRawIndexCreator.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2NyZWF0b3IvaW1wbC9md2QvTXVsdGlWYWx1ZUZpeGVkQnl0ZVJhd0luZGV4Q3JlYXRvci5qYXZh) | 25.00% | [9 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/18097?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...tor/impl/fwd/MultiValueVarByteRawIndexCreator.java](https://app.codecov.io/gh/apache/pinot/pull/18097?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Fsegment%2Fcreator%2Fimpl%2Ffwd%2FMultiValueVarByteRawIndexCreator.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2NyZWF0b3IvaW1wbC9md2QvTXVsdGlWYWx1ZVZhckJ5dGVSYXdJbmRleENyZWF0b3IuamF2YQ==) | 22.22% | [7 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/18097?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...spi/config/table/CompressionCodecCapabilities.java](https://app.codecov.io/gh/apache/pinot/pull/18097?src=pr&el=tree&filepath=pinot-spi%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fspi%2Fconfig%2Ftable%2FCompressionCodecCapabilities.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL0NvbXByZXNzaW9uQ29kZWNDYXBhYmlsaXRpZXMuamF2YQ==) | 81.25% | [5 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/pinot/pull/18097?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...e/pinot/spi/config/table/CompressionCodecSpec.java](https://app.codecov.io/gh/apache/pinot/pull/18097?src=pr&el=tree&filepath=pinot-spi%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fspi%2Fconfig%2Ftable%2FCompressionCodecSpec.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL0NvbXByZXNzaW9uQ29kZWNTcGVjLmphdmE=) | 71.42% | [4 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/pinot/pull/18097?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ocal/segment/index/loader/ForwardIndexHandler.java](https://app.codecov.io/gh/apache/pinot/pull/18097?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Fsegment%2Findex%2Floader%2FForwardIndexHandler.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9Gb3J3YXJkSW5kZXhIYW5kbGVyLmphdmE=) | 80.00% | [2 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/pinot/pull/18097?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...t/segment/local/io/compression/GzipCompressor.java](https://app.codecov.io/gh/apache/pinot/pull/18097?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Fio%2Fcompression%2FGzipCompressor.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pby9jb21wcmVzc2lvbi9HemlwQ29tcHJlc3Nvci5qYXZh) | 0.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/18097?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../local/io/compression/LZ4WithLengthCompressor.java](https://app.codecov.io/gh/apache/pinot/pull/18097?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Fio%2Fcompression%2FLZ4WithLengthCompressor.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pby9jb21wcmVzc2lvbi9MWjRXaXRoTGVuZ3RoQ29tcHJlc3Nvci5qYXZh) | 0.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/18097?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| ... and [13 more](https://app.codecov.io/gh/apache/pinot/pull/18097?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | |
> :exclamation: There is a different number of reports uploaded between BASE (0c6521a) and HEAD (39e9169). Click for more details.
>
> <details><summary>HEAD has 32 uploads less than BASE</summary>
>
>| Flag | BASE (0c6521a) | HEAD (39e9169) |
>|------|------|------|
>|java-21|4|1|
>|unittests1|2|1|
>|unittests|4|1|
>|temurin|9|1|
>|java-11|5|0|
>|unittests2|2|0|
>|integration|5|0|
>|integration1|2|0|
>|integration2|1|0|
>|custom-integration1|2|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #18097 +/- ##
============================================
- Coverage 63.78% 56.11% -7.67%
+ Complexity 1573 787 -786
============================================
Files 3167 2472 -695
Lines 191840 140985 -50855
Branches 29517 22600 -6917
============================================
- Hits 122360 79113 -43247
+ Misses 59829 55095 -4734
+ Partials 9651 6777 -2874
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/18097/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/18097/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/18097/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/18097/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/18097/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/18097/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/18097/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.11% <56.84%> (-7.64%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/18097/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.11% <56.84%> (-7.67%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/18097/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.11% <56.84%> (-7.67%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/18097/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.11% <56.84%> (-0.04%)` | :arrow_down: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/18097/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/18097?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add parameterized forward index compression codecs [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #18097:
URL: https://github.com/apache/pinot/pull/18097#discussion_r3070980184
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/fwd/MultiValueFixedByteRawIndexCreator.java:
##########
@@ -58,50 +59,67 @@ public MultiValueFixedByteRawIndexCreator(File baseIndexDir, ChunkCompressionTyp
int totalDocs, DataType valueType, int maxNumberOfMultiValueElements, boolean deriveNumDocsPerChunk,
int writerVersion, int targetMaxChunkSizeBytes, int targetDocsPerChunk)
throws IOException {
- this(new File(baseIndexDir, column + Indexes.RAW_MV_FORWARD_INDEX_FILE_EXTENSION), compressionType, totalDocs,
- valueType, maxNumberOfMultiValueElements, deriveNumDocsPerChunk, writerVersion, targetMaxChunkSizeBytes,
- targetDocsPerChunk);
+ this(new File(baseIndexDir, column + Indexes.RAW_MV_FORWARD_INDEX_FILE_EXTENSION), compressionType, (Integer) null,
+ totalDocs, valueType, maxNumberOfMultiValueElements, deriveNumDocsPerChunk, writerVersion,
+ targetMaxChunkSizeBytes, targetDocsPerChunk);
+ }
+
+ public MultiValueFixedByteRawIndexCreator(File baseIndexDir, ChunkCompressionType compressionType,
Review Comment:
remove this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add parameterized forward index compression codecs [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #18097:
URL: https://github.com/apache/pinot/pull/18097#discussion_r3070979394
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/fwd/MultiValueFixedByteRawIndexCreator.java:
##########
Review Comment:
remove this constructor, keep only one constructor.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add parameterized forward index compression codecs [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #18097:
URL: https://github.com/apache/pinot/pull/18097#discussion_r3070980913
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/fwd/MultiValueFixedByteRawIndexCreator.java:
##########
@@ -58,50 +59,67 @@ public MultiValueFixedByteRawIndexCreator(File baseIndexDir, ChunkCompressionTyp
int totalDocs, DataType valueType, int maxNumberOfMultiValueElements, boolean deriveNumDocsPerChunk,
int writerVersion, int targetMaxChunkSizeBytes, int targetDocsPerChunk)
throws IOException {
- this(new File(baseIndexDir, column + Indexes.RAW_MV_FORWARD_INDEX_FILE_EXTENSION), compressionType, totalDocs,
- valueType, maxNumberOfMultiValueElements, deriveNumDocsPerChunk, writerVersion, targetMaxChunkSizeBytes,
- targetDocsPerChunk);
+ this(new File(baseIndexDir, column + Indexes.RAW_MV_FORWARD_INDEX_FILE_EXTENSION), compressionType, (Integer) null,
+ totalDocs, valueType, maxNumberOfMultiValueElements, deriveNumDocsPerChunk, writerVersion,
+ targetMaxChunkSizeBytes, targetDocsPerChunk);
+ }
+
+ public MultiValueFixedByteRawIndexCreator(File baseIndexDir, ChunkCompressionType compressionType,
+ @Nullable Integer compressionLevel, String column, int totalDocs, DataType valueType,
+ int maxNumberOfMultiValueElements, boolean deriveNumDocsPerChunk, int writerVersion,
+ int targetMaxChunkSizeBytes, int targetDocsPerChunk)
+ throws IOException {
+ this(new File(baseIndexDir, column + Indexes.RAW_MV_FORWARD_INDEX_FILE_EXTENSION), compressionType,
+ compressionLevel, totalDocs, valueType, maxNumberOfMultiValueElements, deriveNumDocsPerChunk, writerVersion,
+ targetMaxChunkSizeBytes, targetDocsPerChunk);
}
public MultiValueFixedByteRawIndexCreator(File indexFile, ChunkCompressionType compressionType, int totalDocs,
DataType valueType, int maxNumberOfMultiValueElements, boolean deriveNumDocsPerChunk, int writerVersion)
throws IOException {
- this(indexFile, compressionType, totalDocs, valueType, maxNumberOfMultiValueElements, deriveNumDocsPerChunk,
- writerVersion, ForwardIndexConfig.getDefaultTargetMaxChunkSizeBytes(),
+ this(indexFile, compressionType, (Integer) null, totalDocs, valueType, maxNumberOfMultiValueElements,
+ deriveNumDocsPerChunk, writerVersion, ForwardIndexConfig.getDefaultTargetMaxChunkSizeBytes(),
ForwardIndexConfig.getDefaultTargetDocsPerChunk());
}
public MultiValueFixedByteRawIndexCreator(File indexFile, ChunkCompressionType compressionType, int totalDocs,
Review Comment:
remove this
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/fwd/MultiValueFixedByteRawIndexCreator.java:
##########
@@ -58,50 +59,67 @@ public MultiValueFixedByteRawIndexCreator(File baseIndexDir, ChunkCompressionTyp
int totalDocs, DataType valueType, int maxNumberOfMultiValueElements, boolean deriveNumDocsPerChunk,
int writerVersion, int targetMaxChunkSizeBytes, int targetDocsPerChunk)
throws IOException {
- this(new File(baseIndexDir, column + Indexes.RAW_MV_FORWARD_INDEX_FILE_EXTENSION), compressionType, totalDocs,
- valueType, maxNumberOfMultiValueElements, deriveNumDocsPerChunk, writerVersion, targetMaxChunkSizeBytes,
- targetDocsPerChunk);
+ this(new File(baseIndexDir, column + Indexes.RAW_MV_FORWARD_INDEX_FILE_EXTENSION), compressionType, (Integer) null,
+ totalDocs, valueType, maxNumberOfMultiValueElements, deriveNumDocsPerChunk, writerVersion,
+ targetMaxChunkSizeBytes, targetDocsPerChunk);
+ }
+
+ public MultiValueFixedByteRawIndexCreator(File baseIndexDir, ChunkCompressionType compressionType,
+ @Nullable Integer compressionLevel, String column, int totalDocs, DataType valueType,
+ int maxNumberOfMultiValueElements, boolean deriveNumDocsPerChunk, int writerVersion,
+ int targetMaxChunkSizeBytes, int targetDocsPerChunk)
+ throws IOException {
+ this(new File(baseIndexDir, column + Indexes.RAW_MV_FORWARD_INDEX_FILE_EXTENSION), compressionType,
+ compressionLevel, totalDocs, valueType, maxNumberOfMultiValueElements, deriveNumDocsPerChunk, writerVersion,
+ targetMaxChunkSizeBytes, targetDocsPerChunk);
}
public MultiValueFixedByteRawIndexCreator(File indexFile, ChunkCompressionType compressionType, int totalDocs,
Review Comment:
remove this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add parameterized forward index compression codecs [pinot]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #18097:
URL: https://github.com/apache/pinot/pull/18097#discussion_r3034392162
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/utils/TableConfigUtilsTest.java:
##########
@@ -1486,8 +1487,8 @@ public void testValidateBFOnBoolean() {
TableConfig tableconfig3 = new TableConfigBuilder(TableType.REALTIME).setTableName(TABLE_NAME).build();
ObjectNode indexesNode = JsonNodeFactory.instance.objectNode();
indexesNode.putObject("bloom");
- FieldConfig fieldConfig =
- new FieldConfig("MyCol", FieldConfig.EncodingType.DICTIONARY, null, null, null, null, indexesNode, null, null);
+ FieldConfig fieldConfig = new FieldConfig("MyCol", FieldConfig.EncodingType.DICTIONARY, null, null,
+ (CompressionCodecSpec) null, null, indexesNode, null, null);
Review Comment:
This test now needs an explicit `(CompressionCodecSpec) null` cast because the FieldConfig constructors are overloaded on codec type. Using `new FieldConfig.Builder(...)` (or another non-ambiguous construction path) would avoid the cast and make the intent clearer.
##########
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/FieldConfig.java:
##########
@@ -76,34 +77,52 @@ public class FieldConfig extends BaseJsonConfig {
private final List<IndexType> _indexTypes;
private final JsonNode _indexes;
private final JsonNode _tierOverwrites;
+ @JsonProperty("compressionCodec")
+ private final CompressionCodecSpec _compressionCodecSpec;
private final CompressionCodec _compressionCodec;
private final Map<String, String> _properties;
private final TimestampConfig _timestampConfig;
@Deprecated
public FieldConfig(String name, EncodingType encodingType, @Nullable IndexType indexType,
@Nullable CompressionCodec compressionCodec, @Nullable Map<String, String> properties) {
- this(name, encodingType, indexType, null, compressionCodec, null, null, properties, null);
+ this(name, encodingType, indexType, null, CompressionCodecSpec.fromCompressionCodec(compressionCodec), null, null,
+ properties, null);
}
public FieldConfig(String name, EncodingType encodingType, @Nullable List<IndexType> indexTypes,
@Nullable CompressionCodec compressionCodec, @Nullable Map<String, String> properties) {
- this(name, encodingType, null, indexTypes, compressionCodec, null, null, properties, null);
+ this(name, encodingType, null, indexTypes, CompressionCodecSpec.fromCompressionCodec(compressionCodec), null, null,
+ properties, null);
}
@Deprecated
public FieldConfig(String name, EncodingType encodingType, @Nullable IndexType indexType,
@Nullable List<IndexType> indexTypes, @Nullable CompressionCodec compressionCodec,
@Nullable TimestampConfig timestampConfig, @Nullable Map<String, String> properties) {
- this(name, encodingType, indexType, indexTypes, compressionCodec, timestampConfig, null, properties, null);
+ this(name, encodingType, indexType, indexTypes, CompressionCodecSpec.fromCompressionCodec(compressionCodec),
+ timestampConfig, null, properties, null);
+ }
+
+ /**
+ * Deprecated compatibility constructor retained for binary compatibility with callers that still construct
+ * {@link FieldConfig} directly with the enum-based compression codec.
+ */
+ @Deprecated
+ public FieldConfig(String name, EncodingType encodingType, @Nullable IndexType indexType,
+ @Nullable List<IndexType> indexTypes, @Nullable CompressionCodec compressionCodec,
+ @Nullable TimestampConfig timestampConfig, @Nullable JsonNode indexes,
+ @Nullable Map<String, String> properties, @Nullable JsonNode tierOverwrites) {
+ this(name, encodingType, indexType, indexTypes, CompressionCodecSpec.fromCompressionCodec(compressionCodec),
+ timestampConfig, indexes, properties, tierOverwrites);
}
Review Comment:
The deprecated enum-based compatibility constructor has the same arity as the new spec-based (JsonCreator) constructor, which makes source calls that pass `null` for the codec parameter ambiguous (several tests now require `(CompressionCodecSpec) null` casts). To reduce ongoing friction for callers, consider documenting this explicitly on the deprecated constructor and/or steering call sites toward `FieldConfig.Builder` / the spec-based constructor so `null` does not require a cast.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add parameterized forward index compression codecs [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #18097:
URL: https://github.com/apache/pinot/pull/18097#discussion_r3072573981
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/fwd/MultiValueVarByteRawIndexCreator.java:
##########
@@ -41,58 +41,26 @@ public class MultiValueVarByteRawIndexCreator implements ForwardIndexCreator {
private final VarByteChunkWriter _indexWriter;
private final DataType _valueType;
- /**
- * Create a var-byte raw index creator for the given column
- *
- * @param baseIndexDir Index directory
- * @param compressionType Type of compression to use
- * @param column Name of column to index
- * @param totalDocs Total number of documents to index
- * @param valueType Type of the values
- * @param maxRowLengthInBytes the length in bytes of the largest row
- * @param maxNumberOfElements the maximum number of elements in a row
- */
- public MultiValueVarByteRawIndexCreator(File baseIndexDir, ChunkCompressionType compressionType, String column,
- int totalDocs, DataType valueType, int maxRowLengthInBytes, int maxNumberOfElements)
- throws IOException {
- this(baseIndexDir, compressionType, column, totalDocs, valueType, ForwardIndexConfig.getDefaultRawWriterVersion(),
- maxRowLengthInBytes, maxNumberOfElements, ForwardIndexConfig.getDefaultTargetMaxChunkSizeBytes(),
- ForwardIndexConfig.getDefaultTargetDocsPerChunk());
- }
-
- /**
- * Create a var-byte raw index creator for the given column
- *
- * @param baseIndexDir Index directory
- * @param compressionType Type of compression to use
- * @param column Name of column to index
- * @param totalDocs Total number of documents to index
- * @param valueType Type of the values
- * @param maxRowLengthInBytes the size in bytes of the largest row, the chunk size cannot be smaller than this
- * @param maxNumberOfElements the maximum number of elements in a row
- * @param writerVersion writer format version
- */
- public MultiValueVarByteRawIndexCreator(File baseIndexDir, ChunkCompressionType compressionType, String column,
- int totalDocs, DataType valueType, int writerVersion, int maxRowLengthInBytes, int maxNumberOfElements,
- int targetMaxChunkSizeBytes, int targetDocsPerChunk)
+ public MultiValueVarByteRawIndexCreator(File baseIndexDir, ChunkCompressionType compressionType,
Review Comment:
add back comments
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add parameterized forward index compression codecs [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #18097:
URL: https://github.com/apache/pinot/pull/18097#discussion_r3072575596
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/fwd/SingleValueFixedByteRawIndexCreator.java:
##########
@@ -36,41 +36,13 @@ public class SingleValueFixedByteRawIndexCreator implements ForwardIndexCreator
private final FixedByteChunkForwardIndexWriter _indexWriter;
private final DataType _valueType;
- /**
- * Constructor for the class
- *
- * @param baseIndexDir Index directory
- * @param compressionType Type of compression to use
- * @param column Name of column to index
- * @param totalDocs Total number of documents to index
- * @param valueType Type of the values
- * @throws IOException
- */
- public SingleValueFixedByteRawIndexCreator(File baseIndexDir, ChunkCompressionType compressionType, String column,
- int totalDocs, DataType valueType)
- throws IOException {
- this(baseIndexDir, compressionType, column, totalDocs, valueType, ForwardIndexConfig.getDefaultRawWriterVersion(),
- ForwardIndexConfig.getDefaultTargetDocsPerChunk());
- }
-
- /**
- * Constructor for the class
- *
- * @param baseIndexDir Index directory
- * @param compressionType Type of compression to use
- * @param column Name of column to index
- * @param totalDocs Total number of documents to index
- * @param valueType Type of the values
- * @param writerVersion writer format version
- * @throws IOException
- */
- public SingleValueFixedByteRawIndexCreator(File baseIndexDir, ChunkCompressionType compressionType, String column,
- int totalDocs, DataType valueType, int writerVersion, int targetDocsPerChunk)
+ public SingleValueFixedByteRawIndexCreator(File baseIndexDir, ChunkCompressionType compressionType,
Review Comment:
add back comments
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/fwd/SingleValueVarByteRawIndexCreator.java:
##########
@@ -41,57 +41,24 @@ public class SingleValueVarByteRawIndexCreator implements ForwardIndexCreator {
private final VarByteChunkWriter _indexWriter;
private final DataType _valueType;
- /**
- * Create a var-byte raw index creator for the given column
- * @param baseIndexDir Index directory
- * @param compressionType Type of compression to use
- * @param column Name of column to index
- * @param totalDocs Total number of documents to index
- * @param valueType Type of the values
- * @param maxLength length of longest entry (in bytes)
- * @throws IOException
- */
- public SingleValueVarByteRawIndexCreator(File baseIndexDir, ChunkCompressionType compressionType, String column,
- int totalDocs, DataType valueType, int maxLength)
- throws IOException {
- this(baseIndexDir, compressionType, column, totalDocs, valueType, maxLength, false,
- ForwardIndexConfig.getDefaultRawWriterVersion(), ForwardIndexConfig.getDefaultTargetMaxChunkSizeBytes(),
- ForwardIndexConfig.getDefaultTargetDocsPerChunk());
- }
-
- /**
- * Create a var-byte raw index creator for the given column
- * @param baseIndexDir Index directory
- * @param compressionType Type of compression to use
- * @param column Name of column to index
- * @param totalDocs Total number of documents to index
- * @param valueType Type of the values
- * @param maxLength length of longest entry (in bytes)
- * @param deriveNumDocsPerChunk true if writer should auto-derive the number of rows per chunk
- * @param writerVersion writer format version
- * @param targetMaxChunkSizeBytes target max chunk size in bytes, applicable only for V4 or when
- * deriveNumDocsPerChunk is true
- * @param targetDocsPerChunk target number of docs per chunk
- * @throws IOException
- */
- public SingleValueVarByteRawIndexCreator(File baseIndexDir, ChunkCompressionType compressionType, String column,
- int totalDocs, DataType valueType, int maxLength, boolean deriveNumDocsPerChunk, int writerVersion,
- int targetMaxChunkSizeBytes, int targetDocsPerChunk)
+ public SingleValueVarByteRawIndexCreator(File baseIndexDir, ChunkCompressionType compressionType,
Review Comment:
add back comments
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add parameterized forward index compression codecs [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 closed pull request #18097: Add parameterized forward index compression codecs
URL: https://github.com/apache/pinot/pull/18097
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org