refactor backup restore to handle serialization errors and conflicts #316
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
vroldanbet
force-pushed
the
backup-import-with-retries-and-conflict-handling
branch
8 times, most recently
from
2e08955
to
068d914
Compare
vroldanbet
force-pushed
the
backup-import-with-retries-and-conflict-handling
branch
from
4e6adf5
to
659f208
Compare
|
Reference: authzed/spicedb#1544 |
vroldanbet
force-pushed
the
backup-import-with-retries-and-conflict-handling
branch
2 times, most recently
from
8ff0438
to
81c5100
Compare
vroldanbet
changed the title
jzelinskie
reviewed
Dec 21, 2023
vroldanbet
force-pushed
the
backup-import-with-retries-and-conflict-handling
branch
from
4f20903
to
1a9fec5
Compare
This commit refactors the logic on the backup command into a restorer type. The fundamental differences with the previous logic are: - batch conflicts are detected, and provides to handling strategies: fail, skip or touch when using touch, they will also be retried if needed - serialization errors are detected, and optionally can be retried using WithRelationships with TOUCH semantics - retry strategy uses a backoff any error on bulk import will cause the stream to close, so attempts to recover from that by retrying means retrying with another bulk import stream or using WriteRelationships. The latter was used because it normalizes retries as BulkImport does not support TOUCH semantics.
to include MySQL and Postgres
since MySQL sometimes can get locked for a long time on serialization errors
so the restorer becomes fully self-contained and facilitate exporting it
the flags were not registered in the deprecated `zed restore` command when it was added for back-compat after the command became `zed backup restore`
turned into a string that can be parsed into an enum
josephschorr
force-pushed
the
backup-import-with-retries-and-conflict-handling
branch
from
1a9fec5
to
461353e
Compare
vroldanbet
deleted the
backup-import-with-retries-and-conflict-handling
branch
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Related to authzed/spicedb#1544
Related to authzed/spicedb#1542
This commit refactors the logic on the backup command into a restorer type. The fundamental differences with the previous logic are:
any error on bulk import will cause the stream to close, so attempts to recover from that by retrying means retrying with another bulk import stream or using WriteRelationships. The latter was used because it normalizes retries as BulkImport does not support TOUCH semantics.
use
New flags are added to the
zed backup restorecommand:--conflict-strategydefaults tofail, which meanszedwill fail if a conflicting relationship is found. This is kept around to remain backward compatible with the original behavior.skipwould skip importing any batch that contains a conflicting relationship. Please note this may skip relationships that are part of the batch but not stored in SpiceDBtouchwill issue aWriteRelationshipscall with each batch--disable-retriesis added to perform retries, which is the original behavior. Once this PR lands, retries will be enabled by default. The maximum number of retries is 10 and has an exponential backoff. Both are non-configurable.--request-timeoutis added to add a timeout toWriteRelationshipcalls used on conflicts.BulkImportAPI calls are not subject to the timeout.