-
Notifications
You must be signed in to change notification settings - Fork 12
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
colocated aware random picker #2301
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some initial comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
if err != nil { | ||
break | ||
} | ||
if task != pickedTask2 && task != pickedTask1 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how can the picked Task be equal to pickedTask1
, it is already marked as done above right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, it can't. Just wanted to exclude both task1 task2, to get task3.
Describe the changes in this pull request
Introduce a ColocatedAwareRandomTaskPicker which picks tasks based on weights/probabilities of the tables involved. This will essentially lead to multiple tables being ingested at the same time.
The max no. of tasks in parallel will be equal to the value of parallel-jobs (either default calculation of 0.25x cores in the case of adaptive parallelism, or the value provided by the user).
Describe if there are any user-facing changes
How was this pull request tested?
Does your PR have changes that can cause upgrade issues?