Add recording distribution of waitingSplits queue size by radek-kondziolka · Pull Request #13453 · trinodb/trino

radek-kondziolka · 2022-08-02T08:47:21Z

In some case we need to know what is the saturation level of workers from the coordinator point of view.
The one metric that would be helpful to know is size of the TaskExecutor.waitingSplits queue. This is why we want to sample distribution of its length.

skrzypo987

Looks good % stuff related to waiting for airlift release.
Please ping me when the release is done

core/trino-main/src/main/java/io/trino/execution/executor/MultilevelSplitQueue.java

radek-kondziolka · 2022-08-10T06:16:47Z

@skrzypo987 , release is done

skrzypo987 · 2022-08-10T06:21:03Z

@skrzypo987 , release is done

Please restart the CI (rebase to current master)

lukasz-stec

lgtm % comments

core/trino-main/src/main/java/io/trino/execution/executor/MultilevelSplitQueue.java

lukasz-stec

lgtm

core/trino-main/src/test/java/io/trino/execution/executor/TestTaskExecutor.java

core/trino-main/src/main/java/io/trino/execution/executor/TaskExecutor.java

sopel39 · 2022-08-11T16:06:31Z

@radek-starburst could you show example of the stat?

radek-kondziolka · 2022-08-12T09:32:01Z

@radek-starburst could you show example of the stat?
@sopel39 , yes sure. I've run concurrency benchmark with:

32 concurrent queries:

AllSplitSizeDistribution.AllTime.P01=0.9
AllSplitSizeDistribution.AllTime.P05=2.28
AllSplitSizeDistribution.AllTime.P10=166.22
AllSplitSizeDistribution.AllTime.P50=3490.76
AllSplitSizeDistribution.AllTime.P90=11438.89
AllSplitSizeDistribution.AllTime.P95=13756
AllSplitSizeDistribution.AllTime.P99=19254
AllSplitSizeDistribution.AllTime.Avg=4801.80

64 concurrent queries:

AllSplitSizeDistribution.AllTime.P01=0.30
AllSplitSizeDistribution.AllTime.P05=36.38
AllSplitSizeDistribution.AllTime.P10=330.88
AllSplitSizeDistribution.AllTime.P50=7988.55
AllSplitSizeDistribution.AllTime.P90=20937.60
AllSplitSizeDistribution.AllTime.P95=25026.22
AllSplitSizeDistribution.AllTime.P99=33483.939
AllSplitSizeDistribution.AllTime.Avg=9409.70

It looks like it works. Do not merge, I wait for benchmarks result to avoid an unintended regression.

radek-kondziolka · 2022-08-17T09:05:26Z

TPCH duration: -0.55 
TPCDS duration:  -2.51
TPCH CPU: 0.78 
TPCDS CPU: 0.02

No visible "stats stuff" in the top (> 0.17%) of JFR profile. I think it is negligible.

sopel39

lgtm % comments

core/trino-main/src/main/java/io/trino/execution/executor/TaskExecutor.java

sopel39

lgtm % comments % test failing

core/trino-main/src/main/java/io/trino/execution/executor/TaskExecutor.java

core/trino-main/src/test/java/io/trino/execution/executor/TestTaskExecutor.java

core/trino-main/src/main/java/io/trino/execution/executor/TaskExecutor.java

sopel39 · 2022-08-24T17:06:44Z

core/trino-main/src/main/java/io/trino/execution/executor/TaskExecutor.java

getLeafSplitsSize

A sampling of TaskExecutor.waitingSplits.size's distribution was added and exposed via JMX

sopel39 · 2022-08-25T14:11:57Z

core/trino-main/src/main/java/io/trino/execution/executor/TaskExecutor.java

@@ -21,6 +21,7 @@
 import io.airlift.concurrent.ThreadPoolExecutorMBean;


nit: commit message is incorrect

cla-bot bot added the cla-signed label Aug 2, 2022

radek-kondziolka force-pushed the rk/add_waiting_split_queue_distribution_size branch 3 times, most recently from 95f187b to 9474776 Compare August 2, 2022 09:52

radek-kondziolka requested review from raunaqmorarka and skrzypo987 August 2, 2022 09:52

sopel39 marked this pull request as ready for review August 2, 2022 10:25

sopel39 added the performance label Aug 2, 2022

sopel39 requested a review from lukasz-stec August 2, 2022 10:32

skrzypo987 reviewed Aug 3, 2022

View reviewed changes

radek-kondziolka force-pushed the rk/add_waiting_split_queue_distribution_size branch from 9474776 to 5cc3549 Compare August 3, 2022 06:41

lukasz-stec reviewed Aug 5, 2022

View reviewed changes

core/trino-main/src/main/java/io/trino/execution/executor/MultilevelSplitQueue.java Outdated Show resolved Hide resolved

core/trino-main/src/main/java/io/trino/execution/executor/MultilevelSplitQueue.java Outdated Show resolved Hide resolved

radek-kondziolka force-pushed the rk/add_waiting_split_queue_distribution_size branch from 5cc3549 to 34029df Compare August 9, 2022 11:29

radek-kondziolka requested review from lukasz-stec and skrzypo987 August 10, 2022 06:16

lukasz-stec reviewed Aug 10, 2022

View reviewed changes

core/trino-main/src/main/java/io/trino/execution/executor/MultilevelSplitQueue.java Outdated Show resolved Hide resolved

