Task manager: release task locks on stop, recheck claims held elsewhere #35

Merged

jcoffey-dev merged 1 commits from fix/task-lock-recovery into main

2026-09-24 15:38:39 +00:00

jcoffey-dev commented

2026-09-24 15:19:36 +00:00

Owner

Status of the root cause

Not confirmed. In the 3-node PostgreSQL + NATS rehearsal, 47 IndexTrace, 5 IndexTrace (Retry) and 27 IndexDocument tasks stayed pending long after the one-hour task lock should have expired. I read the task manager end to end (claim scan, local holds, distributed lock, workers, update_tasks) and found no path that keeps a task claimed forever under normal operation: a task claimed by a node that dies normally runs elsewhere within about an hour plus a scan. This PR fixes every mechanism I found that stretches that, or can make it permanent. It doesn't claim to fix the specific failure the rehearsal saw.

The counts point at the claiming node's workers not running tasks at all. 47 and 27 are both under the index channel capacity (max(indexBatchSize, 10) = 100), and the tasks show as never run (Pending), not failed. If the rehearsal logs are still around, these would settle it:

Error sending task. (ThreadError): a dead worker. See 3 below.
Repeated task-manager.task-locked for the same ids on every node after the hour: the lock was being re-taken.
Failed to remove task(s) from queue / Task was deleted while being processed: the post-run write failed, so the tasks went back to Pending.

What was found and changed

A graceful stop never released its locks (main.rs exits without touching them). The server now tracks the task locks it holds (common::ipc::TaskLocks, per server). After the shutdown signal, release_task_locks() stops new claims and releases every held lock, so other nodes can pick those tasks up at their next scan. A SIGKILL still leaves the locks to expire.
A failed claim held the task locally for a full lock lifetime (manager.rs, Locked.expires = now + DEFAULT_LOCK_EXPIRY + 1). Suppose the node that holds the lock claimed it just after this node's scan began, or runs a clock ahead of this one. Then the local hold ran out a moment before the lock did, and was set for another full hour: two hours in all. Failed claims are now retried every 5 minutes (a twelfth of the lock lifetime).
- The task manager also wakes up for those retries. Before, a node without a coordinator only woke on its next due task, a notify, or 5 minutes.
A panicking worker starved the whole cluster. A worker is one tokio task per task type. If it panicked, its receiver was gone for good, but the scan still saw channel capacity. So it kept claiming that type's tasks, failing the hand-over (Error sending task.) and keeping the lock. Because that node saw each task first, it re-took each lock the moment it expired, and no other node ever got them.
- Each batch now runs on its own task. A panic is logged (Task worker failed), the batch's locks are released, and the worker keeps going.
- A failed hand-over now releases the lock too.
- (index() has _ => unreachable!(), and the general worker's run_task does the same for index tasks. A task whose stored type doesn't match its queue entry would hit those.)
A claimed task that couldn't be read, or was already gone, kept its lock for the hour. It is now released.
An IndexDocument task for a file hit continue and returned no result. update_tasks pairs results with tasks by position, so every later result in that batch landed on the wrong task. It now returns Ignored. Nothing queues such a task today.

The lock lifetime stays one hour.

Tests

store::task_locks::task_lock_tests (new) plays a second node by writing its task locks straight into the shared in-memory store, the way a node that claimed tasks and died leaves them. It uses a 12 s lock lifetime.

Tasks the other node claimed stay put while its locks hold, then run on this node once they expire, without anything else waking the task manager.
When the other node's locks outlive this node's view of them (18 s against 12 s), the tasks run soon after those locks expire, not a whole lifetime later.
After release_task_locks(), the other node can claim this node's tasks at once, and this node claims nothing more.

Runs:

RocksDb, Sqlite and PostgreSql: pass (about 31 s each).
With the old recheck behavior put back, it fails ("4 task(s) still pending after 22 s").
store::store_tests (RocksDb): pass.
system::system_tests (RocksDb), which covers the task manager tests: passed twice. Two earlier runs failed, once in email delivery (SMTP read timeout) and once in the task manager step. Current main also failed the task manager step once here, so I'm treating those as flaky on this loaded machine.

