Send dynamic filters from coordinator to workers #5183

raunaqmorarka · 2020-09-16T08:27:46Z

Fixes #3972

presto-main/src/main/java/io/prestosql/server/remotetask/HttpRemoteTask.java

core/trino-main/src/main/java/io/trino/server/TaskUpdateRequest.java

core/trino-main/src/main/java/io/trino/execution/RemoteTaskFactory.java

core/trino-main/src/main/java/io/trino/execution/SqlStageExecution.java

core/trino-main/src/main/java/io/trino/execution/scheduler/SqlQueryScheduler.java

core/trino-main/src/main/java/io/trino/operator/TaskContext.java

sopel39 · 2021-04-14T14:59:54Z

This also requires:

a feature toggle since now coordinator could broadcast more data
stats to track request sizes (see: io.trino.server.remotetask.RemoteTaskStats#updateWithPlanBytes) and possibly others

Regarding #5183 (comment), I'm leaning towards sending DF updates via TaskUpdateRequest only (without dedicated service), but it would be good to get second opinion from @dain about it.

core/trino-main/src/main/java/io/trino/server/remotetask/HttpRemoteTask.java

core/trino-main/src/main/java/io/trino/SystemSessionProperties.java

sopel39 · 2021-07-01T20:25:03Z

core/trino-main/src/main/java/io/trino/execution/SqlStageExecution.java

@@ -167,6 +174,12 @@ private SqlStageExecution(
            }
        }
        this.exchangeSources = fragmentToExchangeSource.build();
+        if (distributeDynamicFilters) {


Don't we need to check if DF are enabled in general too?

If DF is disabled, then the plan won't have any DFs and getOutboundDynamicFilters should come back empty

core/trino-main/src/main/java/io/trino/operator/TaskContext.java

core/trino-main/src/main/java/io/trino/sql/planner/LocalExecutionPlanner.java

core/trino-main/src/main/java/io/trino/server/DynamicFilterService.java

core/trino-main/src/main/java/io/trino/server/remotetask/HttpRemoteTask.java

core/trino-main/src/main/java/io/trino/sql/planner/LocalExecutionPlanner.java

sopel39 · 2021-07-02T10:06:11Z

core/trino-main/src/main/java/io/trino/server/remotetask/HttpRemoteTask.java

@@ -520,7 +520,8 @@ private synchronized void sendUpdate()
        }


nit: would it be possible to add test to TestHttpRemoteTask?

Added some testing around sending of DFs in TestHttpRemoteTask
I didn't see a reasonable way to test the race condition though

core/trino-main/src/main/java/io/trino/server/remotetask/HttpRemoteTask.java

core/trino-main/src/main/java/io/trino/execution/SqlStageExecution.java

core/trino-main/src/test/java/io/trino/server/TestDynamicFilterService.java

plugin/trino-memory/src/test/java/io/trino/plugin/memory/TestMemorySmoke.java

testing/trino-tests/src/test/java/io/trino/execution/TestCoordinatorDynamicFiltering.java

sopel39

lgtm % test comments

testing/trino-tests/src/test/java/io/trino/execution/TestCoordinatorDynamicFiltering.java

sopel39

lgtm % test comments

core/trino-main/src/main/java/io/trino/server/remotetask/HttpRemoteTask.java

core/trino-main/src/test/java/io/trino/server/TestDynamicFilterService.java

core/trino-main/src/test/java/io/trino/server/remotetask/TestHttpRemoteTask.java

testing/trino-tests/src/test/java/io/trino/execution/TestCoordinatorDynamicFiltering.java

sopel39

small comments

core/trino-main/src/test/java/io/trino/server/remotetask/TestHttpRemoteTask.java

Explicitly disable optimizer.rewrite-filtering-semi-join-to-inner-join in dynamic filtering tests to ensure that the semi-join related tests actually test the code path of semi-join dynamic filtering.

cla-bot bot added the cla-signed label Sep 16, 2020

raunaqmorarka commented Sep 16, 2020

View reviewed changes

presto-main/src/main/java/io/prestosql/server/remotetask/HttpRemoteTask.java Outdated Show resolved Hide resolved

raunaqmorarka force-pushed the df_stripe branch 4 times, most recently from 82aa582 to ae8b3e6 Compare September 17, 2020 16:36

raunaqmorarka force-pushed the df_stripe branch from ae8b3e6 to 49f8dd7 Compare October 11, 2020 10:14

raunaqmorarka force-pushed the df_stripe branch 4 times, most recently from abd7dff to 2dbdbb5 Compare October 21, 2020 18:10

raunaqmorarka force-pushed the df_stripe branch 4 times, most recently from 51bd66f to d422648 Compare February 1, 2021 20:37