core/trino-main/src/main/java/io/trino/execution/executor/MultilevelSplitQueue.java Outdated Show resolved Hide resolved

radek-kondziolka force-pushed the rk/add_waiting_split_queue_distribution_size branch 2 times, most recently from c436c96 to cdb798e Compare August 11, 2022 07:19

radek-kondziolka requested a review from lukasz-stec August 11, 2022 07:22

radek-kondziolka force-pushed the rk/add_waiting_split_queue_distribution_size branch from cdb798e to c8989c3 Compare August 11, 2022 07:24

lukasz-stec approved these changes Aug 11, 2022

View reviewed changes

core/trino-main/src/test/java/io/trino/execution/executor/TestTaskExecutor.java Outdated Show resolved Hide resolved

radek-kondziolka force-pushed the rk/add_waiting_split_queue_distribution_size branch from c8989c3 to d12a397 Compare August 11, 2022 09:11

sopel39 reviewed Aug 11, 2022

View reviewed changes

core/trino-main/src/main/java/io/trino/execution/executor/TaskExecutor.java Outdated Show resolved Hide resolved

radek-kondziolka force-pushed the rk/add_waiting_split_queue_distribution_size branch from d12a397 to 799824e Compare August 11, 2022 15:28

radek-kondziolka requested a review from sopel39 August 11, 2022 15:48

sopel39 reviewed Aug 11, 2022

View reviewed changes

core/trino-main/src/main/java/io/trino/execution/executor/TaskExecutor.java Outdated Show resolved Hide resolved

skrzypo987 approved these changes Aug 11, 2022

View reviewed changes

radek-kondziolka force-pushed the rk/add_waiting_split_queue_distribution_size branch from 799824e to b3cfdfe Compare August 12, 2022 09:22

sopel39 reviewed Aug 17, 2022

View reviewed changes

radek-kondziolka force-pushed the rk/add_waiting_split_queue_distribution_size branch from b3cfdfe to c0a1bf3 Compare August 19, 2022 10:45

radek-kondziolka requested a review from sopel39 August 19, 2022 10:46

sopel39 reviewed Aug 19, 2022

View reviewed changes

radek-kondziolka force-pushed the rk/add_waiting_split_queue_distribution_size branch from c0a1bf3 to b38dbb3 Compare August 19, 2022 11:49

radek-kondziolka requested a review from sopel39 August 19, 2022 11:50

sopel39 approved these changes Aug 23, 2022

View reviewed changes

radek-kondziolka force-pushed the rk/add_waiting_split_queue_distribution_size branch from b38dbb3 to 5e01f29 Compare August 23, 2022 10:50

radek-kondziolka requested a review from sopel39 August 23, 2022 10:54

sopel39 reviewed Aug 23, 2022

View reviewed changes

core/trino-main/src/main/java/io/trino/execution/executor/TaskExecutor.java Outdated Show resolved Hide resolved

radek-kondziolka force-pushed the rk/add_waiting_split_queue_distribution_size branch from 5e01f29 to 2b92ab9 Compare August 23, 2022 12:18

sopel39 approved these changes Aug 23, 2022

View reviewed changes

core/trino-main/src/main/java/io/trino/execution/executor/TaskExecutor.java Outdated Show resolved Hide resolved

core/trino-main/src/main/java/io/trino/execution/executor/TaskExecutor.java Outdated Show resolved Hide resolved

radek-kondziolka force-pushed the rk/add_waiting_split_queue_distribution_size branch from 2b92ab9 to 703b423 Compare August 23, 2022 16:02

martint reviewed Aug 23, 2022

View reviewed changes

core/trino-main/src/main/java/io/trino/execution/executor/TaskExecutor.java Outdated Show resolved Hide resolved

radek-kondziolka force-pushed the rk/add_waiting_split_queue_distribution_size branch from 703b423 to b11f027 Compare August 24, 2022 06:26

sopel39 reviewed Aug 24, 2022

View reviewed changes

Add recording distribution of waitingSplits queue size

5d0e7a8

A sampling of TaskExecutor.waitingSplits.size's distribution was added and exposed via JMX

radek-kondziolka force-pushed the rk/add_waiting_split_queue_distribution_size branch from b11f027 to 5d0e7a8 Compare August 25, 2022 07:07

sopel39 approved these changes Aug 25, 2022

View reviewed changes

sopel39 merged commit 473e2fd into trinodb:master Aug 25, 2022

github-actions bot added this to the 394 milestone Aug 25, 2022

colebow mentioned this pull request Aug 29, 2022

Add Trino 394 release notes #13813

Merged

		@@ -21,6 +21,7 @@
		import io.airlift.concurrent.ThreadPoolExecutorMBean;

Conversation

radek-kondziolka commented Aug 2, 2022

Uh oh!

skrzypo987 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

radek-kondziolka commented Aug 10, 2022

Uh oh!

skrzypo987 commented Aug 10, 2022

Uh oh!

lukasz-stec left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lukasz-stec left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sopel39 commented Aug 11, 2022

Uh oh!

radek-kondziolka commented Aug 12, 2022

Uh oh!

radek-kondziolka commented Aug 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sopel39 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sopel39 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sopel39 Aug 24, 2022

Choose a reason for hiding this comment

Uh oh!

sopel39 Aug 25, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

5 participants

radek-kondziolka commented Aug 17, 2022 •

edited

Loading