Skip to content

Adds retry loop with lock timeouts#52

Open
rnubel wants to merge 3 commits intomasterfrom
lock-retry-loop
Open

Adds retry loop with lock timeouts#52
rnubel wants to merge 3 commits intomasterfrom
lock-retry-loop

Conversation

@rnubel
Copy link
Owner

@rnubel rnubel commented May 15, 2020

This functionality is critical for applying patches to large, active databases in an automated context. The default values here are battle-tested against Enova's busiest database, but can be fully customized. If you don't need the behavior at all, set the timeouts and max retries to -1.

}
defer os.Remove(tmpfile.Name()) // clean up

if _, err := tmpfile.WriteString(timeoutStatements(c)); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do the timeouts need to be reset after each migration?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yes, since each migration runs through a separate invocation of psql and thus a new connection.

Copy link
Owner Author

Choose a reason for hiding this comment

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

On the pq driver, when the connection doesn't change, not necessarily -- but it does prevent any in-migration overrides from bleeding through to the next migration.

@rnubel rnubel force-pushed the lock-retry-loop branch from 5d276af to 6a2999e Compare June 3, 2020 19:04
Copy link
Contributor

@NcUltimate NcUltimate left a comment

Choose a reason for hiding this comment

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

LGTM 👍

This functionality is critical for applying patches to large, active databases
in an automated context. If you don't need it, set the timeouts to -1.
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.

2 participants