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/14 18:11:48 UTC
[GitHub] [beam] omarismail94 opened a new pull request #11710: [BEAM-9964] Setting --workerCacheMB for Streaming Pipeline
omarismail94 opened a new pull request #11710:
URL: https://github.com/apache/beam/pull/11710
R:@ pabloem
Setting --workerCacheMB seems to affect batch pipelines only. For Streaming, the cache seems to be hardcoded to 100Mb [1]. If possible, I would like to make it allowable to change the cache value in Streaming when setting -workerCacheMB.
Passed ./gradlew -p runners/google-cloud-dataflow-java check on my computer
[1] https://github.com/apache/beam/blob/5e659bb80bcbf70795f6806e05a255ee72706d9f/runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/WindmillStateCache.java#L73
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [ ] 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.
- [ ] 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] omarismail94 commented on a change in pull request #11710: [BEAM-9964] Setting --workerCacheMB for Streaming Pipeline
Posted by GitBox <gi...@apache.org>.
omarismail94 commented on a change in pull request #11710:
URL: https://github.com/apache/beam/pull/11710#discussion_r425430039
##########
File path: runners/google-cloud-dataflow-java/worker/src/test/java/org/apache/beam/runners/dataflow/worker/WindmillStateCacheTest.java
##########
@@ -130,7 +133,8 @@ private static StateNamespace triggerNamespace(long start, int triggerIdx) {
@Before
public void setUp() {
- cache = new WindmillStateCache();
+ options = PipelineOptionsFactory.as(DataflowWorkerHarnessOptions.class);
+ cache = new WindmillStateCache(options.getWorkerCacheMb());
assertEquals(0, cache.getWeight());
Review comment:
Fixed this by adding a new Test method in WindmillStateCacheTest class. I created a new getter in the WindmillStateCache to retrieve the size of max weight on bytes, and compared it to the initial value set
----------------------------------------------------------------
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 #11710: [BEAM-9964] Setting --workerCacheMB for Streaming Pipeline
Posted by GitBox <gi...@apache.org>.
steveniemitz commented on pull request #11710:
URL: https://github.com/apache/beam/pull/11710#issuecomment-628818041
ahh this is great. We've been running a similar patch in our fork forever.
----------------------------------------------------------------
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] pabloem commented on pull request #11710: [BEAM-9964] Setting --workerCacheMB for Streaming Pipeline
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #11710:
URL: https://github.com/apache/beam/pull/11710#issuecomment-629476263
thanks @omarismail94 !
----------------------------------------------------------------
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 #11710: [BEAM-9964] Setting --workerCacheMB for Streaming Pipeline
Posted by GitBox <gi...@apache.org>.
steveniemitz commented on pull request #11710:
URL: https://github.com/apache/beam/pull/11710#issuecomment-628828183
> These shouldn't need to be battles but more like:
>
> ```
> you: dev@ hey we got this patch we use locally it is X
> dev@: sounds great, submit a PR or we have a larger plan around this do you want to work on it with us
> ```
heh, I feel bad clogging up this PR with unrelated conversations. If that process you described was how it worked in real life, that'd be great. Feel free to ping me on the ASF slack (at-steve) if you want to chat more about this.
----------------------------------------------------------------
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] pabloem commented on pull request #11710: [BEAM-9964] Setting --workerCacheMB for Streaming Pipeline
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #11710:
URL: https://github.com/apache/beam/pull/11710#issuecomment-628924201
Run Java PostCommit
----------------------------------------------------------------
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 #11710: [BEAM-9964] Setting --workerCacheMB for Streaming Pipeline
Posted by GitBox <gi...@apache.org>.
lukecwik commented on pull request #11710:
URL: https://github.com/apache/beam/pull/11710#issuecomment-628820475
> ahh this is great. We've been running a similar patch in our fork forever.
Feel free to submit patches upstream
----------------------------------------------------------------
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] pabloem merged pull request #11710: [BEAM-9964] Setting --workerCacheMB for Streaming Pipeline
Posted by GitBox <gi...@apache.org>.
pabloem merged pull request #11710:
URL: https://github.com/apache/beam/pull/11710
----------------------------------------------------------------
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] ibzib commented on pull request #11710: [BEAM-9964] Setting --workerCacheMB for Streaming Pipeline
Posted by GitBox <gi...@apache.org>.
ibzib commented on pull request #11710:
URL: https://github.com/apache/beam/pull/11710#issuecomment-629343652
Run Java PreCommit
----------------------------------------------------------------
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 #11710: [BEAM-9964] Setting --workerCacheMB for Streaming Pipeline
Posted by GitBox <gi...@apache.org>.
steveniemitz commented on pull request #11710:
URL: https://github.com/apache/beam/pull/11710#issuecomment-635468825
hm, I was just rebasing my work against this commit and realized something. I had moved the flag to `DataflowPipelineDebugOptions`. `DataflowWorkerHarnessOptions` are not included in the options set visible to the "client" side of things.
This means that you can't actually set this flag from a job submission, eg:
```
$ java -jar myjob.jar ... --runner=DataflowRunner --workerCacheMb=400
Exception in thread "main" java.lang.IllegalArgumentException: Class interface ...MyOptions missing a property named 'workerCacheMb'.
at org.apache.beam.sdk.options.PipelineOptionsFactory.parseObjects(PipelineOptionsFactory.java:1625)
```
----------------------------------------------------------------
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] pabloem commented on pull request #11710: [BEAM-9964] Setting --workerCacheMB for Streaming Pipeline
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #11710:
URL: https://github.com/apache/beam/pull/11710#issuecomment-628901207
Run Java PostCommit
----------------------------------------------------------------
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] pabloem commented on a change in pull request #11710: [BEAM-9964] Setting --workerCacheMB for Streaming Pipeline
Posted by GitBox <gi...@apache.org>.
pabloem commented on a change in pull request #11710:
URL: https://github.com/apache/beam/pull/11710#discussion_r425345741
##########
File path: runners/google-cloud-dataflow-java/worker/src/test/java/org/apache/beam/runners/dataflow/worker/WindmillStateCacheTest.java
##########
@@ -130,7 +133,8 @@ private static StateNamespace triggerNamespace(long start, int triggerIdx) {
@Before
public void setUp() {
- cache = new WindmillStateCache();
+ options = PipelineOptionsFactory.as(DataflowWorkerHarnessOptions.class);
+ cache = new WindmillStateCache(options.getWorkerCacheMb());
assertEquals(0, cache.getWeight());
Review comment:
Can you add a check to this test to make sure that the `maximumWeight` of the cache is the 100 MB? (perhaps use a number different than 100 to be sure).
----------------------------------------------------------------
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] omarismail94 commented on pull request #11710: [BEAM-9964] Setting --workerCacheMB for Streaming Pipeline
Posted by GitBox <gi...@apache.org>.
omarismail94 commented on pull request #11710:
URL: https://github.com/apache/beam/pull/11710#issuecomment-628886984
retest this please
----------------------------------------------------------------
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] pabloem commented on pull request #11710: [BEAM-9964] Setting --workerCacheMB for Streaming Pipeline
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #11710:
URL: https://github.com/apache/beam/pull/11710#issuecomment-628890978
retest this please
----------------------------------------------------------------
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] omarismail94 commented on pull request #11710: [BEAM-9964] Setting --workerCacheMB for Streaming Pipeline
Posted by GitBox <gi...@apache.org>.
omarismail94 commented on pull request #11710:
URL: https://github.com/apache/beam/pull/11710#issuecomment-628888729
New changes passed ./gradlew -p runners/google-cloud-dataflow-java check on my computer
----------------------------------------------------------------
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] pabloem commented on pull request #11710: [BEAM-9964] Setting --workerCacheMB for Streaming Pipeline
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #11710:
URL: https://github.com/apache/beam/pull/11710#issuecomment-628891444
retest this please
----------------------------------------------------------------
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] ibzib commented on pull request #11710: [BEAM-9964] Setting --workerCacheMB for Streaming Pipeline
Posted by GitBox <gi...@apache.org>.
ibzib commented on pull request #11710:
URL: https://github.com/apache/beam/pull/11710#issuecomment-629342947
Run Java PreCommit
----------------------------------------------------------------
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 #11710: [BEAM-9964] Setting --workerCacheMB for Streaming Pipeline
Posted by GitBox <gi...@apache.org>.
steveniemitz commented on pull request #11710:
URL: https://github.com/apache/beam/pull/11710#issuecomment-628824973
> > ahh this is great. We've been running a similar patch in our fork forever.
>
> Feel free to submit patches upstream
heh, I've been choosing my battles ;)
----------------------------------------------------------------
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] pabloem commented on pull request #11710: [BEAM-9964] Setting --workerCacheMB for Streaming Pipeline
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #11710:
URL: https://github.com/apache/beam/pull/11710#issuecomment-628958485
Seems like java precommits are broken on master - but this change LGTM. I'll wait for precommits to be fixed if possible. Thanks @omarismail94 !
----------------------------------------------------------------
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] pabloem commented on pull request #11710: [BEAM-9964] Setting --workerCacheMB for Streaming Pipeline
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #11710:
URL: https://github.com/apache/beam/pull/11710#issuecomment-629454663
Run Java PreCommit
----------------------------------------------------------------
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 #11710: [BEAM-9964] Setting --workerCacheMB for Streaming Pipeline
Posted by GitBox <gi...@apache.org>.
lukecwik commented on pull request #11710:
URL: https://github.com/apache/beam/pull/11710#issuecomment-628826053
> > > ahh this is great. We've been running a similar patch in our fork forever.
> >
> >
> > Feel free to submit patches upstream
>
> heh, I've been choosing my battles ;)
These shouldn't need to be battles but more like:
```
you: dev@ hey we got this patch we use locally it is X
dev@: sounds great, submit a PR or we have a larger plan around this do you want to work on it with us
```
----------------------------------------------------------------
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