Skip to content

[BEAM-8412] xlang test: set sdk worker parallelism to 1 #9817

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ibzib merged 1 commit into from
Oct 18, 2019
Merged

[BEAM-8412] xlang test: set sdk worker parallelism to 1 #9817

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions sdks/python/test-suites/portable/py2/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ task crossLanguagePortableWordCount {
"--shutdown_sources_on_final_watermark",
"--environment_cache_millis=10000",
"--expansion_service_jar=${testServiceExpansionJar}",
// Writes to local filesystem might fail for multiple SDK workers.
"--sdk_worker_parallelism=1"
Copy link
Contributor

@tweise tweise Oct 17, 2019

Choose a reason for hiding this comment

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

The default for this option is 1. Is it set somewhere else to 0?

Copy link
Contributor

@tweise tweise Oct 17, 2019

Choose a reason for hiding this comment

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

I just noticed that this option was repeated in the Python SDK with a different default. We should fix that: https://github.com/apache/beam/blob/master/sdks/python/apache_beam/options/pipeline_options.py#L848

Copy link
Contributor

@chamikaramj chamikaramj Oct 17, 2019

Choose a reason for hiding this comment

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

+1 for fixing Python SDK's default value instead of here. BTW does this mean that Flink's cross-language implementation is somehow broken when there is more than one worker process per worker node ?

cc: @mxm

Copy link
Contributor

@tweise tweise Oct 18, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@chamikaramj WriteToText relies on shared filesystem, so more than one worker can cause it to fail with the local FS.

If it takes longer to sort out the pipeline option default, then it's probably also OK to merge this first and unblock the CI (assuming no other pre-commit issues).

Copy link
Contributor

@mxm mxm Oct 18, 2019

Choose a reason for hiding this comment

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

Let's add a comment here why this is necessary. Otherwise LGTM.

Copy link
Author

@ibzib ibzib Oct 18, 2019

Choose a reason for hiding this comment

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

@mxm Done.
@tweise #9829

]
exec {
executable 'sh'
Expand Down