Skip to content

[BEAM-2213] Convert Executor Services to use Daemon Threads#2954

Closed
tgroh wants to merge 2 commits intoapache:masterfrom
tgroh:executor_daemons
Closed

[BEAM-2213] Convert Executor Services to use Daemon Threads#2954
tgroh wants to merge 2 commits intoapache:masterfrom
tgroh:executor_daemons

Conversation

@tgroh
Copy link
Member

@tgroh tgroh commented May 8, 2017

Be sure to do all of the following to help us incorporate your contribution
quickly and easily:

  • Make sure the PR title is formatted like:
    [BEAM-<Jira issue #>] Description of pull request
  • Make sure tests pass via mvn clean verify.
  • Replace <Jira issue #> in the title with the actual Jira issue
    number, if there is one.
  • If this contribution is large, please file an Apache
    Individual Contributor License Agreement.

This will cause the DirectRunner to automatically shut down when the
worker threads are shut down.

This is mostly a fork of #2936, with a couple of additional executors changed and non-ExecutorService work removed.

@tgroh
Copy link
Member Author

tgroh commented May 8, 2017

R: @dhalperi

Copy link
Contributor

@dhalperi dhalperi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tag with BEAM-2213 ?

new ThreadFactoryBuilder()
.setThreadFactory(MoreExecutors.platformThreadFactory())
.setNameFormat("direct-runner-worker")
.build());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a comment why this one is not a daemon?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@tgroh tgroh changed the title Convert Executor Services to use Daemon Threads [BEAM-2213] Convert Executor Services to use Daemon Threads May 8, 2017
This will cause the DirectRunner to automatically shut down when the
worker threads are shut down.
@tgroh tgroh force-pushed the executor_daemons branch from 000f5d3 to 1aaae7c Compare May 8, 2017 16:50
@dhalperi
Copy link
Contributor

dhalperi commented May 8, 2017

[ERROR] /home/jenkins/jenkins-slave/workspace/beam_PreCommit_Java_MavenInstall/runners/direct-java/src/main/java/org/apache/beam/runners/direct/SplittableProcessElementsEvaluatorFactory.java:176: Line is longer than 100 characters (found 106). [LineLength]

@coveralls
Copy link

Coverage Status

Coverage increased (+0.003%) to 70.588% when pulling 6804ffd on tgroh:executor_daemons into 2b470ea on apache:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.007%) to 70.577% when pulling 6804ffd on tgroh:executor_daemons into 2b470ea on apache:master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants