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

Add provider HasPending method #751

Merged
merged 12 commits into from
Apr 21, 2024
Merged

Add provider HasPending method #751

merged 12 commits into from
Apr 21, 2024

Conversation

mfridman
Copy link
Collaborator

@mfridman mfridman commented Apr 20, 2024

Fix #640

This PR adds a new provider method to check for pending migrations.

func (p *Provider) HasPending(context.Context) (bool, error) {}

This functionality is especially useful for cases like @roblaszczak described in #507 (comment) (I'll try to summarize this when writing up the documentation for this).

Important

This underlying implementation does not respect the SessionLocker and can be used to check for pending migrations without blocking or being blocked by other operations.

I've also added this before running .Up(), .UpTo, .UpByOne methods, which means those methods don't have to acquire the session lock (if enabled) until it's truly required to begin an operation.

This is quite necessary in k8s-type environments where newer pods have long-running migrations, and older pods being bounced/rebooted/recovering from a crash/etc. are blocked by the SessionLocker and the entire application is down waiting on the long-running migration from one of the newer pods to complete.

There's an edge case that needs to be accounted for. Since HasPending doesn't use a SessionLocker, we need to make the initial table version creation retriable since there can be multiple instances trying to create it in parallel. Worst case instance 1 creates the table and instance 2 fails on its first try, then on the second try it succeeds. Retry 3 times every 1 second.

Debug notes

seq 100 | xargs -I{} -P 4 -n 1 sh -c 'go test -timeout 30s -run ^TestPostgresHasPending$ ./internal/testing/integration -race -count=1 >> log.out'

@mfridman mfridman mentioned this pull request Apr 21, 2024
7 tasks
@mfridman mfridman merged commit 1ad801c into master Apr 21, 2024
5 checks passed
@mfridman mfridman deleted the mf/provider branch April 21, 2024 17:05
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.

Avoid acquiring lock if no pending migrations
1 participant