Batch moodle:sync Moodle writes across users to stop stacking

[Celeo]

Summary

Follow-up to #112. That PR fixed the whole-table existence-check HTTP storm (getUserId() called once per VATUSA user just to check "is this CID in Moodle?"), but live production evidence after deploying it showed moodle:sync runs are still stacking — just more slowly than before. This PR batches the remaining per-user write calls across users, not just within a user, to get the whole run's HTTP call count down from tens of thousands to roughly one hundred.

Why #112 wasn't enough

#112 replaced the existence-check loop with one query (VATUSAMoodle::getAllUserIdMap()) and batched each matched user's own writes into per-user bulk calls. That fixed the dominant cost, but the write path in sync() still ran once per matched user — and the matched set is large: a direct query against Moodle's DB confirmed 25,152 users currently match (deleted = 0, non-blank idnumber). For each of those, the old-#112 code still made:

• 1x core_user_update_users (single-item — never batched by Fix moodle:sync stacking by cutting per-user Moodle HTTP call volume #112)
• 1x core_cohort_add_cohort_members (batched within one user by Fix moodle:sync stacking by cutting per-user Moodle HTTP call volume #112, but still one HTTP call per user)
• 1x core_role_assign_roles (same)
• 0-1x enrol_manual_enrol_users for staff/instructor/mentor users, preceded by getAllSubcategories/getCoursesInCategory HTTP calls that weren't memoized, so the same category's course list could be re-fetched by HTTP on every staff user who touched it

That's still on the order of 75,000-100,000 HTTP round-trips for a full run. After deploying #112:

• A manual foreground run (php artisan moodle:sync) ran 36+ minutes without finishing (backgrounded past the shell's timeout).
• Four consecutive scheduled ticks (06:00, 09:00, 12:00, 15:00 UTC) each logged Starting scheduled task: moodle:sync with zero matching Finished lines.
• ps on the api-worker pod showed two live overlapping moodle:sync processes (3h55m and 55m elapsed) before the older one was manually killed to stop the immediate bleeding.

Runtime dropped a lot from the pre-#112 17+ hours, but still exceeds the withoutOverlapping(120) 2-hour lock window, so the same stacking mechanism recurs — just on a longer clock.

What's changing

• MoodleSync::sync() is split into a pure compute step, computeSyncItems(). It keeps every existing cohort/role/enrolment decision (rating cohorts, home facility, visiting facilities, STU/INS/TA/CBT/FACCBT/MTR branches — all unchanged) but returns the computed items as arrays instead of immediately calling the Moodle bulk methods. Per-user clearUserCohorts()/clearUserRoles() stay per-user since they're direct DB deletes, not HTTP, and were never the bottleneck.
• handle()'s chunk(1000) loop now accumulates items across the whole chunk and flushes 4 bulk HTTP calls per chunk (update/cohorts/roles/enrolments) instead of ~4 per user. For 25,152 users at chunk size 1000, that's ~26 chunks × 4 calls ≈ ~104 HTTP calls total for a full run, down from ~75,000-100,000.
• New VATUSAMoodle::updateUsersBulk(array $items), mirroring the existing assignCohortsBulk/assignRolesBulk/enrolUsersBulk pattern — core_user_update_users was the one per-user HTTP call Fix moodle:sync stacking by cutting per-user Moodle HTTP call volume #112 left unbatched.
• getCoursesInCategory() is now memoized the same way getCategories() already was, keyed by category id, so a staff user's category tree isn't re-fetched by HTTP if another user already pulled the same category's courses earlier in the run.
• Added logger() calls around the bulk pass: matched-user count at start, per-chunk item counts, a final run summary, and a log-and-rethrow around each of the 4 bulk calls per chunk. Right now a run is a black box until it finishes (or never does) — tonight's incident was only diagnosable via ps and a raw Moodle DB query. These lines mean the next run's actual progress is visible in application logs in real time, and a mid-run failure names the exact chunk and call that broke instead of an opaque Moodle error.
• The single-user CLI path (moodle:sync {cid}) is preserved with the same create-or-update decision as before, just routed through computeSyncItems() and flushed immediately as single-item bulk calls (functionally identical to the old immediate-flush behavior, aside from one intentionally-accepted redundant updateUsersBulk() call after createUser() for brand-new users — harmless since createUser() already sets those same fields).

Behavior preservation

No changes to Kernel.php, withoutOverlapping(120), or the signature of any existing single-item VATUSAMoodle method (getUserId, assignCohort, assignRole, enrolUser, createUser, updateUser) — those still serve other external callers (ULSHelper, AcademyController, MoodleCompetency) unchanged. All new behavior is additive (updateUsersBulk(), the getCoursesInCategory() memoization) or a refactor of MoodleSync's internals that preserves the same per-user decision logic, just deferring the HTTP flush.

Testing

• php -l clean on both changed files.
• ./vendor/bin/phpunit: 5/5 passing (unchanged MoodleUserIdMapTest from Fix moodle:sync stacking by cutting per-user Moodle HTTP call volume #112; no new local test added since the bulk-flush shape isn't testable without a live/mock Moodle — same limitation noted in Fix moodle:sync stacking by cutting per-user Moodle HTTP call volume #112).
• Ran the single-user CLI sanity check (moodle:sync 1640903) against real prod Moodle after deploying Fix moodle:sync stacking by cutting per-user Moodle HTTP call volume #112; will re-run after this deploys too.

Verification checklist for this PR (real Moodle, no isolated staging Moodle exists — confirmed current-dev has zero Moodle secrets/DB connection configured)

• Deploy to current prod (same constraint as Fix moodle:sync stacking by cutting per-user Moodle HTTP call volume #112 — there's nowhere else to test against).
• kubectl -n current exec <api-worker-pod> -- php artisan moodle:sync — manual foreground run, timed. Expect it to actually complete (not need backgrounding past a 10-minute shell timeout).
• Watch kubectl -n current exec <pod> -- ps -o pid,etime,rss,args during the run — confirm memory stays bounded (should not grow much beyond one chunk's worth of accumulated arrays at a time).
• kubectl -n current logs <pod> --since=1h | grep -i moodle:sync — confirm the new per-chunk log lines appear and a Finished scheduled task: moodle:sync line finally shows up.
• After the next scheduled ticks, confirm no more than one live moodle:sync process at any ps snapshot, and Starting/Finished log pairs match up every 3 hours.
• Spot-check a sampled user's cohorts/roles/enrolments in Moodle match what the old per-item path would have produced (no behavioral drift from cross-user batching).

Risks

• Bulk-call size. Each chunk can send up to 1000 array entries in a single core_cohort_add_cohort_members/core_role_assign_roles/enrol_manual_enrol_users/core_user_update_users call. Moodle's Web Service layer is documented as bulk-capable for all of these, but some Moodle configs cap request body size, and the underlying vendored MoodleRest client serializes these as form-encoded POST params, which can get large at 1000 entries. If a chunk of 1000 proves too big in practice, the fix is simply lowering the chunk size (either the outer User::chunk() size or a separate inner flush-batch size) — no structural change needed.
• Partial-batch failure semantics. A single bad record in a 1000-entry bulk call could fail the whole chunk's call for that item type. The new log-and-rethrow wrapper will at least surface which chunk and which call failed immediately, rather than silently completing with partial data — but a failure now affects up to 1000 users' worth of one item type at once, versus one user before. Given Moodle's array-based Web Service functions are documented as processing each array entry independently (a bad entry shouldn't 500 the whole call), this is believed low-risk, but hasn't been confirmed against Moodle's actual error-handling behavior at this batch size.
• Still not a hard runtime guarantee. If ~104 HTTP calls still doesn't clear the 120-minute window (unlikely, but the manual dev-side timing test that would confirm this wasn't feasible pre-merge — no isolated Moodle to test against), the next lever is Lever 3 from the original plan: a structural stacking backstop (heartbeat lock independent of the scheduler mutex) rather than continuing to chase raw throughput.

🤖 Generated with Claude Code