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/05/12 11:38:41 UTC
[GitHub] [pinot] richardstartin opened a new pull request, #8687: deduplicate predicates pessimistically
richardstartin opened a new pull request, #8687:
URL: https://github.com/apache/pinot/pull/8687
Deduplicating predicates frequently shows up in profiles for a range of queries because the hash code of a `Predicate` is both polymorphic and relatively slow to compute.
<img width="883" alt="Screenshot 2022-05-12 at 12 35 17" src="https://user-images.githubusercontent.com/16439049/168065839-5ce5b30a-5a41-47fe-b7e0-b82ed1a61ed4.png">
<img width="889" alt="Screenshot 2022-05-12 at 12 35 37" src="https://user-images.githubusercontent.com/16439049/168065896-d79c08e9-1513-4691-9d87-7caaf046ae52.png">
--
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 merged pull request #8687: deduplicate predicates optimistically
Posted by GitBox <gi...@apache.org>.
richardstartin merged PR #8687:
URL: https://github.com/apache/pinot/pull/8687
--
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 #8687: deduplicate predicates optimistically
Posted by GitBox <gi...@apache.org>.
richardstartin commented on PR #8687:
URL: https://github.com/apache/pinot/pull/8687#issuecomment-1125278231
@Jackie-Jiang lots of comments, I changed the word "Map" to "Mapping" throughout.
--
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 #8687: deduplicate predicates pessimistically
Posted by GitBox <gi...@apache.org>.
richardstartin commented on PR #8687:
URL: https://github.com/apache/pinot/pull/8687#issuecomment-1124926098
This doesn't have a huge impact but given that planning is single threaded, it could be worth it:, though it trades an improvement in the common case for what will be a regression in rare cases
```
Benchmark (_numRows) (_query) (_scenario) Mode Cnt Score Error Units
BenchmarkQueries.query 1500000 SELECT INT_COL, SORTED_COL, SUM(RAW_INT_COL) FROM MyTable WHERE INT_COL = 0 and SORTED_COL = 1 GROUP BY INT_COL, SORTED_COL ORDER BY SORTED_COL, INT_COL ASC EXP(0.001) avgt 5 0.667 ± 0.141 ms/op
BenchmarkQueries.query 1500000 SELECT INT_COL, SORTED_COL, SUM(RAW_INT_COL) FROM MyTable WHERE INT_COL = 0 and SORTED_COL = 1 GROUP BY INT_COL, SORTED_COL ORDER BY SORTED_COL, INT_COL ASC EXP(0.5) avgt 5 0.500 ± 0.255 ms/op
BenchmarkQueries.query 1500000 SELECT INT_COL, SORTED_COL, SUM(RAW_INT_COL) FROM MyTable WHERE INT_COL = 0 and SORTED_COL = 1 GROUP BY INT_COL, SORTED_COL ORDER BY SORTED_COL, INT_COL ASC EXP(0.999) avgt 5 0.828 ± 0.117 ms/op
```
```
Benchmark (_numRows) (_query) (_scenario) Mode Cnt Score Error Units
BenchmarkQueries.query 1500000 SELECT INT_COL, SORTED_COL, SUM(RAW_INT_COL) FROM MyTable WHERE INT_COL = 0 and SORTED_COL = 1 GROUP BY INT_COL, SORTED_COL ORDER BY SORTED_COL, INT_COL ASC EXP(0.001) avgt 5 0.435 ± 0.116 ms/op
BenchmarkQueries.query 1500000 SELECT INT_COL, SORTED_COL, SUM(RAW_INT_COL) FROM MyTable WHERE INT_COL = 0 and SORTED_COL = 1 GROUP BY INT_COL, SORTED_COL ORDER BY SORTED_COL, INT_COL ASC EXP(0.5) avgt 5 0.458 ± 0.238 ms/op
BenchmarkQueries.query 1500000 SELECT INT_COL, SORTED_COL, SUM(RAW_INT_COL) FROM MyTable WHERE INT_COL = 0 and SORTED_COL = 1 GROUP BY INT_COL, SORTED_COL ORDER BY SORTED_COL, INT_COL ASC EXP(0.999) avgt 5 0.598 ± 0.111 ms/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 #8687: deduplicate predicates pessimistically
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #8687:
URL: https://github.com/apache/pinot/pull/8687#issuecomment-1124935557
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8687?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 [#8687](https://codecov.io/gh/apache/pinot/pull/8687?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e6602ed) into [master](https://codecov.io/gh/apache/pinot/commit/f9e55ce7a60fcdfb0fee688059d50af6b6cd112d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f9e55ce) will **decrease** coverage by `1.68%`.
> The diff coverage is `72.72%`.
```diff
@@ Coverage Diff @@
## master #8687 +/- ##
============================================
- Coverage 69.61% 67.92% -1.69%
Complexity 4571 4571
============================================
Files 1722 1722
Lines 89849 89850 +1
Branches 13324 13325 +1
============================================
- Hits 62548 61032 -1516
- Misses 22959 24572 +1613
+ Partials 4342 4246 -96
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `25.25% <72.72%> (+<0.01%)` | :arrow_up: |
| unittests1 | `66.15% <63.63%> (+<0.01%)` | :arrow_up: |
| unittests2 | `14.32% <0.00%> (+<0.01%)` | :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/8687?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...t/core/plan/AggregationGroupByOrderByPlanNode.java](https://codecov.io/gh/apache/pinot/pull/8687/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0FnZ3JlZ2F0aW9uR3JvdXBCeU9yZGVyQnlQbGFuTm9kZS5qYXZh) | `45.45% <0.00%> (-36.37%)` | :arrow_down: |
| [.../org/apache/pinot/core/startree/StarTreeUtils.java](https://codecov.io/gh/apache/pinot/pull/8687/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9TdGFyVHJlZVV0aWxzLmphdmE=) | `56.56% <66.66%> (-12.91%)` | :arrow_down: |
| [...rg/apache/pinot/core/plan/AggregationPlanNode.java](https://codecov.io/gh/apache/pinot/pull/8687/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0FnZ3JlZ2F0aW9uUGxhbk5vZGUuamF2YQ==) | `88.78% <100.00%> (-0.94%)` | :arrow_down: |
| [...ava/org/apache/pinot/core/plan/FilterPlanNode.java](https://codecov.io/gh/apache/pinot/pull/8687/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0ZpbHRlclBsYW5Ob2RlLmphdmE=) | `82.30% <100.00%> (+1.10%)` | :arrow_up: |
| [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/pinot/pull/8687/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: |
| [...nverttorawindex/ConvertToRawIndexTaskExecutor.java](https://codecov.io/gh/apache/pinot/pull/8687/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-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvY29udmVydHRvcmF3aW5kZXgvQ29udmVydFRvUmF3SW5kZXhUYXNrRXhlY3V0b3IuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...e/pinot/common/minion/MergeRollupTaskMetadata.java](https://codecov.io/gh/apache/pinot/pull/8687/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWluaW9uL01lcmdlUm9sbHVwVGFza01ldGFkYXRhLmphdmE=) | `0.00% <0.00%> (-94.74%)` | :arrow_down: |
| [...plugin/segmentuploader/SegmentUploaderDefault.java](https://codecov.io/gh/apache/pinot/pull/8687/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-cGlub3QtcGx1Z2lucy9waW5vdC1zZWdtZW50LXVwbG9hZGVyL3Bpbm90LXNlZ21lbnQtdXBsb2FkZXItZGVmYXVsdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcGx1Z2luL3NlZ21lbnR1cGxvYWRlci9TZWdtZW50VXBsb2FkZXJEZWZhdWx0LmphdmE=) | `0.00% <0.00%> (-87.10%)` | :arrow_down: |
| [.../transform/function/MapValueTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8687/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vTWFwVmFsdWVUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh) | `0.00% <0.00%> (-85.30%)` | :arrow_down: |
| [...ot/common/messages/RoutingTableRebuildMessage.java](https://codecov.io/gh/apache/pinot/pull/8687/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWVzc2FnZXMvUm91dGluZ1RhYmxlUmVidWlsZE1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (-81.82%)` | :arrow_down: |
| ... and [142 more](https://codecov.io/gh/apache/pinot/pull/8687/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/8687?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/8687?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 [f9e55ce...e6602ed](https://codecov.io/gh/apache/pinot/pull/8687?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 diff in pull request #8687: deduplicate predicates optimistically
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on code in PR #8687:
URL: https://github.com/apache/pinot/pull/8687#discussion_r871652667
##########
pinot-core/src/main/java/org/apache/pinot/core/startree/StarTreeUtils.java:
##########
@@ -94,13 +94,13 @@ public static AggregationFunctionColumnPair[] extractAggregationFunctionPairs(
*/
@Nullable
public static Map<String, List<CompositePredicateEvaluator>> extractPredicateEvaluatorsMap(IndexSegment indexSegment,
- @Nullable FilterContext filter, Map<Predicate, PredicateEvaluator> predicateEvaluatorMap) {
+ @Nullable FilterContext filter, List<Pair<Predicate, PredicateEvaluator>> predicateEvaluatorMap) {
Review Comment:
```suggestion
@Nullable FilterContext filter, List<Pair<Predicate, PredicateEvaluator>> predicateEvaluators) {
```
##########
pinot-core/src/main/java/org/apache/pinot/core/startree/StarTreeUtils.java:
##########
@@ -187,7 +187,7 @@ public static boolean isFitForStarTree(StarTreeV2Metadata starTreeV2Metadata,
*/
@Nullable
private static Pair<String, List<PredicateEvaluator>> isOrClauseValidForStarTree(IndexSegment indexSegment,
- FilterContext filter, Map<Predicate, PredicateEvaluator> predicateEvaluatorMap) {
+ FilterContext filter, List<Pair<Predicate, PredicateEvaluator>> predicateEvaluatorMap) {
Review Comment:
```suggestion
FilterContext filter, List<Pair<Predicate, PredicateEvaluator>> predicateEvaluators) {
```
##########
pinot-core/src/main/java/org/apache/pinot/core/plan/FilterPlanNode.java:
##########
@@ -68,7 +67,7 @@ public class FilterPlanNode implements PlanNode {
private final FilterContext _filter;
// Cache the predicate evaluators
- private final Map<Predicate, PredicateEvaluator> _predicateEvaluatorMap = new HashMap<>();
+ private final List<Pair<Predicate, PredicateEvaluator>> _predicateEvaluatorMap = new ArrayList<>();
Review Comment:
```suggestion
private final List<Pair<Predicate, PredicateEvaluator>> _predicateEvaluators = new ArrayList<>();
```
##########
pinot-core/src/main/java/org/apache/pinot/core/plan/FilterPlanNode.java:
##########
@@ -108,7 +107,7 @@ public BaseFilterOperator run() {
/**
* Returns a map from predicates to their evaluators.
Review Comment:
Update the comment
##########
pinot-core/src/main/java/org/apache/pinot/core/startree/StarTreeUtils.java:
##########
@@ -258,7 +258,7 @@ private static boolean extractOrClausePredicates(FilterContext filter, List<Pred
*/
@Nullable
private static PredicateEvaluator getPredicateEvaluator(IndexSegment indexSegment, Predicate predicate,
- Map<Predicate, PredicateEvaluator> predicateEvaluatorMap) {
+ List<Pair<Predicate, PredicateEvaluator>> predicatesAndEvaluators) {
Review Comment:
(nit) for consistency
```suggestion
List<Pair<Predicate, PredicateEvaluator>> predicateEvaluators) {
```
--
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