Partition Pruning - SQL Part [HZ-2515] #24813

ivanthescientist · 2023-06-14T14:48:57Z

Fixes HZ-2515

This reverts commit cdc1f5e.

…ast#22509)"" This reverts commit 3c180fd.

hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/PlanExecutor.java

hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/SqlPlanImpl.java

hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/PlanExecutor.java

...ain/java/com/hazelcast/jet/sql/impl/opt/prunability/PartitionStrategyConditionExtractor.java

...ast-sql/src/main/java/com/hazelcast/jet/sql/impl/opt/metadata/HazelcastRelMdPrunability.java

hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/PlanExecutor.java

Fly-Style

Well, I have nothing to add/fix for MVP version, I am approving this.

hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/PlanExecutor.java

hazelcast/src/main/java/com/hazelcast/jet/impl/execution/init/ExecutionPlanBuilder.java

k-jamroz · 2023-07-17T17:33:13Z

hazelcast/src/main/java/com/hazelcast/jet/impl/execution/init/PartitionArrangement.java

-        }
+        allPartitions = partitionAssignment.values().stream()
+                .flatMapToInt(a -> Arrays.stream(a))
+                .sorted()


@Fly-Style sorted increases complexity from O(N) to O(NlogN) N - number of partitions. Any comments on that? TDD update? Big clusters have lots of partitions and this change impacts every Jet job.

this implementation was quick and simple for demo but also important assumption changes here, allPartitions javadoc has to be updated. Also localPartitions no longer may contain all local partitions. What is the impact of that?

sorted increases complexity from O(N) to O(NlogN) N - number of partitions. Any comments on that?

You wrote this, can't comment it much. I will remove sorted.

Also localPartitions no longer may contain all local partitions.

Nothing wrong happens here from "contain all local partitions" perspective.

You wrote this

this was quick fix for the demo to be reviewed by you if it is correct and makes sense. If you did not change it I assumed that you analyzed it and determined that it is correct - right?

I will remove sorted.

You restored it (BTW, we assume that some arrays are sorted even though it is not documented as guaranteed), so my question is still valid.

It seems to me that you avoided analysis of the impact or other solutions, but at least now it does not increase runtime for non-pruned jobs with large partitions numbers.
I will close this comment when this is described in TDD.

I will close this comment when this is described in TDD

It is not related to design, it's implementation detail. Why is it worth describing it in TDD?

increased complexity is not a small deal (everything > O(N) should be treated with care). When using member pruning with thousands of partitions this could be a place that limits performace. It should be at least mentioned that some code in case of partition pruning has worse than linear runtime.

Signed-off-by: Sasha Syrotenko <oleksandr.syrotenko@hazelcast.com>

…tee order

… case

…runability

…b for interactive queries

…d local member partition key

k-jamroz · 2023-07-20T16:02:55Z

...cast-sql/src/main/java/com/hazelcast/jet/sql/impl/opt/physical/CreateTopLevelDagVisitor.java

