Skip to content

Fix race conditions in CloseCampaign by blocking

Administrator requested to merge mrn/block-closing-campaign into main

Created by: mrnugget

This is one of the PRs to fix #12827.

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.

Merge request reports