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 2021/10/04 21:02:44 UTC

[GitHub] [pinot] richardstartin opened a new pull request #7513: Improved range queries

richardstartin opened a new pull request #7513:
URL: https://github.com/apache/pinot/pull/7513


   ## Description
   This makes range queries using the new range index faster (benchmark comparison to follow) in two ways:
   1. Do an open ended query when one of the query parameters is the min value (which always zero) or the max value (which depends on the data type)
   2. Use a new method in `RoaringBitmap` to use the result of the first query to skip over rows.
   
   ## 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] richardstartin commented on a change in pull request #7513: Improved range queries

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



##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/BitSlicedRangeIndexReader.java
##########
@@ -97,12 +106,22 @@ public ImmutableRoaringBitmap getPartiallyMatchingDocIds(double min, double max)
     return null;
   }
 
-  private ImmutableRoaringBitmap queryRangeBitmap(long min, long max) {
+  private ImmutableRoaringBitmap queryRangeBitmap(long min, long max, long columnMax) {
     RangeBitmap rangeBitmap = mapRangeBitmap();
-    RoaringBitmap lte = rangeBitmap.lte(max);
-    RoaringBitmap gte = rangeBitmap.gte(min);
-    lte.and(gte);
-    return lte.toMutableRoaringBitmap();
+    if (Long.compareUnsigned(max, columnMax) < 0) {
+      RoaringBitmap lte = rangeBitmap.lte(max);
+      if (Long.compareUnsigned(min, 0) > 0) {

Review comment:
       It's an unsigned comparison, values stored in the index start at 0 and effectively end at -1L. This feels more readable, but checking it's not 0 is probably faster.




-- 
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] Jackie-Jiang commented on a change in pull request #7513: Improved range queries

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7513:
URL: https://github.com/apache/pinot/pull/7513#discussion_r721755402



##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/BitSlicedRangeIndexReader.java
##########
@@ -45,30 +50,34 @@ public BitSlicedRangeIndexReader(PinotDataBuffer dataBuffer) {
     _min = dataBuffer.getLong(offset);
     offset += Long.BYTES;
     _offset = offset;
+    _max = metadata.hasDictionary()

Review comment:
       Here using either of them should be fine, especially when the creator is using the min/max value from the column metadata.
   In some places reading the metadata min/max value might cause precision loss because it involves ser/de of the values.




-- 
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 #7513: Improved range queries

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


   # [Codecov](https://codecov.io/gh/apache/pinot/pull/7513?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 [#7513](https://codecov.io/gh/apache/pinot/pull/7513?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (005f58a) into [master](https://codecov.io/gh/apache/pinot/commit/70374e2be54aeb2bd64d5d9759e5beff62d4c81a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (70374e2) will **decrease** coverage by `6.06%`.
   > The diff coverage is `95.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/pinot/pull/7513/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/7513?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    #7513      +/-   ##
   ============================================
   - Coverage     72.00%   65.93%   -6.07%     
   - Complexity     3404     3411       +7     
   ============================================
     Files          1523     1477      -46     
     Lines         75509    73699    -1810     
     Branches      11004    10811     -193     
   ============================================
   - Hits          54373    48597    -5776     
   - Misses        17480    21648    +4168     
   + Partials       3656     3454     -202     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration1 | `?` | |
   | integration2 | `?` | |
   | unittests1 | `69.91% <95.00%> (+0.09%)` | :arrow_up: |
   | unittests2 | `15.26% <0.00%> (+0.76%)` | :arrow_up: |
   
   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/7513?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/column/PhysicalColumnIndexContainer.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2NvbHVtbi9QaHlzaWNhbENvbHVtbkluZGV4Q29udGFpbmVyLmphdmE=) | `82.96% <0.00%> (ø)` | |
   | [...gment/index/readers/BitSlicedRangeIndexReader.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvQml0U2xpY2VkUmFuZ2VJbmRleFJlYWRlci5qYXZh) | `81.08% <100.00%> (+7.00%)` | :arrow_up: |
   | [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/7513/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/7513/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: |
   | [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../apache/pinot/common/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...he/pinot/common/messages/SegmentReloadMessage.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWVzc2FnZXMvU2VnbWVudFJlbG9hZE1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...pinot/core/data/manager/realtime/TimerService.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvVGltZXJTZXJ2aWNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhjZXB0aW9uL1Rhc2tDYW5jZWxsZWRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...not/common/exception/HttpErrorStatusException.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL0h0dHBFcnJvclN0YXR1c0V4Y2VwdGlvbi5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | ... and [368 more](https://codecov.io/gh/apache/pinot/pull/7513/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/7513?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/7513?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 [70374e2...005f58a](https://codecov.io/gh/apache/pinot/pull/7513?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] Jackie-Jiang commented on a change in pull request #7513: Improved range queries

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7513:
URL: https://github.com/apache/pinot/pull/7513#discussion_r721732010



##########
File path: pinot-perf/src/main/java/org/apache/pinot/perf/BenchmarkRangeIndex.java
##########
@@ -196,7 +196,11 @@ public void setup()
     @TearDown(Level.Trial)
     public void tearDown()
         throws IOException {
-      FileUtils.forceDelete(_indexDir);
+      try {
+        FileUtils.forceDelete(_indexDir);

Review comment:
       I think we did it in a lot of places. We might want to create a PR to change all of them to `FileUtils.deleteQuietly()`?




-- 
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 #7513: Improved range queries

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



##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/BitSlicedRangeIndexReader.java
##########
@@ -45,30 +50,34 @@ public BitSlicedRangeIndexReader(PinotDataBuffer dataBuffer) {
     _min = dataBuffer.getLong(offset);
     offset += Long.BYTES;
     _offset = offset;
+    _max = metadata.hasDictionary()

Review comment:
       Isn't it better to rely on the metadata? If information were stored in two places it can be out of sync. This way the column metadata is the source of truth.




-- 
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 #7513: Improved range queries

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


   # [Codecov](https://codecov.io/gh/apache/pinot/pull/7513?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 [#7513](https://codecov.io/gh/apache/pinot/pull/7513?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a5776b3) into [master](https://codecov.io/gh/apache/pinot/commit/70374e2be54aeb2bd64d5d9759e5beff62d4c81a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (70374e2) will **decrease** coverage by `6.16%`.
   > The diff coverage is `95.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/pinot/pull/7513/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/7513?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    #7513      +/-   ##
   ============================================
   - Coverage     72.00%   65.83%   -6.17%     
   - Complexity     3404     3409       +5     
   ============================================
     Files          1523     1477      -46     
     Lines         75509    73767    -1742     
     Branches      11004    10825     -179     
   ============================================
   - Hits          54373    48568    -5805     
   - Misses        17480    21740    +4260     
   + Partials       3656     3459     -197     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration1 | `?` | |
   | integration2 | `?` | |
   | unittests1 | `69.85% <95.00%> (+0.02%)` | :arrow_up: |
   | unittests2 | `15.25% <0.00%> (+0.75%)` | :arrow_up: |
   
   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/7513?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/column/PhysicalColumnIndexContainer.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2NvbHVtbi9QaHlzaWNhbENvbHVtbkluZGV4Q29udGFpbmVyLmphdmE=) | `82.96% <0.00%> (ø)` | |
   | [...gment/index/readers/BitSlicedRangeIndexReader.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvQml0U2xpY2VkUmFuZ2VJbmRleFJlYWRlci5qYXZh) | `81.08% <100.00%> (+7.00%)` | :arrow_up: |
   | [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/7513/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/7513/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: |
   | [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../apache/pinot/common/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...he/pinot/common/messages/SegmentReloadMessage.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWVzc2FnZXMvU2VnbWVudFJlbG9hZE1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...pinot/core/data/manager/realtime/TimerService.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvVGltZXJTZXJ2aWNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhjZXB0aW9uL1Rhc2tDYW5jZWxsZWRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...not/common/exception/HttpErrorStatusException.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL0h0dHBFcnJvclN0YXR1c0V4Y2VwdGlvbi5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | ... and [366 more](https://codecov.io/gh/apache/pinot/pull/7513/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/7513?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/7513?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 [70374e2...a5776b3](https://codecov.io/gh/apache/pinot/pull/7513?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] kishoreg merged pull request #7513: Improved range queries

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


   


-- 
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 #7513: Improved range queries

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



##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/BitSlicedRangeIndexReader.java
##########
@@ -97,12 +106,22 @@ public ImmutableRoaringBitmap getPartiallyMatchingDocIds(double min, double max)
     return null;
   }
 
-  private ImmutableRoaringBitmap queryRangeBitmap(long min, long max) {
+  private ImmutableRoaringBitmap queryRangeBitmap(long min, long max, long columnMax) {
     RangeBitmap rangeBitmap = mapRangeBitmap();
-    RoaringBitmap lte = rangeBitmap.lte(max);
-    RoaringBitmap gte = rangeBitmap.gte(min);
-    lte.and(gte);
-    return lte.toMutableRoaringBitmap();
+    if (Long.compareUnsigned(max, columnMax) < 0) {
+      RoaringBitmap lte = rangeBitmap.lte(max);
+      if (Long.compareUnsigned(min, 0) > 0) {

Review comment:
       I don't want to change this because it's less surprising if all of the comparisons use `Long.compareUnsigned` than if only the ones that can't be optimised to something else do. The other operations here will take at least 10s of microseconds even for small segments, so this isn't worth optimising IMO.




-- 
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] Jackie-Jiang commented on a change in pull request #7513: Improved range queries

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7513:
URL: https://github.com/apache/pinot/pull/7513#discussion_r721738360



##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/BitSlicedRangeIndexReader.java
##########
@@ -45,30 +50,34 @@ public BitSlicedRangeIndexReader(PinotDataBuffer dataBuffer) {
     _min = dataBuffer.getLong(offset);
     offset += Long.BYTES;
     _offset = offset;
+    _max = metadata.hasDictionary()

Review comment:
       Suggest making them part of the header so that the index is self-contained. It will be a backward-incompatible change, but since we haven't released it yet, it should be fine.

##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/BitSlicedRangeIndexReader.java
##########
@@ -97,12 +106,22 @@ public ImmutableRoaringBitmap getPartiallyMatchingDocIds(double min, double max)
     return null;
   }
 
-  private ImmutableRoaringBitmap queryRangeBitmap(long min, long max) {
+  private ImmutableRoaringBitmap queryRangeBitmap(long min, long max, long columnMax) {
     RangeBitmap rangeBitmap = mapRangeBitmap();
-    RoaringBitmap lte = rangeBitmap.lte(max);
-    RoaringBitmap gte = rangeBitmap.gte(min);
-    lte.and(gte);
-    return lte.toMutableRoaringBitmap();
+    if (Long.compareUnsigned(max, columnMax) < 0) {
+      RoaringBitmap lte = rangeBitmap.lte(max);
+      if (Long.compareUnsigned(min, 0) > 0) {

Review comment:
       Is this equivalent to `min != 0` (faster)?

##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/BitSlicedRangeIndexReader.java
##########
@@ -23,20 +23,25 @@
 import javax.annotation.Nullable;
 import org.apache.pinot.segment.local.segment.creator.impl.inv.BitSlicedRangeIndexCreator;
 import org.apache.pinot.segment.local.utils.FPOrdering;
+import org.apache.pinot.segment.spi.ColumnMetadata;
 import org.apache.pinot.segment.spi.index.reader.RangeIndexReader;
 import org.apache.pinot.segment.spi.memory.PinotDataBuffer;
 import org.roaringbitmap.RangeBitmap;
 import org.roaringbitmap.RoaringBitmap;
 import org.roaringbitmap.buffer.ImmutableRoaringBitmap;
+import org.roaringbitmap.buffer.MutableRoaringBitmap;
 
 
 public class BitSlicedRangeIndexReader implements RangeIndexReader<ImmutableRoaringBitmap> {
 
   private final PinotDataBuffer _dataBuffer;
   private final long _offset;
   private final long _min;
+  private final long _max;
+  private final long _maxRid;

Review comment:
       (nit) We usually use `_numDocs`. It is currently exclusive, which can possibly cause confusion




-- 
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 #7513: Improved range queries

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



##########
File path: pinot-perf/src/main/java/org/apache/pinot/perf/BenchmarkRangeIndex.java
##########
@@ -196,7 +196,11 @@ public void setup()
     @TearDown(Level.Trial)
     public void tearDown()
         throws IOException {
-      FileUtils.forceDelete(_indexDir);
+      try {
+        FileUtils.forceDelete(_indexDir);

Review comment:
       This was changed in the cleanup last time, but is actually necessary, depending on the OS the tmp file will get cleaned up aggressively so this may or may not throw and may or may not be necessary, but doing this and throwing fails the run.




-- 
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 pull request #7513: Improved range queries

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


   Here's v1 vs v2, master vs branch for INT
   
   ```
   Benchmark                    (_dataType)  (_decile)  (_numDocs)                          (_scenario)  (_seed)  Mode  Cnt     Score     Error  Units
   BenchmarkRangeIndex.queryV1          INT          5     1000000                          NORMAL(0,1)       42  avgt    5  6382.125 ± 155.147  us/op
   BenchmarkRangeIndex.queryV1          INT          5     1000000                NORMAL(10000000,1000)       42  avgt    5  1628.366 ±  61.117  us/op
   BenchmarkRangeIndex.queryV1          INT          5     1000000                          EXP(0.0001)       42  avgt    5  1614.237 ±  44.704  us/op
   BenchmarkRangeIndex.queryV1          INT          5     1000000                             EXP(0.5)       42  avgt    5  3004.145 ± 201.485  us/op
   BenchmarkRangeIndex.queryV1          INT          5     1000000              UNIFORM(0,100000000000)       42  avgt    5  1615.303 ±  50.722  us/op
   BenchmarkRangeIndex.queryV1          INT          5     1000000  UNIFORM(100000000000, 100000000100)       42  avgt    5  1698.668 ±  32.079  us/op
   BenchmarkRangeIndex.queryV2          INT          5     1000000                          NORMAL(0,1)       42  avgt    5   166.180 ±   2.320  us/op
   BenchmarkRangeIndex.queryV2          INT          5     1000000                NORMAL(10000000,1000)       42  avgt    5   589.345 ±  30.562  us/op
   BenchmarkRangeIndex.queryV2          INT          5     1000000                          EXP(0.0001)       42  avgt    5   791.033 ±   2.815  us/op
   BenchmarkRangeIndex.queryV2          INT          5     1000000                             EXP(0.5)       42  avgt    5   359.792 ±   1.857  us/op
   BenchmarkRangeIndex.queryV2          INT          5     1000000              UNIFORM(0,100000000000)       42  avgt    5   889.927 ±  23.994  us/op
   BenchmarkRangeIndex.queryV2          INT          5     1000000  UNIFORM(100000000000, 100000000100)       42  avgt    5   321.739 ±   2.812  us/op
   
   Benchmark                    (_dataType)  (_decile)  (_numDocs)                          (_scenario)  (_seed)  Mode  Cnt    Score    Error  Units
   BenchmarkRangeIndex.queryV2          INT          5     1000000                          NORMAL(0,1)       42  avgt    5  150.122 ± 11.315  us/op
   BenchmarkRangeIndex.queryV2          INT          5     1000000                NORMAL(10000000,1000)       42  avgt    5  551.072 ± 19.352  us/op
   BenchmarkRangeIndex.queryV2          INT          5     1000000                          EXP(0.0001)       42  avgt    5  680.365 ± 27.719  us/op
   BenchmarkRangeIndex.queryV2          INT          5     1000000                             EXP(0.5)       42  avgt    5  337.774 ±  3.527  us/op
   BenchmarkRangeIndex.queryV2          INT          5     1000000              UNIFORM(0,100000000000)       42  avgt    5  905.320 ± 31.135  us/op
   BenchmarkRangeIndex.queryV2          INT          5     1000000  UNIFORM(100000000000, 100000000100)       42  avgt    5  293.619 ± 17.613  us/op
   ```


-- 
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 pull request #7513: Improved range queries

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


   Here's the improvement for the top decile:
   
   ```
   Benchmark                    (_dataType)  (_decile)  (_numDocs)                          (_scenario)  (_seed)  Mode  Cnt    Score    Error  Units
   BenchmarkRangeIndex.queryV2          INT          9     1000000                          NORMAL(0,1)       42  avgt    5  165.554 ±  0.887  us/op
   BenchmarkRangeIndex.queryV2          INT          9     1000000                NORMAL(10000000,1000)       42  avgt    5  623.678 ±  6.921  us/op
   BenchmarkRangeIndex.queryV2          INT          9     1000000                          EXP(0.0001)       42  avgt    5  742.783 ± 16.053  us/op
   BenchmarkRangeIndex.queryV2          INT          9     1000000                             EXP(0.5)       42  avgt    5  336.136 ± 13.360  us/op
   BenchmarkRangeIndex.queryV2          INT          9     1000000              UNIFORM(0,100000000000)       42  avgt    5  827.582 ± 36.197  us/op
   BenchmarkRangeIndex.queryV2          INT          9     1000000  UNIFORM(100000000000, 100000000100)       42  avgt    5  306.778 ±  4.297  us/op
   
   Benchmark                    (_dataType)  (_decile)  (_numDocs)                          (_scenario)  (_seed)  Mode  Cnt    Score    Error  Units
   BenchmarkRangeIndex.queryV2          INT          9     1000000                          NORMAL(0,1)       42  avgt    5  104.768 ±  0.140  us/op
   BenchmarkRangeIndex.queryV2          INT          9     1000000                NORMAL(10000000,1000)       42  avgt    5  282.236 ±  2.902  us/op
   BenchmarkRangeIndex.queryV2          INT          9     1000000                          EXP(0.0001)       42  avgt    5  382.632 ±  1.818  us/op
   BenchmarkRangeIndex.queryV2          INT          9     1000000                             EXP(0.5)       42  avgt    5  164.045 ±  2.375  us/op
   BenchmarkRangeIndex.queryV2          INT          9     1000000              UNIFORM(0,100000000000)       42  avgt    5  466.629 ± 24.464  us/op
   BenchmarkRangeIndex.queryV2          INT          9     1000000  UNIFORM(100000000000, 100000000100)       42  avgt    5  153.618 ±  1.899  us/op
   ```


-- 
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 #7513: Improved range queries

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


   # [Codecov](https://codecov.io/gh/apache/pinot/pull/7513?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 [#7513](https://codecov.io/gh/apache/pinot/pull/7513?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (005f58a) into [master](https://codecov.io/gh/apache/pinot/commit/70374e2be54aeb2bd64d5d9759e5beff62d4c81a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (70374e2) will **decrease** coverage by `2.08%`.
   > The diff coverage is `95.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/pinot/pull/7513/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/7513?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    #7513      +/-   ##
   ============================================
   - Coverage     72.00%   69.91%   -2.09%     
   + Complexity     3404     3331      -73     
   ============================================
     Files          1523     1129     -394     
     Lines         75509    53416   -22093     
     Branches      11004     8047    -2957     
   ============================================
   - Hits          54373    37348   -17025     
   + Misses        17480    13425    -4055     
   + Partials       3656     2643    -1013     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration1 | `?` | |
   | integration2 | `?` | |
   | unittests1 | `69.91% <95.00%> (+0.09%)` | :arrow_up: |
   | 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/7513?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/column/PhysicalColumnIndexContainer.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2NvbHVtbi9QaHlzaWNhbENvbHVtbkluZGV4Q29udGFpbmVyLmphdmE=) | `82.96% <0.00%> (ø)` | |
   | [...gment/index/readers/BitSlicedRangeIndexReader.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvQml0U2xpY2VkUmFuZ2VJbmRleFJlYWRlci5qYXZh) | `81.08% <100.00%> (+7.00%)` | :arrow_up: |
   | [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/7513/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/7513/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: |
   | [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../apache/pinot/common/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...he/pinot/common/messages/SegmentReloadMessage.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWVzc2FnZXMvU2VnbWVudFJlbG9hZE1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...pinot/core/data/manager/realtime/TimerService.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvVGltZXJTZXJ2aWNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...not/common/exception/HttpErrorStatusException.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL0h0dHBFcnJvclN0YXR1c0V4Y2VwdGlvbi5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...t/core/startree/plan/StarTreeDocIdSetPlanNode.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlRG9jSWRTZXRQbGFuTm9kZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | ... and [600 more](https://codecov.io/gh/apache/pinot/pull/7513/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/7513?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/7513?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 [70374e2...005f58a](https://codecov.io/gh/apache/pinot/pull/7513?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] richardstartin commented on a change in pull request #7513: Improved range queries

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



##########
File path: pinot-perf/src/main/java/org/apache/pinot/perf/BenchmarkRangeIndex.java
##########
@@ -196,7 +196,11 @@ public void setup()
     @TearDown(Level.Trial)
     public void tearDown()
         throws IOException {
-      FileUtils.forceDelete(_indexDir);
+      try {
+        FileUtils.forceDelete(_indexDir);

Review comment:
       Yes fine by me




-- 
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 #7513: Improved range queries

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



##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/BitSlicedRangeIndexReader.java
##########
@@ -23,20 +23,25 @@
 import javax.annotation.Nullable;
 import org.apache.pinot.segment.local.segment.creator.impl.inv.BitSlicedRangeIndexCreator;
 import org.apache.pinot.segment.local.utils.FPOrdering;
+import org.apache.pinot.segment.spi.ColumnMetadata;
 import org.apache.pinot.segment.spi.index.reader.RangeIndexReader;
 import org.apache.pinot.segment.spi.memory.PinotDataBuffer;
 import org.roaringbitmap.RangeBitmap;
 import org.roaringbitmap.RoaringBitmap;
 import org.roaringbitmap.buffer.ImmutableRoaringBitmap;
+import org.roaringbitmap.buffer.MutableRoaringBitmap;
 
 
 public class BitSlicedRangeIndexReader implements RangeIndexReader<ImmutableRoaringBitmap> {
 
   private final PinotDataBuffer _dataBuffer;
   private final long _offset;
   private final long _min;
+  private final long _max;
+  private final long _maxRid;

Review comment:
       I will change 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

[GitHub] [pinot] codecov-commenter edited a comment on pull request #7513: Improved range queries

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


   # [Codecov](https://codecov.io/gh/apache/pinot/pull/7513?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 [#7513](https://codecov.io/gh/apache/pinot/pull/7513?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (26749ae) into [master](https://codecov.io/gh/apache/pinot/commit/70374e2be54aeb2bd64d5d9759e5beff62d4c81a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (70374e2) will **decrease** coverage by `39.62%`.
   > The diff coverage is `57.31%`.
   
   > :exclamation: Current head 26749ae differs from pull request most recent head a5776b3. Consider uploading reports for the commit a5776b3 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/pinot/pull/7513/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/7513?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    #7513       +/-   ##
   =============================================
   - Coverage     72.00%   32.38%   -39.63%     
   =============================================
     Files          1523     1514        -9     
     Lines         75509    75270      -239     
     Branches      11004    10987       -17     
   =============================================
   - Hits          54373    24376    -29997     
   - Misses        17480    48783    +31303     
   + Partials       3656     2111     -1545     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration1 | `30.70% <57.31%> (+0.19%)` | :arrow_up: |
   | integration2 | `29.14% <22.56%> (+0.17%)` | :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/7513?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...va/org/apache/pinot/controller/ControllerConf.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9Db250cm9sbGVyQ29uZi5qYXZh) | `50.00% <ø> (-6.92%)` | :arrow_down: |
   | [.../org/apache/pinot/core/common/MinionConstants.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9jb21tb24vTWluaW9uQ29uc3RhbnRzLmphdmE=) | `0.00% <ø> (ø)` | |
   | [.../pinot/core/data/manager/BaseTableDataManager.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvQmFzZVRhYmxlRGF0YU1hbmFnZXIuamF2YQ==) | `68.75% <0.00%> (-17.12%)` | :arrow_down: |
   | [...ent/index/column/PhysicalColumnIndexContainer.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2NvbHVtbi9QaHlzaWNhbENvbHVtbkluZGV4Q29udGFpbmVyLmphdmE=) | `0.00% <0.00%> (-82.97%)` | :arrow_down: |
   | [...gment/index/readers/BitSlicedRangeIndexReader.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvQml0U2xpY2VkUmFuZ2VJbmRleFJlYWRlci5qYXZh) | `0.00% <0.00%> (-74.08%)` | :arrow_down: |
   | [...server/starter/helix/HelixInstanceDataManager.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvc3RhcnRlci9oZWxpeC9IZWxpeEluc3RhbmNlRGF0YU1hbmFnZXIuamF2YQ==) | `84.61% <0.00%> (-0.55%)` | :arrow_down: |
   | [...a/org/apache/pinot/spi/env/PinotConfiguration.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZW52L1Bpbm90Q29uZmlndXJhdGlvbi5qYXZh) | `0.00% <0.00%> (-97.47%)` | :arrow_down: |
   | [...ntroller/helix/core/PinotHelixResourceManager.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90SGVsaXhSZXNvdXJjZU1hbmFnZXIuamF2YQ==) | `46.76% <28.57%> (-18.07%)` | :arrow_down: |
   | [...roker/requesthandler/BaseBrokerRequestHandler.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcmVxdWVzdGhhbmRsZXIvQmFzZUJyb2tlclJlcXVlc3RIYW5kbGVyLmphdmE=) | `65.83% <57.14%> (-5.60%)` | :arrow_down: |
   | [...on/tasks/mergerollup/MergeRollupTaskGenerator.java](https://codecov.io/gh/apache/pinot/pull/7513/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-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvbWVyZ2Vyb2xsdXAvTWVyZ2VSb2xsdXBUYXNrR2VuZXJhdG9yLmphdmE=) | `75.45% <63.88%> (-18.89%)` | :arrow_down: |
   | ... and [1023 more](https://codecov.io/gh/apache/pinot/pull/7513/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/7513?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/7513?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 [70374e2...a5776b3](https://codecov.io/gh/apache/pinot/pull/7513?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] richardstartin commented on pull request #7513: Improved range queries

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


   Flaky test Error:  Failures: 
   34332
   Error:    OfflineClusterIntegrationTest.setUp:183->ClusterTest.uploadSegments:356 » Execution


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