Report total active threads by edman124 · Pull Request #28513 · apache/beam

edman124 · 2023-09-18T23:32:30Z

These metrics are needed as a part of an internal project for thread scaling.
Total number of active threads will be joined with worker cpu utilization to see if we are bounded by the # of parallel work items, ultimately to reduce worker idleness.

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
Update CHANGES.md with noteworthy changes.
If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.

edman124 · 2023-09-18T23:42:44Z

ptal @twang126

twang126

pls take a quick look and undo the changes that are just formatting, etc. (there's more than just the parts I commented at)

...va/worker/src/main/java/org/apache/beam/runners/dataflow/worker/StreamingDataflowWorker.java

...orker/src/test/java/org/apache/beam/runners/dataflow/worker/StreamingDataflowWorkerTest.java

github-actions · 2023-09-19T01:50:35Z

Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment assign set of reviewers

edman124 · 2023-09-20T17:13:19Z

Run Java PreCommit

...orker/src/test/java/org/apache/beam/runners/dataflow/worker/StreamingDataflowWorkerTest.java

github-actions · 2023-09-20T21:34:46Z

Assigning reviewers. If you would like to opt out of this review, comment assign to next reviewer:

R: @damccorm added as fallback since no labels match configuration

Available commands:

stop reviewer notifications - opt out of the automated review tooling
remind me after tests pass - tag the comment author after tests pass
waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

edman124 · 2023-09-26T21:49:38Z

ptal @bvolpato

...va/worker/src/main/java/org/apache/beam/runners/dataflow/worker/StreamingDataflowWorker.java

bvolpato

In general LGTM, shared suggestion to make max active threads more useful

edman124 · 2023-09-27T21:04:44Z

ptal @damccorm should be ready to merge.

...orker/src/test/java/org/apache/beam/runners/dataflow/worker/StreamingDataflowWorkerTest.java

bvolpato

LGTM, just get rid of some debugging / personal information if possible, please

bvolpato · 2023-09-28T18:56:54Z

Please ./gradlew spotlessApply

damccorm · 2023-09-29T14:46:39Z

...orker/src/test/java/org/apache/beam/runners/dataflow/worker/StreamingDataflowWorkerTest.java

+    ShardedKey key1Shard1 = ShardedKey.create(ByteString.copyFromUtf8("key1"), 1);
+
+    // real work
+    MockActiveWork m1 =


The test is now failing to build, please address the precommit failures (e.g. https://github.com/apache/beam/actions/runs/6344373926/job/17234456430?pr=28513)

damccorm · 2023-10-02T13:13:21Z

Run Java PreCommit

damccorm

Thanks

Abacn · 2023-10-19T16:02:58Z

...orker/src/test/java/org/apache/beam/runners/dataflow/worker/StreamingDataflowWorkerTest.java

+      executor.execute(m2, m2.getWorkItem().getSerializedSize());
+      this.wait();
+      // Seems current executor executes the initial work item twice
+      this.wait();


this may have caused #28957

It could be the first thread started runs before the synchronized block starts causing a deadlock. I have a fix in #29041 that uses countdown latches.

Thanks for the fix. While #29041 is in progress, would you mind lgtm #29065 so the current stuck test won't affect the whole test suite so that it can still finish?

edman124 added 4 commits September 15, 2023 22:37

New active threads metric and initial tests

d333918

create unit tests for max active threads metric

fa671b2

remove test filter

6b55b9f

fix formatting with spotless apply

7e60a26

github-actions bot added runners dataflow labels Sep 18, 2023

edman124 marked this pull request as draft September 18, 2023 23:36

edman124 marked this pull request as ready for review September 18, 2023 23:37

twang126 reviewed Sep 18, 2023

View reviewed changes

edman124 added 3 commits September 19, 2023 00:06

revert format changes

eab8eed

revert format change straggler

cf3bc0f

revert format change straggler

9e65c40

remove unnecessary comment

b6c6d20

twang126 reviewed Sep 20, 2023

View reviewed changes

...orker/src/test/java/org/apache/beam/runners/dataflow/worker/StreamingDataflowWorkerTest.java Show resolved Hide resolved

twang126 reviewed Sep 20, 2023

View reviewed changes

Merge branch 'apache:master' into master

e194aeb

github-actions bot added the Next Action: Reviewers label Sep 20, 2023

edman124 added 2 commits September 21, 2023 23:05

synchronize threads in unit tests

9ecb9c0

fix formatting with spotless apply

66b8430

bvolpato reviewed Sep 27, 2023

View reviewed changes

...va/worker/src/main/java/org/apache/beam/runners/dataflow/worker/StreamingDataflowWorker.java Show resolved Hide resolved

bvolpato reviewed Sep 27, 2023

View reviewed changes

...va/worker/src/main/java/org/apache/beam/runners/dataflow/worker/StreamingDataflowWorker.java Outdated Show resolved Hide resolved

bvolpato suggested changes Sep 27, 2023

View reviewed changes

bvolpato reviewed Sep 28, 2023

View reviewed changes

...orker/src/test/java/org/apache/beam/runners/dataflow/worker/StreamingDataflowWorkerTest.java Outdated Show resolved Hide resolved

bvolpato approved these changes Sep 28, 2023

View reviewed changes

edman124 and others added 2 commits September 28, 2023 18:44

remove comments and rename counter

b7fdb4c

Merge branch 'master' into master

1fc3589

twang126 approved these changes Sep 28, 2023

View reviewed changes

fix formatting with spotless apply

419d4e0

damccorm reviewed Sep 29, 2023

View reviewed changes

edman124 and others added 2 commits September 29, 2023 13:36

Merge branch 'apache:master' into master

b6db0d3

fix tests for StreamingDataflowWorker change and fixed createMockWork

4843640

damccorm approved these changes Oct 2, 2023

View reviewed changes

damccorm merged commit bf1e829 into apache:master Oct 2, 2023

Abacn mentioned this pull request Oct 19, 2023

TImeout StreamingDataflowWorkerTest when test stuck #29065

Merged

3 tasks

Abacn reviewed Oct 19, 2023

View reviewed changes

This was referenced Oct 27, 2023

Automatically switch to upload_graph when the graph is large #28621

Merged

Report Outstanding Bundles and Bytes #29041

Merged

Conversation

edman124 commented Sep 18, 2023

GitHub Actions Tests Status (on master branch)

Uh oh!

edman124 commented Sep 18, 2023

Uh oh!

twang126 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Sep 19, 2023

Uh oh!

edman124 commented Sep 20, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Sep 20, 2023

Uh oh!

edman124 commented Sep 26, 2023

Uh oh!

Uh oh!

Uh oh!

bvolpato left a comment

Choose a reason for hiding this comment

Uh oh!

edman124 commented Sep 27, 2023

Uh oh!

Uh oh!

bvolpato left a comment

Choose a reason for hiding this comment

Uh oh!

bvolpato commented Sep 28, 2023

Uh oh!

damccorm Sep 29, 2023

Choose a reason for hiding this comment

Uh oh!

damccorm commented Oct 2, 2023

Uh oh!

damccorm left a comment

Choose a reason for hiding this comment

Uh oh!

Abacn Oct 19, 2023

Choose a reason for hiding this comment

Uh oh!

edman124 Oct 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Abacn Oct 19, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

twang126 left a comment •

edited

Loading

edman124 Oct 19, 2023 •

edited

Loading