[2.6.x] Concurrent artifact creation test & fixes #5169
Merged
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.
This PR fixes a concurrency problem in the SQL storage when creating artifacts. There are several "upsert" style operations that must happen when creating artifacts, including:
There were existing race conditions (database dependent) for some or all of these operations.
This PR changes 1 and 2 so that the "Ensure Group Exists" and "Ensure Content Exists" operations occur in their own transactions. This allows us to forgo using
UPSERT
and instead simply try theINSERT
and react to the primary key constraint violation. If a PK violation occurs then we simply go on to the next part of the create (e.g. if creating the group fails, it means the group already exists and we can skip to the next thing).This new approach fixes the concurrency issues and also simplifies the logic a little bit. But it does this at the expense of introducing a
small
chance that a Group will be created even though it is not needed (e.g. if multiple concurrent operations attempt to auto-create the group but then all fail). The same is true of content.As a result, this change MAY result in empty Groups or orphaned content. Orphaned content will get cleaned up automatically, but empty groups would need to be deleted manually. I think this is an acceptable tradeoff.