joshthoward requested a review from martint April 5, 2021 20:26

raunaqmorarka force-pushed the df_stripe branch from d422648 to f0bafbd Compare April 9, 2021 19:06

sopel39 reviewed Apr 14, 2021

View reviewed changes

core/trino-main/src/main/java/io/trino/server/remotetask/HttpRemoteTask.java Outdated Show resolved Hide resolved

raunaqmorarka force-pushed the df_stripe branch from f0bafbd to 3e230b9 Compare April 15, 2021 07:19

raunaqmorarka force-pushed the df_stripe branch 6 times, most recently from 1f1651f to 0d122bd Compare June 23, 2021 14:45

raunaqmorarka changed the title ~~[WIP] Send dynamic filters from coordinator to workers~~ Send dynamic filters from coordinator to workers Jun 23, 2021

raunaqmorarka requested review from sopel39 and dain June 23, 2021 14:53

sopel39 reviewed Jul 1, 2021

View reviewed changes

raunaqmorarka force-pushed the df_stripe branch 2 times, most recently from f9c5234 to 37b451c Compare July 2, 2021 08:11

raunaqmorarka requested a review from sopel39 July 2, 2021 08:11

sopel39 reviewed Jul 2, 2021

View reviewed changes

testing/trino-tests/src/test/java/io/trino/execution/TestCoordinatorDynamicFiltering.java Show resolved Hide resolved

sopel39 reviewed Jul 2, 2021

View reviewed changes

raunaqmorarka force-pushed the df_stripe branch 3 times, most recently from 115618f to ace437b Compare July 2, 2021 16:17

raunaqmorarka requested a review from sopel39 July 2, 2021 16:23

raunaqmorarka force-pushed the df_stripe branch 2 times, most recently from d70f0c4 to 0dbd119 Compare July 5, 2021 09:00

sopel39 reviewed Jul 5, 2021

View reviewed changes

raunaqmorarka force-pushed the df_stripe branch from 0dbd119 to f29fecd Compare July 5, 2021 11:26

raunaqmorarka requested a review from sopel39 July 5, 2021 11:29

sopel39 approved these changes Jul 6, 2021

View reviewed changes

raunaqmorarka force-pushed the df_stripe branch from f29fecd to ad61982 Compare July 6, 2021 10:22

raunaqmorarka added 5 commits July 8, 2021 16:44

Move LocalDynamicFiltersCollector to TaskContext

12fa584

Ensure semijoins dynamic filtering is covered by tests

309ad20

Explicitly disable optimizer.rewrite-filtering-semi-join-to-inner-join in dynamic filtering tests to ensure that the semi-join related tests actually test the code path of semi-join dynamic filtering.

Move testJoinDynamicFilteringMultiJoin above helper methods

31dbd1b

Extract method HttpRemoteTask#triggerUpdate

bd3e1cf

Send dynamic filters from coordinator to workers

d7675e2

raunaqmorarka force-pushed the df_stripe branch from ad61982 to d7675e2 Compare July 8, 2021 11:14

sopel39 merged commit 7854abc into trinodb:master Jul 9, 2021

sopel39 mentioned this pull request Jul 9, 2021

Release notes for 360 #8455

Closed

11 tasks

raunaqmorarka deleted the df_stripe branch July 9, 2021 11:23

hashhar mentioned this pull request Dec 31, 2021

Add 2021 year in review post trinodb/trino.io#266

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Send dynamic filters from coordinator to workers #5183

Send dynamic filters from coordinator to workers #5183

raunaqmorarka commented Sep 16, 2020 •

edited

sopel39 commented Apr 14, 2021 •

edited

sopel39 Jul 1, 2021

raunaqmorarka Jul 2, 2021

sopel39 Jul 2, 2021

raunaqmorarka Jul 2, 2021

sopel39 left a comment

sopel39 left a comment

sopel39 left a comment

Send dynamic filters from coordinator to workers #5183

Send dynamic filters from coordinator to workers #5183

Conversation

raunaqmorarka commented Sep 16, 2020 • edited

sopel39 commented Apr 14, 2021 • edited

sopel39 Jul 1, 2021

Choose a reason for hiding this comment

raunaqmorarka Jul 2, 2021

Choose a reason for hiding this comment

sopel39 Jul 2, 2021

Choose a reason for hiding this comment

raunaqmorarka Jul 2, 2021

Choose a reason for hiding this comment

sopel39 left a comment

Choose a reason for hiding this comment

sopel39 left a comment

Choose a reason for hiding this comment

sopel39 left a comment

Choose a reason for hiding this comment

raunaqmorarka commented Sep 16, 2020 •

edited

sopel39 commented Apr 14, 2021 •

edited