## Status of the root cause **Not confirmed.** In the 3-node PostgreSQL + NATS rehearsal, 47 IndexTrace, 5 IndexTrace (Retry) and 27 IndexDocument tasks stayed pending long after the one-hour task lock should have expired. I read the task manager end to end (claim scan, local holds, distributed lock, workers, `update_tasks`) and found no path that keeps a task claimed forever under normal operation: a task claimed by a node that dies normally runs elsewhere within about an hour plus a scan. This PR fixes every mechanism I found that stretches that, or can make it permanent. It doesn't claim to fix the specific failure the rehearsal saw. The counts point at the claiming node's workers not running tasks at all. 47 and 27 are both under the index channel capacity (`max(indexBatchSize, 10)` = 100), and the tasks show as never run (Pending), not failed. If the rehearsal logs are still around, these would settle it: - `Error sending task.` (ThreadError): a dead worker. See 3 below. - Repeated `task-manager.task-locked` for the same ids on every node after the hour: the lock was being re-taken. - `Failed to remove task(s) from queue` / `Task was deleted while being processed`: the post-run write failed, so the tasks went back to Pending. ## What was found and changed 1. **A graceful stop never released its locks** (`main.rs` exits without touching them). The server now tracks the task locks it holds (`common::ipc::TaskLocks`, per server). After the shutdown signal, `release_task_locks()` stops new claims and releases every held lock, so other nodes can pick those tasks up at their next scan. A SIGKILL still leaves the locks to expire. 2. **A failed claim held the task locally for a full lock lifetime** (`manager.rs`, `Locked.expires = now + DEFAULT_LOCK_EXPIRY + 1`). Suppose the node that holds the lock claimed it just after this node's scan began, or runs a clock ahead of this one. Then the local hold ran out a moment before the lock did, and was set for another full hour: two hours in all. Failed claims are now retried every 5 minutes (a twelfth of the lock lifetime). - The task manager also wakes up for those retries. Before, a node without a coordinator only woke on its next due task, a notify, or 5 minutes. 3. **A panicking worker starved the whole cluster.** A worker is one tokio task per task type. If it panicked, its receiver was gone for good, but the scan still saw channel capacity. So it kept claiming that type's tasks, failing the hand-over (`Error sending task.`) and keeping the lock. Because that node saw each task first, it re-took each lock the moment it expired, and no other node ever got them. - Each batch now runs on its own task. A panic is logged (`Task worker failed`), the batch's locks are released, and the worker keeps going. - A failed hand-over now releases the lock too. - (`index()` has `_ => unreachable!()`, and the general worker's `run_task` does the same for index tasks. A task whose stored type doesn't match its queue entry would hit those.) 4. **A claimed task that couldn't be read, or was already gone, kept its lock for the hour.** It is now released. 5. **An IndexDocument task for a file** hit `continue` and returned no result. `update_tasks` pairs results with tasks by position, so every later result in that batch landed on the wrong task. It now returns `Ignored`. Nothing queues such a task today. The lock lifetime stays one hour. ## Tests `store::task_locks::task_lock_tests` (new) plays a second node by writing its task locks straight into the shared in-memory store, the way a node that claimed tasks and died leaves them. It uses a 12 s lock lifetime. 1. Tasks the other node claimed stay put while its locks hold, then run on this node once they expire, without anything else waking the task manager. 2. When the other node's locks outlive this node's view of them (18 s against 12 s), the tasks run soon after those locks expire, not a whole lifetime later. 3. After `release_task_locks()`, the other node can claim this node's tasks at once, and this node claims nothing more. Runs: - `RocksDb`, `Sqlite` and `PostgreSql`: pass (about 31 s each). - With the old recheck behavior put back, it fails ("4 task(s) still pending after 22 s"). - `store::store_tests` (RocksDb): pass. - `system::system_tests` (RocksDb), which covers the task manager tests: passed twice. Two earlier runs failed, once in email delivery (SMTP read timeout) and once in the task manager step. Current `main` also failed the task manager step once here, so I'm treating those as flaky on this loaded machine.

jcoffey-dev added 1 commit 2026-09-24 15:19:36 +00:00

Task manager: release task locks on stop, recheck claims held elsewhere

ci / build (pull_request) Successful in 17m2s

Details

ci / fork-checks (pull_request) Successful in 18s

Details

7c80a12d75

A cluster rehearsal (PostgreSQL + NATS) left index tasks pending well
past the one-hour task lock after the node that claimed them was stopped
or killed. The exact cause there isn't confirmed; this closes every path
found in the task manager that stretches a takeover past the lock, or
keeps a task claimed without running it:

- A graceful stop never released the locks it held, so every task the
  node had claimed stayed blocked for an hour. The server now tracks the
  locks it holds (common::ipc::TaskLocks) and, once the shutdown signal
  arrives, stops claiming and releases them before exiting.
- A node that failed to claim a task (another node held it) set its own
  local hold for a full lock lifetime from that scan. If the holder
  claimed it just after the scan began, or ran on a clock ahead, that
  hold ran out a moment before the lock did and was set for another
  hour: two hours in all. Such claims are now tried again every five
  minutes (a twelfth of the lock lifetime), and the task manager wakes
  up for them: before, a node without a coordinator could sleep up to
  five minutes past the recheck, or until something else woke it.
- A worker that panicked took its task type down on that node for good,
  while the scan kept claiming that type's tasks and failing to hand them
  over, re-taking each lock as it expired and so starving every other
  node of them. Each batch now runs on a task of its own; a panic is
  logged, the batch's locks are released and the worker carries on. A
  failed hand-over releases the lock too.
- A claimed task the worker couldn't read, or found gone, kept its lock
  for the hour. It is released.
- An IndexDocument task for a file (not indexed) returned no result,
  which shifted every later result in the batch onto the wrong task in
  update_tasks. It returns Ignored. Nothing queues such a task today.

The lock lifetime stays one hour; it now lives per server so the tests
can shorten it.

store::task_locks::task_lock_tests plays a second node by writing its
locks straight into the in-memory store: tasks it claimed and abandoned
run here once its locks expire, including locks that outlive this node's
view of them, and a graceful stop hands this node's locks back at once
and claims nothing more. It passes on RocksDB, SQLite and PostgreSQL.
With the old recheck it fails.

jcoffey-dev merged commit c974a0918e into main

2026-09-24 15:38:39 +00:00

jcoffey-dev deleted branch fix/task-lock-recovery

2026-09-24 15:38:39 +00:00

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: inbuxa/inbuxa-server#35