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

fix(bigquery/storage/managedstorage): improve internal locking #6304

Merged
merged 10 commits into from Jul 7, 2022

Conversation

shollyman
Copy link
Contributor

@shollyman shollyman commented Jul 5, 2022

This PR addresses some potential deadlocks introduced as part of a refactor
of critical section code. Based on a suggestion from @fsaintjacques this
PR switches to closures/funcs where unlocking is handled via defer, rather than explicit unlocking.

This PR adds testing to help ensure further refactors don't reintroduce deadlocks in either
the append or stream closure methods.

This PR also includes small change to the retry predicate to disallow retries for
context-based errors (cancelations/expirations).

Fixes #6278

This PR addresses some potential deadlocks introduced as part of a refactor
of critical section code.  It adds a testing to help isolate deadlock code,
and makes a small change to the retry predicate to disallow retries of
context-based errors (cancelations/expirations).
@shollyman shollyman requested review from steffnay and a team July 5, 2022 03:48
@shollyman shollyman requested a review from a team as a code owner July 5, 2022 03:48
@product-auto-label product-auto-label bot added size: l Pull request size is large. api: bigquery Issues related to the BigQuery API. labels Jul 5, 2022
@shollyman shollyman requested a review from codyoss July 6, 2022 18:30
Copy link
Member

@codyoss codyoss left a comment

Choose a reason for hiding this comment

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

LGTM me overall, just a nit and a question

@shollyman shollyman merged commit a2925ce into googleapis:main Jul 7, 2022
@shollyman shollyman deleted the fun-with-locking branch July 7, 2022 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigquery Issues related to the BigQuery API. size: l Pull request size is large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bigquery/storage/managedwriter: deadlocks due to locking changes
4 participants