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 2023/01/06 21:58:35 UTC
[GitHub] [pinot] klsince opened a new pull request, #10077: Add an option to cache fields of visited nodes while doing BFS over StarTree index
klsince opened a new pull request, #10077:
URL: https://github.com/apache/pinot/pull/10077
Add a query option `scanStarTreeNodesOnHeap` to turn on/off whether to cache the tree node's fields in object fields when traversing the StarTree index. Caching node's fields can avoid probing the underlying index buffer back and forth when moving from parent to children nodes, thus making reads over the index buffer sequential when doing BFS over the index tree. This helps when data buffer is backed by data on remote storage.
--
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 diff in pull request #10077: Add an option to reduce random access by caching fields of visited nodes when doing BFS over StarTree index
Posted by GitBox <gi...@apache.org>.
richardstartin commented on code in PR #10077:
URL: https://github.com/apache/pinot/pull/10077#discussion_r1064140835
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/startree/OffHeapStarTreeNode.java:
##########
@@ -39,11 +39,13 @@ public class OffHeapStarTreeNode implements StarTreeNode {
private final PinotDataBuffer _dataBuffer;
private final int _nodeId;
private final int _firstChildId;
+ private final int _lastChildId;
Review Comment:
For future reference, in case you haven't used it before, you can actually measure it on Hotspot using [jol](https://mvnrepository.com/artifact/org.openjdk.jol/jol-core/0.16). You just write something like:
```java
System.out.println(ClassLayout.parseClass(StarTreeNode.class).toPrintable());
```
But this takes into account you JVM settings, so if you really want to go above 32GiB heap you'd have to disable compressed oops.
--
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 diff in pull request #10077: Add an option to reduce random access by caching fields of visited nodes when doing BFS over StarTree index
Posted by GitBox <gi...@apache.org>.
klsince commented on code in PR #10077:
URL: https://github.com/apache/pinot/pull/10077#discussion_r1064099460
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/startree/OffHeapStarTreeNode.java:
##########
@@ -39,11 +39,13 @@ public class OffHeapStarTreeNode implements StarTreeNode {
private final PinotDataBuffer _dataBuffer;
private final int _nodeId;
private final int _firstChildId;
+ private final int _lastChildId;
Review Comment:
I counted obj ref as 8, but think I should count it as 4 bytes as UseCompressedOops is used in most cases
--
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 diff in pull request #10077: Add an option to reduce random access by caching fields of visited nodes when doing BFS over StarTree index
Posted by GitBox <gi...@apache.org>.
klsince commented on code in PR #10077:
URL: https://github.com/apache/pinot/pull/10077#discussion_r1063925289
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/startree/OffHeapStarTreeNode.java:
##########
@@ -39,11 +39,13 @@ public class OffHeapStarTreeNode implements StarTreeNode {
private final PinotDataBuffer _dataBuffer;
private final int _nodeId;
private final int _firstChildId;
+ private final int _lastChildId;
Review Comment:
Looks like _lastChildId is needed to keep the abstraction clean to delegate calls from CachedStarTreeNode to OffHeapStarTreeNode. This extra int field doesn't add more overhead considering the object memory layout.
--
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 #10077: Add an option to reduce random access by caching fields of visited nodes when doing BFS over StarTree index
Posted by GitBox <gi...@apache.org>.
richardstartin commented on PR #10077:
URL: https://github.com/apache/pinot/pull/10077#issuecomment-1374421782
Out of interest how do you know this is more disk-friendly (presumably better for block prefetching and fewer page faults)? Have you looked at major faults with sar -B or something like that?
--
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 diff in pull request #10077: Add an option to reduce random access by caching fields of visited nodes when doing BFS over StarTree index
Posted by GitBox <gi...@apache.org>.
richardstartin commented on code in PR #10077:
URL: https://github.com/apache/pinot/pull/10077#discussion_r1063979667
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/startree/OffHeapStarTreeNode.java:
##########
@@ -39,11 +39,13 @@ public class OffHeapStarTreeNode implements StarTreeNode {
private final PinotDataBuffer _dataBuffer;
private final int _nodeId;
private final int _firstChildId;
+ private final int _lastChildId;
Review Comment:
I’m not sure there is any alignment padding for it to fit into, I measured that here: https://github.com/apache/pinot/pull/8570#issue-1209642509
--
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 closed pull request #10077: Add an option to reduce random access by caching fields of visited nodes when doing BFS over StarTree index
Posted by GitBox <gi...@apache.org>.
klsince closed pull request #10077: Add an option to reduce random access by caching fields of visited nodes when doing BFS over StarTree index
URL: https://github.com/apache/pinot/pull/10077
--
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 diff in pull request #10077: Add an option to reduce random access by caching fields of visited nodes when doing BFS over StarTree index
Posted by GitBox <gi...@apache.org>.
klsince commented on code in PR #10077:
URL: https://github.com/apache/pinot/pull/10077#discussion_r1063925350
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java:
##########
@@ -280,6 +280,7 @@ public static class QueryOptionKey {
public static final String SKIP_UPSERT = "skipUpsert";
public static final String USE_STAR_TREE = "useStarTree";
public static final String SCAN_STAR_TREE_NODES = "scanStarTreeNodes";
+ public static final String SCAN_STAR_TREE_NODES_ON_HEAP = "scanStarTreeNodesOnHeap";
Review Comment:
called it reduceRandomAccess to be more descriptive and generic.
--
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 #10077: Add an option to reduce random access by caching fields of visited nodes when doing BFS over StarTree index
Posted by GitBox <gi...@apache.org>.
klsince commented on PR #10077:
URL: https://github.com/apache/pinot/pull/10077#issuecomment-1376693612
> Out of interest how do you know this is more disk-friendly ...
This is a great question! This option is kinda experimental. We're experimenting a ReadAhead mechanism to access segments kept on remote storage via a custom SegmentDirectory object; and found that sequential access pattern would make that a lot more effective. Currently, I don't have stats about how much this option would benefit disk based setup.
--
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 diff in pull request #10077: Add an option to cache fields of visited nodes while doing BFS over StarTree index
Posted by GitBox <gi...@apache.org>.
klsince commented on code in PR #10077:
URL: https://github.com/apache/pinot/pull/10077#discussion_r1063863732
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java:
##########
@@ -280,6 +280,7 @@ public static class QueryOptionKey {
public static final String SKIP_UPSERT = "skipUpsert";
public static final String USE_STAR_TREE = "useStarTree";
public static final String SCAN_STAR_TREE_NODES = "scanStarTreeNodes";
+ public static final String SCAN_STAR_TREE_NODES_ON_HEAP = "scanStarTreeNodesOnHeap";
Review Comment:
maybe just the `scanStarTreeNodes` option to indicate the reads are moving forward (which indicates the use of cache to avoid looking back)
--
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 #10077: Add an option to reduce random access by caching fields of visited nodes when doing BFS over StarTree index
Posted by GitBox <gi...@apache.org>.
richardstartin commented on PR #10077:
URL: https://github.com/apache/pinot/pull/10077#issuecomment-1374422662
If #8570 had negative impacts for certain disk setups, I’d suggest just reverting the startreeenode changes there, rather than making good performance opt in in what could be or (end up being) a common scenario.
--
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 #10077: Add an option to cache fields of visited nodes while doing BFS over StarTree index
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #10077:
URL: https://github.com/apache/pinot/pull/10077#issuecomment-1374228184
# [Codecov](https://codecov.io/gh/apache/pinot/pull/10077?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 [#10077](https://codecov.io/gh/apache/pinot/pull/10077?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (270200a) into [master](https://codecov.io/gh/apache/pinot/commit/04d09e5986c9fe1c69c3001549bc0e0be43b3a92?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (04d09e5) will **increase** coverage by `40.70%`.
> The diff coverage is `79.31%`.
```diff
@@ Coverage Diff @@
## master #10077 +/- ##
=============================================
+ Coverage 27.87% 68.58% +40.70%
- Complexity 53 5694 +5641
=============================================
Files 1979 1992 +13
Lines 107281 107661 +380
Branches 16323 16362 +39
=============================================
+ Hits 29909 73839 +43930
+ Misses 74417 28575 -45842
- Partials 2955 5247 +2292
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `24.53% <3.44%> (-0.11%)` | :arrow_down: |
| integration2 | `?` | |
| unittests1 | `68.00% <79.31%> (?)` | |
| unittests2 | `13.63% <0.00%> (?)` | |
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/10077?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...core/startree/operator/StarTreeFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/10077?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9vcGVyYXRvci9TdGFyVHJlZUZpbHRlck9wZXJhdG9yLmphdmE=) | `86.36% <0.00%> (-0.48%)` | :arrow_down: |
| [...va/org/apache/pinot/spi/utils/CommonConstants.java](https://codecov.io/gh/apache/pinot/pull/10077?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `22.78% <ø> (+22.78%)` | :arrow_up: |
| [...not/segment/local/startree/CachedStarTreeNode.java](https://codecov.io/gh/apache/pinot/pull/10077?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS9DYWNoZWRTdGFyVHJlZU5vZGUuamF2YQ==) | `83.33% <83.33%> (ø)` | |
| [...e/pinot/common/utils/config/QueryOptionsUtils.java](https://codecov.io/gh/apache/pinot/pull/10077?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvY29uZmlnL1F1ZXJ5T3B0aW9uc1V0aWxzLmphdmE=) | `69.23% <100.00%> (+0.60%)` | :arrow_up: |
| [...ot/segment/local/startree/OffHeapStarTreeNode.java](https://codecov.io/gh/apache/pinot/pull/10077?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS9PZmZIZWFwU3RhclRyZWVOb2RlLmphdmE=) | `65.90% <100.00%> (+65.90%)` | :arrow_up: |
| [...pinot/core/data/manager/realtime/TimerService.java](https://codecov.io/gh/apache/pinot/pull/10077?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: |
| [...t/core/plan/StreamingInstanceResponsePlanNode.java](https://codecov.io/gh/apache/pinot/pull/10077?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL1N0cmVhbWluZ0luc3RhbmNlUmVzcG9uc2VQbGFuTm9kZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ore/operator/streaming/StreamingResponseUtils.java](https://codecov.io/gh/apache/pinot/pull/10077?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9zdHJlYW1pbmcvU3RyZWFtaW5nUmVzcG9uc2VVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ager/realtime/PeerSchemeSplitSegmentCommitter.java](https://codecov.io/gh/apache/pinot/pull/10077?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUGVlclNjaGVtZVNwbGl0U2VnbWVudENvbW1pdHRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...he/pinot/common/utils/grpc/GrpcRequestBuilder.java](https://codecov.io/gh/apache/pinot/pull/10077?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvZ3JwYy9HcnBjUmVxdWVzdEJ1aWxkZXIuamF2YQ==) | `0.00% <0.00%> (-90.91%)` | :arrow_down: |
| ... and [1446 more](https://codecov.io/gh/apache/pinot/pull/10077?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?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 diff in pull request #10077: Add an option to cache fields of visited nodes while doing BFS over StarTree index
Posted by GitBox <gi...@apache.org>.
klsince commented on code in PR #10077:
URL: https://github.com/apache/pinot/pull/10077#discussion_r1063862763
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/startree/OffHeapStarTreeNode.java:
##########
@@ -39,11 +39,13 @@ public class OffHeapStarTreeNode implements StarTreeNode {
private final PinotDataBuffer _dataBuffer;
private final int _nodeId;
private final int _firstChildId;
+ private final int _lastChildId;
Review Comment:
this can be avoided, by creating a new iterator in the CachedStarTreeNode instead of delegating to OffHeapStarTreeNode's iterator, when it reads lastChildId but that's already too late.
--
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 diff in pull request #10077: Add an option to cache fields of visited nodes while doing BFS over StarTree index
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on code in PR #10077:
URL: https://github.com/apache/pinot/pull/10077#discussion_r1063842531
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/startree/CachedStarTreeNode.java:
##########
@@ -0,0 +1,114 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.segment.local.startree;
+
+import java.util.Iterator;
+import org.apache.pinot.segment.spi.index.startree.StarTreeNode;
+
+
+/**
+ * Getting all fields in current StarTree node immediately and cache them, instead of jumping back and forth inside the
+ * data buffer to read fields from parent and children nodes while doing BFS over the StarTree index tree, making the
+ * buffer reads a lot more sequential.
+ */
+public class CachedStarTreeNode implements StarTreeNode {
+ private final StarTreeNode _delegate;
+ private final int _dimensionId;
+ private final int _dimensionValue;
+ private final int _startDocId;
+ private final int _endDocId;
+ private final int _aggregatedDocId;
+ private final int _numChildren;
+
+ public CachedStarTreeNode(StarTreeNode starTreeNode) {
+ _delegate = starTreeNode;
+ _dimensionId = _delegate.getDimensionId();
+ _dimensionValue = _delegate.getDimensionValue();
+ _startDocId = _delegate.getStartDocId();
+ _endDocId = _delegate.getEndDocId();
+ _aggregatedDocId = _delegate.getAggregatedDocId();
+ _numChildren = _delegate.getNumChildren();
+ }
+
+ @Override
+ public int getDimensionId() {
+ return _dimensionId;
+ }
+
+ @Override
+ public int getDimensionValue() {
+ return _dimensionValue;
+ }
+
+ @Override
+ public int getChildDimensionId() {
+ return _delegate.getChildDimensionId();
+ }
+
+ @Override
+ public int getStartDocId() {
+ return _startDocId;
+ }
+
+ @Override
+ public int getEndDocId() {
+ return _endDocId;
+ }
+
+ @Override
+ public int getAggregatedDocId() {
+ return _aggregatedDocId;
+ }
+
+ @Override
+ public int getNumChildren() {
+ return _numChildren;
+ }
+
+ @Override
+ public boolean isLeaf() {
+ return _delegate.isLeaf();
+ }
+
+ @Override
+ public StarTreeNode getChildForDimensionValue(int dimensionValue) {
+ return new CachedStarTreeNode(_delegate.getChildForDimensionValue(dimensionValue));
Review Comment:
Need to check if `_delegate.getChildForDimensionValue(dimensionValue)` is `null`
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/startree/OffHeapStarTreeNode.java:
##########
@@ -39,11 +39,13 @@ public class OffHeapStarTreeNode implements StarTreeNode {
private final PinotDataBuffer _dataBuffer;
private final int _nodeId;
private final int _firstChildId;
+ private final int _lastChildId;
Review Comment:
Is this change required? This will add extra memory footprint to the star-tree node
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java:
##########
@@ -280,6 +280,7 @@ public static class QueryOptionKey {
public static final String SKIP_UPSERT = "skipUpsert";
public static final String USE_STAR_TREE = "useStarTree";
public static final String SCAN_STAR_TREE_NODES = "scanStarTreeNodes";
+ public static final String SCAN_STAR_TREE_NODES_ON_HEAP = "scanStarTreeNodesOnHeap";
Review Comment:
Should we come up with one single query option for the data locality based optimizations?
--
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