Fix race conditions in CloseCampaign by blocking
Created by: mrnugget
This is one of the PRs to fix #12827 (closed).
Yesterday we found out that there were race conditions in the old CloseCampaign
code:
- Changesets could switch from "queued" to "processing" between our "count all processing changesets" call and the "return if count is > 0" check.
- Changesets could switch from "unpublished" to "published & open" after we did the "list all open changesets" call
On top of that the UpdateChangeset
calls would block if a changeset was processing.
So what we do now instead is cleaner and more consistent.
We mark all changesets associated with the campaign as to-be-closed and enqueue them. That avoids the race conditions of changesets currently being published while we try to close the campaign.
And it works because the set of changesets doesn't change while we do this, since we close the campaign in a transaction and hold the row lock on it, meaning that another user cannot ApplyCampaign
while we're in this code path and add/remove changesets.
Each UpdateChangeset
call will then block again, if one of the changesets is currently being processed. That's good, because it leaves us in a consistent state where we don't overwrite data the reconciler wants to write.
That means that the CloseCampaign
mutation might block until all currently processing changesets are done being processed, if there are processing changesets.
We lose the "cannot close currently processing campaign" error, but that wouldn't work anyway with the new model. This here is far more consistent.
We just need to make sure that we show a spinner on the "close campaign" confirmation button, maybe. We do have a spinner.