You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/01/18 17:24:07 UTC

[GitHub] [pinot] klsince opened a new pull request #8035: refine segment consistency check

klsince opened a new pull request #8035:
URL: https://github.com/apache/pinot/pull/8035


   ## Description
   This PR refines the checks about whether there is a need to update indices during segment loading like adding indices. Previously, the check was based on Set equality, but that could be false positive, e.g. a sorted column is skipped for inverted index and range index. So in this PR, we aligns the logic between needUpdateIndices() and updateIndices() methods of IndexHandler to be more precise on such checks.
   
   
   ## Upgrade Notes
   Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
   * [ ] Yes (Please label as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
   
   Does this PR fix a zero-downtime upgrade introduced earlier?
   * [ ] Yes (Please label this as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
   
   Does this PR otherwise need attention when creating release notes? Things to consider:
   - New configuration options
   - Deprecation of configurations
   - Signature changes to public methods/interfaces
   - New plugins added or old plugins removed
   * [ ] Yes (Please label this PR as **<code>release-notes</code>** and complete the section on Release Notes)
   ## Release Notes
   <!-- If you have tagged this as either backward-incompat or release-notes,
   you MUST add text here that you would like to see appear in release notes of the
   next release. -->
   
   <!-- If you have a series of commits adding or enabling a feature, then
   add this section only in final commit that marks the feature completed.
   Refer to earlier release notes to see examples of text.
   -->
   ## Documentation
   <!-- If you have introduced a new feature or configuration, please add it to the documentation as well.
   See https://docs.pinot.apache.org/developers/developers-and-contributors/update-document
   -->
   


-- 
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

[GitHub] [pinot] codecov-commenter commented on pull request #8035: refine segment consistency check

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #8035:
URL: https://github.com/apache/pinot/pull/8035#issuecomment-1015900777


   # [Codecov](https://codecov.io/gh/apache/pinot/pull/8035?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#8035](https://codecov.io/gh/apache/pinot/pull/8035?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ad87be5) into [master](https://codecov.io/gh/apache/pinot/commit/7ec47c420be9c6aee6c8e95644266fe9b7fe7a2b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7ec47c4) will **increase** coverage by `0.04%`.
   > The diff coverage is `79.31%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/pinot/pull/8035/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/pinot/pull/8035?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #8035      +/-   ##
   ============================================
   + Coverage     71.29%   71.34%   +0.04%     
   - Complexity     4224     4256      +32     
   ============================================
     Files          1599     1599              
     Lines         82957    83055      +98     
     Branches      12375    12395      +20     
   ============================================
   + Hits          59148    59254     +106     
   + Misses        19815    19799      -16     
   - Partials       3994     4002       +8     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration1 | `28.97% <0.00%> (-0.05%)` | :arrow_down: |
   | integration2 | `27.69% <0.00%> (+0.05%)` | :arrow_up: |
   | unittests1 | `68.16% <79.31%> (+0.02%)` | :arrow_up: |
   | unittests2 | `14.31% <0.00%> (-0.05%)` | :arrow_down: |
   
   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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8035?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ent/index/loader/invertedindex/H3IndexHandler.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9pbnZlcnRlZGluZGV4L0gzSW5kZXhIYW5kbGVyLmphdmE=) | `74.07% <73.33%> (+0.54%)` | :arrow_up: |
   | [...t/index/loader/invertedindex/JsonIndexHandler.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9pbnZlcnRlZGluZGV4L0pzb25JbmRleEhhbmRsZXIuamF2YQ==) | `73.33% <73.33%> (+0.75%)` | :arrow_up: |
   | [.../index/loader/invertedindex/RangeIndexHandler.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9pbnZlcnRlZGluZGV4L1JhbmdlSW5kZXhIYW5kbGVyLmphdmE=) | `49.55% <73.33%> (+3.55%)` | :arrow_up: |
   | [...t/index/loader/bloomfilter/BloomFilterHandler.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9ibG9vbWZpbHRlci9CbG9vbUZpbHRlckhhbmRsZXIuamF2YQ==) | `80.82% <75.00%> (+0.49%)` | :arrow_up: |
   | [...nt/index/loader/invertedindex/FSTIndexHandler.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9pbnZlcnRlZGluZGV4L0ZTVEluZGV4SGFuZGxlci5qYXZh) | `85.29% <77.77%> (-1.50%)` | :arrow_down: |
   | [...t/index/loader/invertedindex/TextIndexHandler.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9pbnZlcnRlZGluZGV4L1RleHRJbmRleEhhbmRsZXIuamF2YQ==) | `85.54% <77.77%> (-1.23%)` | :arrow_down: |
   | [...ocal/segment/index/loader/SegmentPreProcessor.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9TZWdtZW50UHJlUHJvY2Vzc29yLmphdmE=) | `80.80% <100.00%> (+0.80%)` | :arrow_up: |
   | [...dex/loader/invertedindex/InvertedIndexHandler.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9pbnZlcnRlZGluZGV4L0ludmVydGVkSW5kZXhIYW5kbGVyLmphdmE=) | `96.87% <100.00%> (+2.75%)` | :arrow_up: |
   | [.../common/request/context/predicate/InPredicate.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVxdWVzdC9jb250ZXh0L3ByZWRpY2F0ZS9JblByZWRpY2F0ZS5qYXZh) | `75.00% <0.00%> (-5.00%)` | :arrow_down: |
   | [...pache/pinot/core/query/optimizer/filter/Range.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvZmlsdGVyL1JhbmdlLmphdmE=) | `95.91% <0.00%> (-4.09%)` | :arrow_down: |
   | ... and [20 more](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/8035?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/8035?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [7ec47c4...ad87be5](https://codecov.io/gh/apache/pinot/pull/8035?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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

[GitHub] [pinot] npawar commented on a change in pull request #8035: refine segment consistency check

Posted by GitBox <gi...@apache.org>.
npawar commented on a change in pull request #8035:
URL: https://github.com/apache/pinot/pull/8035#discussion_r787093993



##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/FSTIndexHandler.java
##########
@@ -79,8 +79,24 @@ public FSTIndexHandler(SegmentMetadata segmentMetadata, IndexLoadingConfig index
 
   @Override
   public boolean needUpdateIndices(SegmentDirectory.Reader segmentReader) {
+    String segmentName = _segmentMetadata.getName();
     Set<String> existingColumns = segmentReader.toSegmentDirectory().getColumnsWithIndex(ColumnIndexType.FST_INDEX);
-    return !existingColumns.equals(_columnsToAddIdx);
+    // Check if any existing index need to be removed.
+    for (String column : existingColumns) {
+      if (!_columnsToAddIdx.remove(column)) {
+        LOGGER.debug("Need to remove existing FST index from segment: {}, column: {}", segmentName, column);
+        return true;
+      }
+    }
+    // Check if any new index need to be added.
+    for (String column : _columnsToAddIdx) {
+      ColumnMetadata columnMetadata = _segmentMetadata.getColumnMetadataFor(column);
+      if (columnMetadata != null) {

Review comment:
       if any of the checks in `checkUnsupportedOperationsForFSTIndex` fail, we would have simply downloaded the segment, only to throw exception. How about checking those here as well (by sharing the method) and failing here itself?

##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/bloomfilter/BloomFilterHandler.java
##########
@@ -65,9 +65,25 @@ public BloomFilterHandler(SegmentMetadata segmentMetadata, IndexLoadingConfig in
 
   @Override
   public boolean needUpdateIndices(SegmentDirectory.Reader segmentReader) {
+    String segmentName = _segmentMetadata.getName();
     Set<String> columnsToAddBF = new HashSet<>(_bloomFilterConfigs.keySet());
     Set<String> existingColumns = segmentReader.toSegmentDirectory().getColumnsWithIndex(ColumnIndexType.BLOOM_FILTER);
-    return !existingColumns.equals(columnsToAddBF);
+    // Check if any existing bloomfilter need to be removed.
+    for (String column : existingColumns) {
+      if (!columnsToAddBF.remove(column)) {
+        LOGGER.debug("Need to remove existing bloom filter from segment: {}, column: {}", segmentName, column);
+        return true;
+      }
+    }
+    // Check if any new bloomfilter need to be added.
+    for (String column : columnsToAddBF) {
+      ColumnMetadata columnMetadata = _segmentMetadata.getColumnMetadataFor(column);
+      if (columnMetadata != null && columnMetadata.hasDictionary()) {

Review comment:
       should we make a short util method like so?
   ```
   boolean shouldCreateBloomFilter(ColumnMetadata columMetadata) {
     return columnMetadata != null && columnMetadata.hasDictionary();
   }
   ```
   It can be shared by `needUpdate` and `update`, so we reduce chances of them becoming inconsistent over time. 

##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/InvertedIndexHandler.java
##########
@@ -56,9 +56,26 @@ public InvertedIndexHandler(SegmentMetadata segmentMetadata, IndexLoadingConfig
 
   @Override
   public boolean needUpdateIndices(SegmentDirectory.Reader segmentReader) {
+    String segmentName = _segmentMetadata.getName();
     Set<String> existingColumns =
         segmentReader.toSegmentDirectory().getColumnsWithIndex(ColumnIndexType.INVERTED_INDEX);
-    return !existingColumns.equals(_columnsToAddIdx);
+    // Check if any existing index need to be removed.
+    for (String column : existingColumns) {
+      if (!_columnsToAddIdx.remove(column)) {
+        LOGGER.debug("Need to remove existing inverted index from segment: {}, column: {}", segmentName, column);
+        return true;
+      }
+    }
+    // Check if any new index need to be added.
+    for (String column : _columnsToAddIdx) {
+      ColumnMetadata columnMetadata = _segmentMetadata.getColumnMetadataFor(column);
+      // Only create inverted index on dictionary-encoded unsorted columns.
+      if (columnMetadata != null && !columnMetadata.isSorted() && columnMetadata.hasDictionary()) {

Review comment:
       same comment here about the local util method to share between methods need/update (and for other indexes too, even if it's just a null check). Future developers and reviewers may not always catch changes to this if in separate places.

##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/RangeIndexHandler.java
##########
@@ -58,8 +58,25 @@ public RangeIndexHandler(SegmentMetadata segmentMetadata, IndexLoadingConfig ind
 
   @Override
   public boolean needUpdateIndices(SegmentDirectory.Reader segmentReader) {
+    String segmentName = _segmentMetadata.getName();
     Set<String> existingColumns = segmentReader.toSegmentDirectory().getColumnsWithIndex(ColumnIndexType.RANGE_INDEX);
-    return !existingColumns.equals(_columnsToAddIdx);
+    // Check if any existing index need to be removed.
+    for (String column : existingColumns) {
+      if (!_columnsToAddIdx.remove(column)) {
+        LOGGER.debug("Need to remove existing range index from segment: {}, column: {}", segmentName, column);

Review comment:
       i feel there should be at least 1 info log from these methods (even both becoming info is fine imo). Will give us more visibility into the decisions made in the `need` methods.

##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/SegmentPreProcessor.java
##########
@@ -143,22 +143,26 @@ public boolean needProcess()
         DefaultColumnHandler defaultColumnHandler = DefaultColumnHandlerFactory
             .getDefaultColumnHandler(null, _segmentMetadata, _indexLoadingConfig, _schema, null);
         if (defaultColumnHandler.needUpdateDefaultColumns()) {
+          LOGGER.debug("Found default columns need updates");
           return true;
         }
       }
       // Check if there is need to update single-column indices, like inverted index, json index etc.
       for (ColumnIndexType type : ColumnIndexType.values()) {
         if (IndexHandlerFactory.getIndexHandler(type, _segmentMetadata, _indexLoadingConfig)
             .needUpdateIndices(segmentReader)) {
+          LOGGER.debug("Found index type: {} needs updates", type);
           return true;
         }
       }
       // Check if there is need to create/modify/remove star-trees.
       if (needProcessStarTrees()) {
+        LOGGER.debug("Found startree index needs updates");
         return true;
       }
       // Check if there is need to update column min max value.
       if (needUpdateColumnMinMaxValue()) {

Review comment:
       make these info? only 0-1 of these will be printed anyway

##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/TextIndexHandler.java
##########
@@ -93,8 +93,24 @@ public TextIndexHandler(SegmentMetadata segmentMetadata, IndexLoadingConfig inde
 
   @Override
   public boolean needUpdateIndices(SegmentDirectory.Reader segmentReader) {
+    String segmentName = _segmentMetadata.getName();
     Set<String> existingColumns = segmentReader.toSegmentDirectory().getColumnsWithIndex(ColumnIndexType.TEXT_INDEX);
-    return !existingColumns.equals(_columnsToAddIdx);
+    // Check if any existing index need to be removed.
+    for (String column : existingColumns) {
+      if (!_columnsToAddIdx.remove(column)) {
+        LOGGER.debug("Need to remove existing text index from segment: {}, column: {}", segmentName, column);
+        return true;
+      }
+    }
+    // Check if any new index need to be added.
+    for (String column : _columnsToAddIdx) {
+      ColumnMetadata columnMetadata = _segmentMetadata.getColumnMetadataFor(column);
+      if (columnMetadata != null) {

Review comment:
       same comment as FST index regarding the `checkUnsupportedOperationsForTextIndex`. Text indexes will be large, and we'd rather fail sooner and skip download. wdyt?




-- 
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

[GitHub] [pinot] klsince commented on a change in pull request #8035: refine segment consistency check

Posted by GitBox <gi...@apache.org>.
klsince commented on a change in pull request #8035:
URL: https://github.com/apache/pinot/pull/8035#discussion_r787135117



##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/FSTIndexHandler.java
##########
@@ -79,8 +79,24 @@ public FSTIndexHandler(SegmentMetadata segmentMetadata, IndexLoadingConfig index
 
   @Override
   public boolean needUpdateIndices(SegmentDirectory.Reader segmentReader) {
+    String segmentName = _segmentMetadata.getName();
     Set<String> existingColumns = segmentReader.toSegmentDirectory().getColumnsWithIndex(ColumnIndexType.FST_INDEX);
-    return !existingColumns.equals(_columnsToAddIdx);
+    // Check if any existing index need to be removed.
+    for (String column : existingColumns) {
+      if (!_columnsToAddIdx.remove(column)) {
+        LOGGER.debug("Need to remove existing FST index from segment: {}, column: {}", segmentName, column);
+        return true;
+      }
+    }
+    // Check if any new index need to be added.
+    for (String column : _columnsToAddIdx) {
+      ColumnMetadata columnMetadata = _segmentMetadata.getColumnMetadataFor(column);
+      if (columnMetadata != null) {

Review comment:
       makes sense to me to fail fast here




-- 
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

[GitHub] [pinot] klsince commented on pull request #8035: refine segment consistency check

Posted by GitBox <gi...@apache.org>.
klsince commented on pull request #8035:
URL: https://github.com/apache/pinot/pull/8035#issuecomment-1016078054


   > lgtm! Just want to check that you've confirmed this - will the failing fast in `needPreprocess` have any other side effects on the segment build process (such as incomplete cleanup). I think not, but just checking.
   
   The `needPreprocess` is wrapped in try-catch block that also wraps the the segment preprocessing. And cleanup happens in the catch block. The new changes would make it fail faster upon unsupported operations, saving some cost. 


-- 
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

[GitHub] [pinot] klsince commented on pull request #8035: refine segment consistency check

Posted by GitBox <gi...@apache.org>.
klsince commented on pull request #8035:
URL: https://github.com/apache/pinot/pull/8035#issuecomment-1016972545


   Think I'm getting closer to the cause with some tests in this [debug PR](https://github.com/apache/pinot/pull/8038), but not clear about the root cause yet. I don't think it's related to changes in this PR and would like to merge this one for now. If anyone is familiar with the compatibility tests, please chime in on the debug PR. Thanks!


-- 
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

[GitHub] [pinot] codecov-commenter edited a comment on pull request #8035: refine segment consistency check

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8035:
URL: https://github.com/apache/pinot/pull/8035#issuecomment-1015900777


   # [Codecov](https://codecov.io/gh/apache/pinot/pull/8035?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#8035](https://codecov.io/gh/apache/pinot/pull/8035?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c7ccb40) into [master](https://codecov.io/gh/apache/pinot/commit/7ec47c420be9c6aee6c8e95644266fe9b7fe7a2b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7ec47c4) will **decrease** coverage by `40.58%`.
   > The diff coverage is `0.00%`.
   
   > :exclamation: Current head c7ccb40 differs from pull request most recent head 72941a9. Consider uploading reports for the commit 72941a9 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/pinot/pull/8035/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/pinot/pull/8035?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@              Coverage Diff              @@
   ##             master    #8035       +/-   ##
   =============================================
   - Coverage     71.29%   30.71%   -40.59%     
   =============================================
     Files          1599     1590        -9     
     Lines         82957    82707      -250     
     Branches      12375    12357       -18     
   =============================================
   - Hits          59148    25406    -33742     
   - Misses        19815    55081    +35266     
   + Partials       3994     2220     -1774     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration1 | `29.03% <0.00%> (+<0.01%)` | :arrow_up: |
   | integration2 | `27.66% <0.00%> (+0.02%)` | :arrow_up: |
   | unittests1 | `?` | |
   | unittests2 | `?` | |
   
   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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8035?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ocal/segment/index/loader/SegmentPreProcessor.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9TZWdtZW50UHJlUHJvY2Vzc29yLmphdmE=) | `0.00% <0.00%> (-80.00%)` | :arrow_down: |
   | [...t/index/loader/bloomfilter/BloomFilterHandler.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9ibG9vbWZpbHRlci9CbG9vbUZpbHRlckhhbmRsZXIuamF2YQ==) | `0.00% <0.00%> (-80.33%)` | :arrow_down: |
   | [...nt/index/loader/invertedindex/FSTIndexHandler.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9pbnZlcnRlZGluZGV4L0ZTVEluZGV4SGFuZGxlci5qYXZh) | `0.00% <0.00%> (-86.80%)` | :arrow_down: |
   | [...ent/index/loader/invertedindex/H3IndexHandler.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9pbnZlcnRlZGluZGV4L0gzSW5kZXhIYW5kbGVyLmphdmE=) | `0.00% <0.00%> (-73.53%)` | :arrow_down: |
   | [...dex/loader/invertedindex/InvertedIndexHandler.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9pbnZlcnRlZGluZGV4L0ludmVydGVkSW5kZXhIYW5kbGVyLmphdmE=) | `0.00% <0.00%> (-94.12%)` | :arrow_down: |
   | [...t/index/loader/invertedindex/JsonIndexHandler.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9pbnZlcnRlZGluZGV4L0pzb25JbmRleEhhbmRsZXIuamF2YQ==) | `0.00% <0.00%> (-72.59%)` | :arrow_down: |
   | [.../index/loader/invertedindex/RangeIndexHandler.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9pbnZlcnRlZGluZGV4L1JhbmdlSW5kZXhIYW5kbGVyLmphdmE=) | `0.00% <0.00%> (-46.00%)` | :arrow_down: |
   | [...t/index/loader/invertedindex/TextIndexHandler.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9pbnZlcnRlZGluZGV4L1RleHRJbmRleEhhbmRsZXIuamF2YQ==) | `0.00% <0.00%> (-86.77%)` | :arrow_down: |
   | [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...ava/org/apache/pinot/spi/config/table/FSTType.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL0ZTVFR5cGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | ... and [1096 more](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/8035?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/8035?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [7ec47c4...72941a9](https://codecov.io/gh/apache/pinot/pull/8035?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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

[GitHub] [pinot] codecov-commenter edited a comment on pull request #8035: refine segment consistency check

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8035:
URL: https://github.com/apache/pinot/pull/8035#issuecomment-1015900777


   # [Codecov](https://codecov.io/gh/apache/pinot/pull/8035?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#8035](https://codecov.io/gh/apache/pinot/pull/8035?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (72941a9) into [master](https://codecov.io/gh/apache/pinot/commit/7ec47c420be9c6aee6c8e95644266fe9b7fe7a2b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7ec47c4) will **decrease** coverage by `6.41%`.
   > The diff coverage is `79.31%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/pinot/pull/8035/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/pinot/pull/8035?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #8035      +/-   ##
   ============================================
   - Coverage     71.29%   64.88%   -6.42%     
   - Complexity     4224     4256      +32     
   ============================================
     Files          1599     1554      -45     
     Lines         82957    81178    -1779     
     Branches      12375    12191     -184     
   ============================================
   - Hits          59148    52672    -6476     
   - Misses        19815    24749    +4934     
   + Partials       3994     3757     -237     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration1 | `?` | |
   | integration2 | `?` | |
   | unittests1 | `68.15% <79.31%> (+0.01%)` | :arrow_up: |
   | unittests2 | `14.32% <0.00%> (-0.05%)` | :arrow_down: |
   
   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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8035?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ent/index/loader/invertedindex/H3IndexHandler.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9pbnZlcnRlZGluZGV4L0gzSW5kZXhIYW5kbGVyLmphdmE=) | `74.07% <73.33%> (+0.54%)` | :arrow_up: |
   | [...t/index/loader/invertedindex/JsonIndexHandler.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9pbnZlcnRlZGluZGV4L0pzb25JbmRleEhhbmRsZXIuamF2YQ==) | `73.33% <73.33%> (+0.75%)` | :arrow_up: |
   | [.../index/loader/invertedindex/RangeIndexHandler.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9pbnZlcnRlZGluZGV4L1JhbmdlSW5kZXhIYW5kbGVyLmphdmE=) | `49.55% <73.33%> (+3.55%)` | :arrow_up: |
   | [...t/index/loader/bloomfilter/BloomFilterHandler.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9ibG9vbWZpbHRlci9CbG9vbUZpbHRlckhhbmRsZXIuamF2YQ==) | `80.82% <75.00%> (+0.49%)` | :arrow_up: |
   | [...nt/index/loader/invertedindex/FSTIndexHandler.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9pbnZlcnRlZGluZGV4L0ZTVEluZGV4SGFuZGxlci5qYXZh) | `85.29% <77.77%> (-1.50%)` | :arrow_down: |
   | [...t/index/loader/invertedindex/TextIndexHandler.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9pbnZlcnRlZGluZGV4L1RleHRJbmRleEhhbmRsZXIuamF2YQ==) | `85.54% <77.77%> (-1.23%)` | :arrow_down: |
   | [...ocal/segment/index/loader/SegmentPreProcessor.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9TZWdtZW50UHJlUHJvY2Vzc29yLmphdmE=) | `80.80% <100.00%> (+0.80%)` | :arrow_up: |
   | [...dex/loader/invertedindex/InvertedIndexHandler.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9pbnZlcnRlZGluZGV4L0ludmVydGVkSW5kZXhIYW5kbGVyLmphdmE=) | `96.87% <100.00%> (+2.75%)` | :arrow_up: |
   | [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...g/apache/pinot/common/metrics/ControllerMeter.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyTWV0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | ... and [371 more](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/8035?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/8035?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [7ec47c4...72941a9](https://codecov.io/gh/apache/pinot/pull/8035?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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

[GitHub] [pinot] klsince commented on a change in pull request #8035: refine segment consistency check

Posted by GitBox <gi...@apache.org>.
klsince commented on a change in pull request #8035:
URL: https://github.com/apache/pinot/pull/8035#discussion_r787134366



##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/bloomfilter/BloomFilterHandler.java
##########
@@ -65,9 +65,25 @@ public BloomFilterHandler(SegmentMetadata segmentMetadata, IndexLoadingConfig in
 
   @Override
   public boolean needUpdateIndices(SegmentDirectory.Reader segmentReader) {
+    String segmentName = _segmentMetadata.getName();
     Set<String> columnsToAddBF = new HashSet<>(_bloomFilterConfigs.keySet());
     Set<String> existingColumns = segmentReader.toSegmentDirectory().getColumnsWithIndex(ColumnIndexType.BLOOM_FILTER);
-    return !existingColumns.equals(columnsToAddBF);
+    // Check if any existing bloomfilter need to be removed.
+    for (String column : existingColumns) {
+      if (!columnsToAddBF.remove(column)) {
+        LOGGER.debug("Need to remove existing bloom filter from segment: {}, column: {}", segmentName, column);
+        return true;
+      }
+    }
+    // Check if any new bloomfilter need to be added.
+    for (String column : columnsToAddBF) {
+      ColumnMetadata columnMetadata = _segmentMetadata.getColumnMetadataFor(column);
+      if (columnMetadata != null && columnMetadata.hasDictionary()) {

Review comment:
       Excellent point!




-- 
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

[GitHub] [pinot] richardstartin commented on a change in pull request #8035: refine segment consistency check

Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8035:
URL: https://github.com/apache/pinot/pull/8035#discussion_r787198792



##########
File path: pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/SegmentPreProcessorTest.java
##########
@@ -1115,6 +1116,18 @@ private void testIfNeedProcess()
       assertFalse(processor.needProcess());
     }
 
+    // No preprocessing needed if required to add certain index on non-existing, sorted or non-dictionary column.
+    for (Consumer<IndexLoadingConfig> prepFunc : createConfigPrepFunctionNeedNoops()) {

Review comment:
       The purpose of a test is to prevent someone from breaking the implementation unexpectedly in the future as much as it is to demonstrate that a change/feature works, so a test must surface diagnostics about failure. This test was already problematic in this regard, but this change doesn't make it better: If this test ever fails, someone will need to put a breakpoint in this loop to find out when the test actually fails, and when they do that, they will see a `Consumer` without any identifiable state with a class name like "...SegmentPreProcessorTest$$Lambda$86/0x00000008001a5c40". The only way to know which consumer the test fails for would be to modify the test to add a counter to be incremented on each iteration, and then look at the nth position in the list created in `createConfigPrepFunctionNeedNoops`. 
   
   This can all be circumnavigated by using `DataProvider` to create combinations of test inputs (including a descriptive test name) to supply to generic testing logic.




-- 
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

[GitHub] [pinot] mcvsubbu commented on a change in pull request #8035: refine segment consistency check

Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #8035:
URL: https://github.com/apache/pinot/pull/8035#discussion_r787170735



##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/JsonIndexHandler.java
##########
@@ -59,8 +59,24 @@ public JsonIndexHandler(SegmentMetadata segmentMetadata, IndexLoadingConfig inde
 
   @Override
   public boolean needUpdateIndices(SegmentDirectory.Reader segmentReader) {
+    String segmentName = _segmentMetadata.getName();
     Set<String> existingColumns = segmentReader.toSegmentDirectory().getColumnsWithIndex(ColumnIndexType.JSON_INDEX);
-    return !existingColumns.equals(_columnsToAddIdx);
+    // Check if any existing index need to be removed.
+    for (String column : existingColumns) {
+      if (!_columnsToAddIdx.remove(column)) {
+        LOGGER.debug("Need to remove existing json index from segment: {}, column: {}", segmentName, column);

Review comment:
       same here, info will help

##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/RangeIndexHandler.java
##########
@@ -58,8 +58,25 @@ public RangeIndexHandler(SegmentMetadata segmentMetadata, IndexLoadingConfig ind
 
   @Override
   public boolean needUpdateIndices(SegmentDirectory.Reader segmentReader) {
+    String segmentName = _segmentMetadata.getName();
     Set<String> existingColumns = segmentReader.toSegmentDirectory().getColumnsWithIndex(ColumnIndexType.RANGE_INDEX);
-    return !existingColumns.equals(_columnsToAddIdx);
+    // Check if any existing index need to be removed.
+    for (String column : existingColumns) {
+      if (!_columnsToAddIdx.remove(column)) {
+        LOGGER.debug("Need to remove existing range index from segment: {}, column: {}", segmentName, column);

Review comment:
       totally agreed. More than one log at load time does not hurt. The more verbose we are, the easlier it is to locate problems

##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/JsonIndexHandler.java
##########
@@ -59,8 +59,24 @@ public JsonIndexHandler(SegmentMetadata segmentMetadata, IndexLoadingConfig inde
 
   @Override
   public boolean needUpdateIndices(SegmentDirectory.Reader segmentReader) {
+    String segmentName = _segmentMetadata.getName();
     Set<String> existingColumns = segmentReader.toSegmentDirectory().getColumnsWithIndex(ColumnIndexType.JSON_INDEX);
-    return !existingColumns.equals(_columnsToAddIdx);
+    // Check if any existing index need to be removed.
+    for (String column : existingColumns) {
+      if (!_columnsToAddIdx.remove(column)) {
+        LOGGER.debug("Need to remove existing json index from segment: {}, column: {}", segmentName, column);
+        return true;
+      }
+    }
+    // Check if any new index need to be added.
+    for (String column : _columnsToAddIdx) {
+      ColumnMetadata columnMetadata = _segmentMetadata.getColumnMetadataFor(column);
+      if (columnMetadata != null) {
+        LOGGER.debug("Need to create new json index for segment: {}, column: {}", segmentName, column);

Review comment:
       ditto

##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/SegmentPreProcessor.java
##########
@@ -143,22 +143,26 @@ public boolean needProcess()
         DefaultColumnHandler defaultColumnHandler = DefaultColumnHandlerFactory
             .getDefaultColumnHandler(null, _segmentMetadata, _indexLoadingConfig, _schema, null);
         if (defaultColumnHandler.needUpdateDefaultColumns()) {
+          LOGGER.debug("Found default columns need updates");
           return true;
         }
       }
       // Check if there is need to update single-column indices, like inverted index, json index etc.
       for (ColumnIndexType type : ColumnIndexType.values()) {
         if (IndexHandlerFactory.getIndexHandler(type, _segmentMetadata, _indexLoadingConfig)
             .needUpdateIndices(segmentReader)) {
+          LOGGER.debug("Found index type: {} needs updates", type);
           return true;
         }
       }
       // Check if there is need to create/modify/remove star-trees.
       if (needProcessStarTrees()) {
+        LOGGER.debug("Found startree index needs updates");
         return true;
       }
       // Check if there is need to update column min max value.
       if (needUpdateColumnMinMaxValue()) {

Review comment:
       +1




-- 
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

[GitHub] [pinot] npawar commented on a change in pull request #8035: refine segment consistency check

Posted by GitBox <gi...@apache.org>.
npawar commented on a change in pull request #8035:
URL: https://github.com/apache/pinot/pull/8035#discussion_r787098624



##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/TextIndexHandler.java
##########
@@ -93,8 +93,24 @@ public TextIndexHandler(SegmentMetadata segmentMetadata, IndexLoadingConfig inde
 
   @Override
   public boolean needUpdateIndices(SegmentDirectory.Reader segmentReader) {
+    String segmentName = _segmentMetadata.getName();
     Set<String> existingColumns = segmentReader.toSegmentDirectory().getColumnsWithIndex(ColumnIndexType.TEXT_INDEX);
-    return !existingColumns.equals(_columnsToAddIdx);
+    // Check if any existing index need to be removed.
+    for (String column : existingColumns) {
+      if (!_columnsToAddIdx.remove(column)) {
+        LOGGER.debug("Need to remove existing text index from segment: {}, column: {}", segmentName, column);
+        return true;
+      }
+    }
+    // Check if any new index need to be added.
+    for (String column : _columnsToAddIdx) {
+      ColumnMetadata columnMetadata = _segmentMetadata.getColumnMetadataFor(column);
+      if (columnMetadata != null) {

Review comment:
       same comment as FST index regarding the `checkUnsupportedOperationsForTextIndex`. Text indexes will be large, and we'd rather fail sooner. wdyt?




-- 
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

[GitHub] [pinot] npawar merged pull request #8035: refine segment consistency check

Posted by GitBox <gi...@apache.org>.
npawar merged pull request #8035:
URL: https://github.com/apache/pinot/pull/8035


   


-- 
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

[GitHub] [pinot] npawar commented on a change in pull request #8035: refine segment consistency check

Posted by GitBox <gi...@apache.org>.
npawar commented on a change in pull request #8035:
URL: https://github.com/apache/pinot/pull/8035#discussion_r787093993



##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/FSTIndexHandler.java
##########
@@ -79,8 +79,24 @@ public FSTIndexHandler(SegmentMetadata segmentMetadata, IndexLoadingConfig index
 
   @Override
   public boolean needUpdateIndices(SegmentDirectory.Reader segmentReader) {
+    String segmentName = _segmentMetadata.getName();
     Set<String> existingColumns = segmentReader.toSegmentDirectory().getColumnsWithIndex(ColumnIndexType.FST_INDEX);
-    return !existingColumns.equals(_columnsToAddIdx);
+    // Check if any existing index need to be removed.
+    for (String column : existingColumns) {
+      if (!_columnsToAddIdx.remove(column)) {
+        LOGGER.debug("Need to remove existing FST index from segment: {}, column: {}", segmentName, column);
+        return true;
+      }
+    }
+    // Check if any new index need to be added.
+    for (String column : _columnsToAddIdx) {
+      ColumnMetadata columnMetadata = _segmentMetadata.getColumnMetadataFor(column);
+      if (columnMetadata != null) {

Review comment:
       if any of the checks in `checkUnsupportedOperationsForFSTIndex` fail, we would have simply copied the segment, only to throw exception. How about checking those here as well (by sharing the method) and failing here itself?




-- 
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

[GitHub] [pinot] klsince commented on a change in pull request #8035: refine segment consistency check

Posted by GitBox <gi...@apache.org>.
klsince commented on a change in pull request #8035:
URL: https://github.com/apache/pinot/pull/8035#discussion_r787239142



##########
File path: pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/SegmentPreProcessorTest.java
##########
@@ -1115,6 +1116,18 @@ private void testIfNeedProcess()
       assertFalse(processor.needProcess());
     }
 
+    // No preprocessing needed if required to add certain index on non-existing, sorted or non-dictionary column.
+    for (Consumer<IndexLoadingConfig> prepFunc : createConfigPrepFunctionNeedNoops()) {

Review comment:
       didn't use DataProvider but a simple Map to provide the test cases, as the test cases are checked both individually and used to load up a temp config upfront. But anyway, I added test name to assertions for easy debugging.  




-- 
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

[GitHub] [pinot] codecov-commenter edited a comment on pull request #8035: refine segment consistency check

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8035:
URL: https://github.com/apache/pinot/pull/8035#issuecomment-1015900777


   # [Codecov](https://codecov.io/gh/apache/pinot/pull/8035?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#8035](https://codecov.io/gh/apache/pinot/pull/8035?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c7ccb40) into [master](https://codecov.io/gh/apache/pinot/commit/7ec47c420be9c6aee6c8e95644266fe9b7fe7a2b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7ec47c4) will **decrease** coverage by `42.26%`.
   > The diff coverage is `0.00%`.
   
   > :exclamation: Current head c7ccb40 differs from pull request most recent head 72941a9. Consider uploading reports for the commit 72941a9 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/pinot/pull/8035/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/pinot/pull/8035?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@              Coverage Diff              @@
   ##             master    #8035       +/-   ##
   =============================================
   - Coverage     71.29%   29.03%   -42.27%     
   =============================================
     Files          1599     1590        -9     
     Lines         82957    82707      -250     
     Branches      12375    12357       -18     
   =============================================
   - Hits          59148    24014    -35134     
   - Misses        19815    56533    +36718     
   + Partials       3994     2160     -1834     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration1 | `29.03% <0.00%> (+<0.01%)` | :arrow_up: |
   | integration2 | `?` | |
   | unittests1 | `?` | |
   | unittests2 | `?` | |
   
   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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8035?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ocal/segment/index/loader/SegmentPreProcessor.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9TZWdtZW50UHJlUHJvY2Vzc29yLmphdmE=) | `0.00% <0.00%> (-80.00%)` | :arrow_down: |
   | [...t/index/loader/bloomfilter/BloomFilterHandler.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9ibG9vbWZpbHRlci9CbG9vbUZpbHRlckhhbmRsZXIuamF2YQ==) | `0.00% <0.00%> (-80.33%)` | :arrow_down: |
   | [...nt/index/loader/invertedindex/FSTIndexHandler.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9pbnZlcnRlZGluZGV4L0ZTVEluZGV4SGFuZGxlci5qYXZh) | `0.00% <0.00%> (-86.80%)` | :arrow_down: |
   | [...ent/index/loader/invertedindex/H3IndexHandler.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9pbnZlcnRlZGluZGV4L0gzSW5kZXhIYW5kbGVyLmphdmE=) | `0.00% <0.00%> (-73.53%)` | :arrow_down: |
   | [...dex/loader/invertedindex/InvertedIndexHandler.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9pbnZlcnRlZGluZGV4L0ludmVydGVkSW5kZXhIYW5kbGVyLmphdmE=) | `0.00% <0.00%> (-94.12%)` | :arrow_down: |
   | [...t/index/loader/invertedindex/JsonIndexHandler.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9pbnZlcnRlZGluZGV4L0pzb25JbmRleEhhbmRsZXIuamF2YQ==) | `0.00% <0.00%> (-72.59%)` | :arrow_down: |
   | [.../index/loader/invertedindex/RangeIndexHandler.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9pbnZlcnRlZGluZGV4L1JhbmdlSW5kZXhIYW5kbGVyLmphdmE=) | `0.00% <0.00%> (-46.00%)` | :arrow_down: |
   | [...t/index/loader/invertedindex/TextIndexHandler.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9pbnZlcnRlZGluZGV4L1RleHRJbmRleEhhbmRsZXIuamF2YQ==) | `0.00% <0.00%> (-86.77%)` | :arrow_down: |
   | [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...ava/org/apache/pinot/spi/config/table/FSTType.java](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL0ZTVFR5cGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | ... and [1153 more](https://codecov.io/gh/apache/pinot/pull/8035/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/8035?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/8035?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [7ec47c4...72941a9](https://codecov.io/gh/apache/pinot/pull/8035?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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