Retry partially applied no-txn migrations from last failed statement #187
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We still don't store the count or do anything with it other than printing, but this is a start.
This is necessary to allow resuming application of failed no-txn migrations from their point of failure onwards. While this should be extremely rare, if we are in the business of retrying `BEGIN ... COMMIT` blocks of no-txn migrations, we're already in extremely rare territory. It is also behaviour that might be useful in the darkest of hours. We will still need to change how we collect pending migrations, because now a migration's applied state is no longer binary (i.e. applied or not), implement seeking and a bunch of other things.
… errors Right now this amounts to throwing exceptions in different places, and using a new custom exception type to carry information of where the last `BEGIN` was higher up in the call stack (this does not feel great), if there was one at all. But so far we don't transmit that yet. Also, this removes calls to retry statements in no-txn migrations from functions deeper in the call stack, so that retrying is done higher up where it's possible to re-read files and seek the stream to the right statement. And this actually helped clean up some of the code. Still much to do, though.
single migration. This invariant is not obvious and so fragile to rely on. This commit also includes a bit more of the plumbing that is necessary to retry no-txn migs appropriately, but there's still much to do.
Also fine-tune error messages, and some other messages, too
This will catch invalid assumptions in the codebase
This is a super specific error case of a failed bootstrapping no-txn migration that runs enough statements to make the default connection string accessible. With this commit codd will create codd_schema after this failure and register this migration was partially applied. This is the most unlikely and useless error case I can think of, and the implementation messes with retry timeouts, so it's not clear this is a good idea. Plus, `codd add` might run into it. Since we have to address other issues with `codd add` given the new code anyway, it might not be that big of a problem. At least we can say codd will be helpful even in the most extreme cases. Even if I can't think of a case where this particular feature will do that.
More type applications to the rescue.. but it did remove one code path that was printing "This is an internal error.."
Now we detect if still in a transaction before sending `COMMIT`. This is helpful now that we no longer throw exceptions when an error happens, and instead manually send ROLLBACK.
We had to reimplement `putCopyEnd` from postgresql-simple, and some things still aren't clear. More investigation is necessary.
This is a nicer way of avoiding libpq internals
mzabani
force-pushed
the
issue-154-retry-last-txn
branch
from
dba5f35 to
6ba3513
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #154
This involved a relatively large refactor, but it was really good as it actually uncovered some internal issues with our type-level transaction sandbox and other defects. Some nice consequences include:
Also, new tests have been added which solidify some retry scenarios even for in-txn migrations and improve codd's output quite a bit.