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

feat(bigquery/storage/managedwriter): graceful connection drains #11463

Merged
merged 4 commits into from
Jan 17, 2025

Conversation

shollyman
Copy link
Contributor

@shollyman shollyman commented Jan 17, 2025

This PR enables a more graceful shutdown when we're doing client initiated reconnects, typically in response to things like schema changes.

With this PR, rather than immediately cancelling the previous connection context, we instead use a goroutine to defer this by a fixed interval, which gives our recv goroutine more time to process any incoming notifications from the service.

This can yield reductions in duplicate rows for some scenarios, particularly when EnableWriteRetries is enabled and we see sequential disconnects (e.g. a multiplex connection observing a number of schema updates).

It does, however, yield possible short term increases in resource usage, as we're now potentially retaining goroutines/channels/connections for a longer interval during these reconnect events. This PR updates testing that looks for leaking resources with this new understanding accordingly.

Towards: #11460

This PR enables a more graceful shutdown when we're doing client
initiated reconnects, typically in response to things like schema
changes.

With this PR, rather than immediately cancelling the previous connection
context, we instead use a goroutine to defer this by a fixed interval,
which gives our recv goroutine more time to process any incoming
notifications from the service.

This can yield a net reduction in duplicate rows for some scenarios,
particularly when EnableWriteRetries is enabled and we see sequential
disconnects (e.g. a multiplex connection observing a number of schema
updates).

It does, however, yield possible short term increases in resource usage,
as we're now potentially retaining goroutines/channels/connections for a
longer interval during these reconnect events.  This PR updates testing
that looks for leaking resources with this new understanding
accordingly.
@product-auto-label product-auto-label bot added the api: bigquery Issues related to the BigQuery API. label Jan 17, 2025
@shollyman shollyman requested a review from alvarowolfx January 17, 2025 02:09
@shollyman shollyman marked this pull request as ready for review January 17, 2025 02:09
@shollyman shollyman requested review from a team as code owners January 17, 2025 02:09
Copy link

@leahecole leahecole left a comment

Choose a reason for hiding this comment

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

Leaving lots of questions and one half opinion

@shollyman shollyman merged commit b29912f into googleapis:main Jan 17, 2025
8 checks passed
@shollyman shollyman deleted the graceful-reconnect branch January 17, 2025 18:37
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants