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.

Sign up for GitHub

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

Jump to bottom

Optimize failed key processing by indexing workitems by shardingkey + workid #33755

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Optimize failed key processing by indexing workitems by shardingkey + workid #33755

wants to merge 15 commits into from

Conversation

arunpandianp
Copy link
Contributor

The n^2 loop in failWorkForKey hols the ActiveWorkState mutex and causes delays when many items fail

@arunpandianp
Copy link
Contributor Author

R: @scwhittle

@github-actions GitHub Actions
Copy link
Contributor

Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment assign set of reviewers

@arunpandianp
Copy link
Contributor Author

ExecutableWork is an autovalue though, so it may have an equals which it seems like it shouldn't. Maybe we should unautovalue that instead of changing here?

I'll think about this more. Don't want to block this PR on it.

@scwhittle
Copy link
Contributor

needs resolving

@arunpandianp
Copy link
Contributor Author

resolved conflicts

@arunpandianp
Copy link
Contributor Author

Run Java PreCommit

2 similar comments
@arunpandianp
Copy link
Contributor Author

Run Java PreCommit

@arunpandianp
Copy link
Contributor Author

Run Java PreCommit

@arunpandianp
Copy link
Contributor Author

The failing tests are real failures, the test logic needs to be modified to account for the fact only one work item can be active for a sharding key after this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scwhittle scwhittle scwhittle approved these changes

Assignees
No one assigned
Labels
dataflow runners
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@arunpandianp @scwhittle