@@ -108,6 +108,9 @@ public class CreateTopLevelDagVisitor extends CreateDagVisitorBase<Vertex> {

    private final DagBuildContextImpl dagBuildContext;

+    private Integer requiredRootPartitionId;
+    private Object coordinatorPartitioningKey;


@Fly-Style requiredRootPartitionId and coordinatorPartitioningKey will be embedded in the DAG, so will be part of cached plan. However, when partition migration happens they may no longer belong to local member and the job will crash.
I am afraid that we have to determine this dynamically, we cannot give up SQL plan caching.

k-jamroz

Approved in order to unblock other PRs, however problem with plan caching and migrations has to be fixed.

ivanthescientist and others added 17 commits May 25, 2023 16:40

draft changes

2f61e8b

Merge branch 'master' into partition_pruning_sql_draft

0cd7090

Revert "Remove redundant distribution trait [HZ-1600] (hazelcast#22509)"

3c180fd

This reverts commit cdc1f5e.

draft optimizer changes

4f033aa

WIP : use RelMd queries to detect partition prunability

79da206

Small fixes and optimizer test support for prunability

c9fc96b

draft connect jet to sql

0513d25

WIP

2e565c3

Handle Calc in HzRelMdPrunability

a1e0b9f

naive AND impl

41e7a62

Revert "Revert "Remove redundant distribution trait [HZ-1600] (hazelc…

c3dc0d9

…ast#22509)"" This reverts commit 3c180fd.

checkstyle fix

46e2ab5

Refactoring; enable prunability in Calcite SQL optimizer

8987f45

small fix

dc29866

Merge branch 'master' into partition_pruning_sql_draft

20a8d03

Merge branch 'master' into partition_pruning_sql_draft

598c974

preliminary fix for nested filters case

3deebda

k-jamroz reviewed Jun 28, 2023

View reviewed changes

hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/PlanExecutor.java Outdated Show resolved Hide resolved

k-jamroz reviewed Jun 28, 2023

View reviewed changes

hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/PlanExecutor.java Outdated Show resolved Hide resolved

k-jamroz reviewed Jun 28, 2023

View reviewed changes

hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/PlanExecutor.java Outdated Show resolved Hide resolved

k-jamroz reviewed Jun 28, 2023

View reviewed changes

hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/SqlPlanImpl.java Outdated Show resolved Hide resolved

k-jamroz reviewed Jun 28, 2023

View reviewed changes

hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/PlanExecutor.java Outdated Show resolved Hide resolved

ivanthescientist added 2 commits July 4, 2023 12:39

refactor Prunability return type

c18cd35

refactor plan executor logic

dd44e11

Fly-Style reviewed Jul 4, 2023

View reviewed changes

...ain/java/com/hazelcast/jet/sql/impl/opt/prunability/PartitionStrategyConditionExtractor.java Outdated Show resolved Hide resolved

Fly-Style reviewed Jul 4, 2023

View reviewed changes

...ast-sql/src/main/java/com/hazelcast/jet/sql/impl/opt/metadata/HazelcastRelMdPrunability.java Show resolved Hide resolved

ivanthescientist and others added 3 commits July 5, 2023 14:18

Merge branch 'master' into partition_pruning_sql_draft

e48d4e7

Merge branch 'master' into partition_pruning_sql_draft

f1faf66

Merge branch 'master' into partition_pruning_sql_draft

1c010be

k-jamroz reviewed Jul 6, 2023

View reviewed changes

hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/PlanExecutor.java Outdated Show resolved Hide resolved

hazelcast deleted a comment from hz-devops-test Jul 17, 2023

Fly-Style approved these changes Jul 17, 2023

View reviewed changes

Explain why we translate column names

2f231ce

k-jamroz reviewed Jul 17, 2023

View reviewed changes

hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/PlanExecutor.java Outdated Show resolved Hide resolved

k-jamroz reviewed Jul 17, 2023

View reviewed changes

hazelcast/src/main/java/com/hazelcast/jet/impl/execution/init/ExecutionPlanBuilder.java Outdated Show resolved Hide resolved

k-jamroz reviewed Jul 17, 2023

View reviewed changes

hazelcast deleted a comment from hz-devops-test Jul 17, 2023

Final review comments

96b0110

Signed-off-by: Sasha Syrotenko <oleksandr.syrotenko@hazelcast.com>

hazelcast deleted a comment from hz-devops-test Jul 17, 2023

Revert back sorted() in PartitionArrangement: HashMap does not guaran…

eae05aa

…tee order

hazelcast deleted a comment from hz-devops-test Jul 18, 2023

hazelcast deleted a comment from hz-devops-test Jul 19, 2023

ivanthescientist and others added 5 commits July 19, 2023 05:51

small review fixes

6029916

test scenario for OBJECT key dynamic param comparisons pruning

c644930

disabled member pruning for __key based filters

9128ef6

Use O(N) allPartitions construction algorithm in default non-prunable…

3b523ab

… case

Use specific implementation for each available rel in HazelcastRelMdP…

92065d6

…runability

Fly-Style requested a review from k-jamroz July 19, 2023 12:57

Fly-Style added 2 commits July 19, 2023 16:04

Document temporal solution when coordinator is always added to the jo…

765fc56

…b for interactive queries

Refactor allToOne usages in CreateTopLevelDagVisitor to use compute…

6ef5f31

…d local member partition key

k-jamroz reviewed Jul 20, 2023

View reviewed changes

k-jamroz approved these changes Jul 20, 2023

View reviewed changes

Fly-Style merged commit 2aea4a1 into hazelcast:master Jul 21, 2023
8 checks passed

ivanthescientist deleted the partition_pruning_sql_draft branch July 24, 2023 08:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Partition Pruning - SQL Part [HZ-2515] #24813

Partition Pruning - SQL Part [HZ-2515] #24813

ivanthescientist commented Jun 14, 2023 •

edited by k-jamroz

Fly-Style left a comment

k-jamroz Jul 17, 2023

k-jamroz Jul 17, 2023

Fly-Style Jul 17, 2023

Fly-Style Jul 17, 2023

Fly-Style Jul 17, 2023

k-jamroz Jul 18, 2023

Fly-Style Jul 19, 2023

k-jamroz Jul 19, 2023

Fly-Style Jul 19, 2023

k-jamroz Jul 20, 2023

k-jamroz Jul 20, 2023

k-jamroz left a comment

Partition Pruning - SQL Part [HZ-2515] #24813

Partition Pruning - SQL Part [HZ-2515] #24813

Conversation

ivanthescientist commented Jun 14, 2023 • edited by k-jamroz

Fly-Style left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k-jamroz left a comment

Choose a reason for hiding this comment

ivanthescientist commented Jun 14, 2023 •

edited by k-jamroz