Skip to content
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.

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

[BEAM-8816] Option to load balance bundle processing w/ multiple SDK workers #10313

Merged
merged 1 commit into from
Dec 16, 2019

Conversation

tweise
Copy link
Contributor

@tweise tweise commented Dec 6, 2019

A new pipeline option that allows for bundles of a given executable stage to be processed with any available SDK worker (the default is to process all bundles with the same SDK worker).

https://lists.apache.org/thread.html/59c02d8b8ea849c158deb39ad9d83af4d8fcb56570501c7fe8f79bb2%40%3Cdev.beam.apache.org%3E


Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

  • Choose reviewer(s) 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.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

Post-Commit Tests Status (on master branch)

Lang SDK Apex Dataflow Flink Gearpump Samza Spark
Go Build Status --- --- Build Status --- --- Build Status
Java Build Status Build Status Build Status Build Status
Build Status
Build Status
Build Status Build Status Build Status
Build Status
Build Status
Python Build Status
Build Status
Build Status
Build Status
--- Build Status
Build Status
Build Status
Build Status
--- --- Build Status
XLang --- --- --- Build Status --- --- ---

Pre-Commit Tests Status (on master branch)

--- Java Python Go Website
Non-portable Build Status Build Status
Build Status
Build Status Build Status
Portable --- Build Status --- ---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

@tweise tweise requested a review from mxm December 6, 2019 17:07
@tweise tweise assigned lukecwik and unassigned lukecwik Dec 6, 2019
@tweise tweise requested a review from lukecwik December 6, 2019 17:07
Copy link
Contributor

@mxm mxm left a comment

Choose a reason for hiding this comment

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

Very useful feature. I don't know why we're just adding this now, seems obvious to have :) Thanks for implementing!

environmentCaches.get(environmentIndex).getUnchecked(executableStage.getEnvironment());
client.ref();
final LoadingCache<Environment, WrappedSdkHarnessClient> currentCache;
if (loadBalanceBundles) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just an idea: Instead of conditionally switching here, could we add an interface to handle the client selection? We could have two implementations, one regular, one for load balancing. Would make the code easier to read here (necessary data structures would go into the implementation) and also avoid runtime checks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also had that idea, maybe as a follow-up.

@tweise tweise force-pushed the BEAM-8816.loadBalanceBundles branch from c7573b7 to c8595ec Compare December 16, 2019 18:56
@tweise
Copy link
Contributor Author

tweise commented Dec 16, 2019

CC: @rakeshcusat

@rakeshcusat
Copy link
Contributor

👍

@tweise
Copy link
Contributor Author

tweise commented Dec 16, 2019

Run Java PreCommit

@tweise tweise merged commit e486202 into apache:master Dec 16, 2019
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.

4 participants