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/15 17:09:57 UTC
[GitHub] [beam] robertwb opened a new pull request #11720: [BEAM-9729] Ignore bundle registration requests.
robertwb opened a new pull request #11720:
URL: https://github.com/apache/beam/pull/11720
Instead always pull bundle descriptors from the runner.
This avoids having potentially duplicated caches of all bundle
------------------------
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] y1chi commented on pull request #11720: [BEAM-9729] Ignore bundle registration requests.
Posted by GitBox <gi...@apache.org>.
y1chi commented on pull request #11720:
URL: https://github.com/apache/beam/pull/11720#issuecomment-702393496
@robertwb friendly ping, should we merge this PR
----------------------------------------------------------------
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] ananvay commented on a change in pull request #11720: [BEAM-9729] Ignore bundle registration requests.
Posted by GitBox <gi...@apache.org>.
ananvay commented on a change in pull request #11720:
URL: https://github.com/apache/beam/pull/11720#discussion_r426013359
##########
File path: sdks/java/harness/src/main/java/org/apache/beam/fn/harness/FnHarness.java
##########
@@ -206,18 +205,16 @@ public static void main(
LoadingCache<String, BeamFnApi.ProcessBundleDescriptor> processBundleDescriptors =
CacheBuilder.newBuilder()
+ .maximumSize(1000)
+ .expireAfterAccess(10, TimeUnit.MINUTES)
Review comment:
Is this too large of a time interval? I'm assuming the SDK would ideally not wait ~10 min before starting the bundle, right?
----------------------------------------------------------------
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] robertwb edited a comment on pull request #11720: [BEAM-9729] Ignore bundle registration requests.
Posted by GitBox <gi...@apache.org>.
robertwb edited a comment on pull request #11720:
URL: https://github.com/apache/beam/pull/11720#issuecomment-629376271
R: @ananvay
----------------------------------------------------------------
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] ananvay commented on pull request #11720: [BEAM-9729] Ignore bundle registration requests.
Posted by GitBox <gi...@apache.org>.
ananvay commented on pull request #11720:
URL: https://github.com/apache/beam/pull/11720#issuecomment-629455295
LGTM. Thanks Robert!
----------------------------------------------------------------
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] ananvay commented on pull request #11720: [BEAM-9729] Ignore bundle registration requests.
Posted by GitBox <gi...@apache.org>.
ananvay commented on pull request #11720:
URL: https://github.com/apache/beam/pull/11720#issuecomment-670116185
/cc: @y1chi
----------------------------------------------------------------
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] robertwb commented on a change in pull request #11720: [BEAM-9729] Ignore bundle registration requests.
Posted by GitBox <gi...@apache.org>.
robertwb commented on a change in pull request #11720:
URL: https://github.com/apache/beam/pull/11720#discussion_r426020019
##########
File path: sdks/java/harness/src/main/java/org/apache/beam/fn/harness/FnHarness.java
##########
@@ -206,18 +205,16 @@ public static void main(
LoadingCache<String, BeamFnApi.ProcessBundleDescriptor> processBundleDescriptors =
CacheBuilder.newBuilder()
+ .maximumSize(1000)
+ .expireAfterAccess(10, TimeUnit.MINUTES)
Review comment:
This is the time duration between never seeing a bundle of this type and discarding its description from the cache. It could be smaller, but I figured it was good to start out very conservatively (both count and time).
----------------------------------------------------------------
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] robertwb merged pull request #11720: [BEAM-9729] Ignore bundle registration requests.
Posted by GitBox <gi...@apache.org>.
robertwb merged pull request #11720:
URL: https://github.com/apache/beam/pull/11720
----------------------------------------------------------------
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] ananvay commented on pull request #11720: [BEAM-9729] Ignore bundle registration requests.
Posted by GitBox <gi...@apache.org>.
ananvay commented on pull request #11720:
URL: https://github.com/apache/beam/pull/11720#issuecomment-629447440
Thanks a lot Robert! Overall LGTM. Just a few minor comments, PTAL.
----------------------------------------------------------------
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] robertwb commented on pull request #11720: [BEAM-9729] Ignore bundle registration requests.
Posted by GitBox <gi...@apache.org>.
robertwb commented on pull request #11720:
URL: https://github.com/apache/beam/pull/11720#issuecomment-629376271
R: @ ananvay
----------------------------------------------------------------
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] stale[bot] commented on pull request #11720: [BEAM-9729] Ignore bundle registration requests.
Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #11720:
URL: https://github.com/apache/beam/pull/11720#issuecomment-660441601
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@beam.apache.org list. Thank you for your contributions.
----------------------------------------------------------------
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] robertwb commented on pull request #11720: [BEAM-9729] Ignore bundle registration requests.
Posted by GitBox <gi...@apache.org>.
robertwb commented on pull request #11720:
URL: https://github.com/apache/beam/pull/11720#issuecomment-702878872
Yes, we should get this in. I'll rebase.
----------------------------------------------------------------
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] robertwb commented on a change in pull request #11720: [BEAM-9729] Ignore bundle registration requests.
Posted by GitBox <gi...@apache.org>.
robertwb commented on a change in pull request #11720:
URL: https://github.com/apache/beam/pull/11720#discussion_r426018974
##########
File path: sdks/java/harness/src/main/java/org/apache/beam/fn/harness/FnHarness.java
##########
@@ -206,18 +205,16 @@ public static void main(
LoadingCache<String, BeamFnApi.ProcessBundleDescriptor> processBundleDescriptors =
CacheBuilder.newBuilder()
+ .maximumSize(1000)
+ .expireAfterAccess(10, TimeUnit.MINUTES)
.build(
new CacheLoader<String, BeamFnApi.ProcessBundleDescriptor>() {
@Override
public BeamFnApi.ProcessBundleDescriptor load(String id) {
- try {
- return blockingControlStub.getProcessBundleDescriptor(
- BeamFnApi.GetProcessBundleDescriptorRequest.newBuilder()
- .setProcessBundleDescriptorId(id)
- .build());
- } catch (Throwable th) {
- return (BeamFnApi.ProcessBundleDescriptor) fnApiRegistry.getById(id);
Review comment:
All runners now support the pull protocol.
----------------------------------------------------------------
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] ananvay commented on a change in pull request #11720: [BEAM-9729] Ignore bundle registration requests.
Posted by GitBox <gi...@apache.org>.
ananvay commented on a change in pull request #11720:
URL: https://github.com/apache/beam/pull/11720#discussion_r426012824
##########
File path: sdks/java/harness/src/main/java/org/apache/beam/fn/harness/FnHarness.java
##########
@@ -206,18 +205,16 @@ public static void main(
LoadingCache<String, BeamFnApi.ProcessBundleDescriptor> processBundleDescriptors =
CacheBuilder.newBuilder()
+ .maximumSize(1000)
+ .expireAfterAccess(10, TimeUnit.MINUTES)
.build(
new CacheLoader<String, BeamFnApi.ProcessBundleDescriptor>() {
@Override
public BeamFnApi.ProcessBundleDescriptor load(String id) {
- try {
- return blockingControlStub.getProcessBundleDescriptor(
- BeamFnApi.GetProcessBundleDescriptorRequest.newBuilder()
- .setProcessBundleDescriptorId(id)
- .build());
- } catch (Throwable th) {
- return (BeamFnApi.ProcessBundleDescriptor) fnApiRegistry.getById(id);
Review comment:
do we not need the try/catch block anymore?
----------------------------------------------------------------
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] ananvay commented on pull request #11720: [BEAM-9729] Ignore bundle registration requests.
Posted by GitBox <gi...@apache.org>.
ananvay commented on pull request #11720:
URL: https://github.com/apache/beam/pull/11720#issuecomment-660446868
friendly ping.
----------------------------------------------------------------
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