Skip to content

Unable to enqueue changeset to be reconciled while it's being reconciled

Created by: mrnugget

Status quo

In order to enqueue a changeset to be picked up by the reconciler, we set its reconciler_state column to queued.

The reconciler then dequeues the changeset by setting the reconciler_state to processing:

https://github.com/sourcegraph/sourcegraph/blob/74ad60c5b67a800804279e036f8c849e14fc02c9/internal/workerutil/dbworker/store/store.go#L298-L315

Once it's done, it sets the state to completed or errored.

Problem

We want to move the closing of changesets into the background (https://github.com/sourcegraph/sourcegraph/issues/12644) by setting a "please close this" boolean on the changeset and enqueuing the changeset so that the reconciler picks it up and closes it.

We also want to allow this while a changeset is being processed by the reconciler, because the reconciler could — in the future — always be processing a changeset and that shouldn't stop the user from marking a changeset as "to be closed". See https://github.com/sourcegraph/sourcegraph/pull/12782#issue-463915652 for more context.

But here is the problem: if we enqueue a changeset while it's being processed by setting reconciler_state = 'queued' then that value will simply be overwritten by the reconciler once it's done with the current processing.

In other words: we can't enqueue a changeset to be processed again while it's being processed.

That's due to the architecture of the underlying dbworker/store, which assumes that every job is a one-off job that ends in a terminal state and is not re-enqueued again.

Possible Solution 1

We split the reconciler_state up into two fields:

  • enqueued: bool
  • processing_state: string

In order to enqueue a changeset, we set enqueued = TRUE.

We then need to change the dbworker/store so that the selectCandidateQuery allows us to set these fields, like this (see the selectCandidateQuery above):


WITH candidate AS (
  SELECT {id} FROM %s
  WHERE
    -- This is new
    ({state} = 'queued' OR (enqueued IS TRUE)) AND
    ({process_after} IS NULL OR {process_after} <= NOW())
    %s
  ORDER BY %s
  FOR UPDATE SKIP LOCKED
  LIMIT 1
)
UPDATE %s
SET
  -- This is new
  enqueued = FALSE,
  {state} = 'processing',
  {started_at} = NOW()
WHERE {id} IN (SELECT {id} FROM candidate)
RETURNING {id}

We could then compute the Changeset.ReconcilerState() by looking at both fields.

And we could always set the enqueued, even while a changeset is being processed, because the field acts like a dirty bit that's only cleared when dequeued. If we set it to true again, while it's being processed, the reconciler will pick it up again after it's done processing.

Possible Solution 2

We could switch away from using the changesets table as re-usable jobs that are re-executed again and again and instead create a new reconciler_job whenever we want the reconciler to look at the changeset.

That would require another table, of course, and that we need to fetch the jobs whenever we want to check the status of a changeset. We could make that easier with a database view, though, in which we always join the changesets against the reconciler_jobs.

But then we'd have the problem that we create a new job while the old one is being processed and both would try to modify the changeset, so we'd need to lock the changeset.

I'm leaning towards solution 1.