You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2020/05/27 03:26:55 UTC
[GitHub] [beam] steveniemitz opened a new pull request #11811: [BEAM-10075] Add a flag to tune the size of the grouping tables used by the dataflow workers
steveniemitz opened a new pull request #11811:
URL: https://github.com/apache/beam/pull/11811
This change simply adds a flag to allow users to set the size of grouping tables, rather than hard coding them to 100 MB. The flag defaults to 100 MB, so users who don't set the flag will see no difference in behavior.
R: @lukecwik
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [x] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [x] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [x] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
Post-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
Lang | SDK | Apex | Dataflow | Flink | Gearpump | Samza | Spark
--- | --- | --- | --- | --- | --- | --- | ---
Go | [](https://builds.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
Java | [](https://builds.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Apex/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Gearpump/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
Python | [](https://builds.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/) | --- | [](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/)
XLang | --- | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/)
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
--- |Java | Python | Go | Website
--- | --- | --- | --- | ---
Non-portable | [](https://builds.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/)
Portable | --- | [](https://builds.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | ---
See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] steveniemitz commented on pull request #11811: [BEAM-10075] Add a flag to tune the size of the grouping tables used by the dataflow workers
Posted by GitBox <gi...@apache.org>.
steveniemitz commented on pull request #11811:
URL: https://github.com/apache/beam/pull/11811#issuecomment-634097057
looks like the precommit failed on flink 1.9 tests:
```
org.apache.beam.runners.flink.translation.wrappers.streaming.io.UnboundedSourceWrapperTest > org.apache.beam.runners.flink.translation.wrappers.streaming.io.UnboundedSourceWrapperTest$ParameterizedUnboundedSourceWrapperTest.testWatermarkEmission[numTasks = 4; numSplits=2] FAILED
```
Which I'm going to guess are unrelated to this change ;)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] steveniemitz commented on a change in pull request #11811: [BEAM-10075] Add a flag to tune the size of the grouping tables used by the dataflow workers
Posted by GitBox <gi...@apache.org>.
steveniemitz commented on a change in pull request #11811:
URL: https://github.com/apache/beam/pull/11811#discussion_r430834337
##########
File path: runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/options/DataflowPipelineDebugOptions.java
##########
@@ -177,6 +177,20 @@ public Dataflow create(PipelineOptions options) {
void setNumberOfWorkerHarnessThreads(int value);
+ /**
+ * Size (in MB) of each grouping table used to pre-combine elements. If unset, defaults to 100 MB.
+ *
+ * <p>CAUTION: If set too large, workers may run into OOM conditions more easily, each worker may
+ * have many grouping tables in-memory concurrently.
+ */
+ @Description(
+ "The size (in MB) of the grouping tables used to pre-combine elements before "
+ + "shuffling. Larger values may reduce the amount of data shuffled.")
+ @Default.Long(100)
+ Long getGroupingTableMaxSizeMb();
+
+ void setGroupingTableMaxSizeMb(Long value);
Review comment:
👍 done!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] steveniemitz commented on pull request #11811: [BEAM-10075] Add a flag to tune the size of the grouping tables used by the dataflow workers
Posted by GitBox <gi...@apache.org>.
steveniemitz commented on pull request #11811:
URL: https://github.com/apache/beam/pull/11811#issuecomment-634414369
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik merged pull request #11811: [BEAM-10075] Add a flag to tune the size of the grouping tables used by the dataflow workers
Posted by GitBox <gi...@apache.org>.
lukecwik merged pull request #11811:
URL: https://github.com/apache/beam/pull/11811
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik commented on pull request #11811: [BEAM-10075] Add a flag to tune the size of the grouping tables used by the dataflow workers
Posted by GitBox <gi...@apache.org>.
lukecwik commented on pull request #11811:
URL: https://github.com/apache/beam/pull/11811#issuecomment-634415558
> looks like `org.apache.beam.sdk.transforms.ParDoLifecycleTest.testTeardownCalledAfterExceptionInFinishBundleStateful` failed in the precommit here, I can't imagine that was from this change?
There are several flakes in the project which need to be fixed. Feel free to rerun the precommit as necessary and/or fix the flakes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik commented on a change in pull request #11811: [BEAM-10075] Add a flag to tune the size of the grouping tables used by the dataflow workers
Posted by GitBox <gi...@apache.org>.
lukecwik commented on a change in pull request #11811:
URL: https://github.com/apache/beam/pull/11811#discussion_r430831035
##########
File path: runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/options/DataflowPipelineDebugOptions.java
##########
@@ -177,6 +177,20 @@ public Dataflow create(PipelineOptions options) {
void setNumberOfWorkerHarnessThreads(int value);
+ /**
+ * Size (in MB) of each grouping table used to pre-combine elements. If unset, defaults to 100 MB.
+ *
+ * <p>CAUTION: If set too large, workers may run into OOM conditions more easily, each worker may
+ * have many grouping tables in-memory concurrently.
+ */
+ @Description(
+ "The size (in MB) of the grouping tables used to pre-combine elements before "
+ + "shuffling. Larger values may reduce the amount of data shuffled.")
+ @Default.Long(100)
+ Long getGroupingTableMaxSizeMb();
+
+ void setGroupingTableMaxSizeMb(Long value);
Review comment:
Please add this option to https://github.com/apache/beam/blob/32c657955d05cb5c558def91c2d1e7edb80fb123/sdks/java/core/src/main/java/org/apache/beam/sdk/options/SdkHarnessOptions.java#L32 instead of here.
Please also plumb the value to https://github.com/apache/beam/blob/32c657955d05cb5c558def91c2d1e7edb80fb123/sdks/java/harness/src/main/java/org/apache/beam/fn/harness/PrecombineGroupingTable.java#L44
We should be able to use a primitive type (also long seems really large for specifying amount of MBs):
```suggestion
int getGroupingTableMaxSizeMb();
void setGroupingTableMaxSizeMb(int value);
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org