Diff moodle:sync cohort membership instead of clearing and re-inserting every run

[Celeo]

Problem

moodle:sync clears and re-inserts every matched user's cohort memberships on every run (~28k writes/pass), whether or not anything changed since the previous run three hours earlier. Each cohort write triggers Moodle's enrol_cohort sync, which bumps mdl_course.cacherev. The rating cohorts (OBS, S1–C1, ZZN, …) all enrol into a single course — "Trainee Orientation", mdl_course.id = 8 — so essentially every cohort insert funnels a cacherev UPDATE onto that one row.

Those UPDATEs serialize on the row lock. Observed live at the managed Moodle DB during an active run:

• 16 concurrent UPDATE mdl_course SET cacherev=… WHERE id='8' stuck 20–110s in updating state
• Innodb_row_lock_current_waits = 15 (purge lag was fine — history length 43, so this is pure lock queuing)
• Throughput collapsing ~28× over a single run: 20,675 cohort writes in the first hour down to 728 in the fourth, as the lock queue on that one row grows

Prior rounds (#112/#113 batching, plus a pacing sleep) cut the HTTP call count but not the per-write cacherev storm, so runs kept degrading and overflowing the withoutOverlapping(120) window.

Meanwhile, organic (non-sync) cohort changes run at single digits per hour — so roughly 99.9% of that churn was rewriting identical data.

Change

Diff each user's desired cohort set against their current membership and write only the actual adds/removes:

• VATUSAMoodle::getCohortMembershipsForUsers() reads current memberships for a whole chunk of users in one query against the moodle DB (indexed on userid).
• VATUSAMoodle::getCohortIdMap() memoizes the cohort idnumber → id map (one query, 269 rows).
• VATUSAMoodle::removeCohortsBulk() removes stale memberships via the Web Service (not a direct DB delete) so enrol_cohort unenrols the user from linked courses properly.
• MoodleSync::diffCohorts() computes the delta; computeSyncItems() no longer calls clearUserCohorts() and now returns desired cohort idnumbers for the caller to diff.

The end state is identical to the old clear-then-reassign — the user ends up in exactly the desired cohorts and no others — but a steady-state run writes almost nothing, so the cacherev row-lock pileup disappears at the source.

Indexing on the affected tables is stock-Moodle-correct, and the cacherev UPDATE is a primary-key point update, so there was nothing to fix at the index layer — the only lever is not generating the redundant writes.

Also: stop retrying failed sub-batches

A write that timed out on our side usually keeps running server-side holding locks; retrying just adds a second writer to the same contended rows and makes the pileup worse. Since moodle:sync recomputes full desired state every run, a skipped sub-batch self-heals on the next pass, so a failed sub-batch is now logged and skipped rather than retried.

Scope

Roles are still cleared-and-reassigned per user for now — a follow-up can apply the same diff to the role path. This PR targets the cohort path, which is the dominant source of the cacherev contention.

Testing

• tests/Unit/MoodleCohortDiffTest.php covers the diff semantics: steady-state emits no writes, missing memberships are added, stale ones removed, unknown desired idnumbers are skipped (never created), and duplicate desired idnumbers dedupe.
• php -l clean on both changed files; full suite green (11 tests, 20 assertions).
• No isolated staging Moodle exists, so the live verification is the same as prior rounds: deploy, run a full pass, and watch information_schema.processlist / Innodb_row_lock_current_waits and per-hour mdl_cohort_members write counts. Success = a steady-state run writes tens (not tens of thousands) and finishes well inside the lock window with no stuck cacherev updates.

🤖 Generated with Claude Code