#4056 avoid explicit rowlock in taskWaitCheck
Merged by tkopecek. Opened by mikem.
mikem/koji task-wait-deadlock  into  master

Download 4056.patch

There's not much reason to explicitly lock the row in advance. The update itself acquires a lock and all we're doing is clearing the awaited flag.

Related: https://pagure.io/koji/issue/4057

:thumbsup:

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-ready

The original row lock was added in ae1db8fb03cc212e6f00f203bd974270fecdd989 and may be related to work on maven chain builds.

I added a sort here so that we update the rows in order. That sort of sorting has been key in avoiding other deadlock situations. Grabbing row locks could deadlock too for that matter.

The main reason to row lock is to ensure the data is consistent between our query and our update, but:

  • finished tasks shouldn't change state any further
  • it's harmless to re-clear the awaited flag
  • I think it's harmless for two racing taskWait calls to both report the same finished task, in the unlikely event of such a race

I was finally able to replicate the issue locally (see notes in #4057), and these changes do indeed seem to fix it.

However, for validation we also want to make sure it doesn't introduce any new issues. I think that would mean some sort of high task load simulation with lots of subtasks. Maybe using something like the subtask task handler.

Metadata Update from @tkopecek:
- Pull-request untagged with: testing-ready
- Pull-request tagged with: no_qe

Commit df86feab fixes this pull-request

Pull-Request has been merged by tkopecek

Metadata