From d4c5efe751d73127ffe458e5ba6b28e745b62e7b Mon Sep 17 00:00:00 2001 From: Celeo Date: Tue, 21 Jul 2026 09:01:55 -0700 Subject: [PATCH] Batch moodle:sync Moodle writes across users, not just within a user PR #112 removed the per-user Moodle existence-check HTTP storm, but live prod evidence showed runs still stack: the 25,152-user matched set still drives one core_user_update_users, one core_cohort_add_cohort_members, one core_role_assign_roles, and (for staff) one enrol_manual_enrol_users call per user, ~75-100k HTTP round-trips total. Scheduled runs since #112's deploy never completed (no "Finished scheduled task" log line across four consecutive 3-hour ticks), and a manual foreground run ran 36+ minutes without finishing before being backgrounded. This splits sync() into a pure compute step (computeSyncItems()) that returns per-user update/cohort/role/enrolment items instead of flushing them immediately, and has handle()'s chunk(1000) loop accumulate those across the whole chunk before flushing 4 bulk HTTP calls per chunk instead of per user (~104 calls total for the full table instead of ~75-100k). Also: - Add VATUSAMoodle::updateUsersBulk(), mirroring the existing assignCohortsBulk/assignRolesBulk/enrolUsersBulk bulk helpers - core_user_update_users was the one per-user HTTP call #112 left unbatched. - Memoize getCoursesInCategory() the same way getCategories() already is, since staff/instructor users were re-fetching the same category's course list by HTTP on every occurrence. - Add 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 bulk call) so a run's progress and any mid-run failure are visible in logs instead of requiring a live kubectl/ps/DB investigation like tonight's. Single-user CLI path (moodle:sync {cid}) and all existing single-item VATUSAMoodle methods (getUserId, assignCohort, assignRole, enrolUser, createUser, updateUser) are unchanged - only new additive bulk/memoized methods were introduced. Kernel.php scheduling and withoutOverlapping(120) are untouched. Co-authored-by: Claude Sonnet 5 --- app/Classes/VATUSAMoodle.php | 32 +++++- app/Console/Commands/MoodleSync.php | 153 ++++++++++++++++++++++------ 2 files changed, 150 insertions(+), 35 deletions(-) diff --git a/app/Classes/VATUSAMoodle.php b/app/Classes/VATUSAMoodle.php index 60d5c71..8a3a07e 100644 --- a/app/Classes/VATUSAMoodle.php +++ b/app/Classes/VATUSAMoodle.php @@ -56,6 +56,9 @@ class VATUSAMoodle extends MoodleRest /** @var array|null Memoized result of getCategories() for the lifetime of this instance */ private $categoriesCache = null; + /** @var array Memoized results of getCoursesInCategory(), keyed by category id */ + private $coursesInCategoryCache = []; + /** @var int Seconds to bound each Moodle HTTP request to */ private const HTTP_TIMEOUT_SECONDS = 30; @@ -400,6 +403,26 @@ public function updateUser(User $user, int $id) ], self::METHOD_POST); } + /** + * Bulk update Users + * + * @param array $items Each item: ['id' => int, 'fname' => string, 'lname' => string, 'email' => string] + * + * @return mixed + * @throws Exception + */ + public function updateUsersBulk(array $items) + { + return $this->request("core_user_update_users", [ + 'users' => array_values(array_map(fn ($item) => [ + 'id' => $item['id'], + 'firstname' => $item['fname'], + 'lastname' => $item['lname'], + 'email' => $item['email'] + ], $items)) + ], self::METHOD_POST); + } + /** * Create Cohort * @@ -673,9 +696,14 @@ function getContext( function getCoursesInCategory( int $catid = null ) { - $params = $catid ? ["field" => "category", "value" => $catid] : []; + $key = $catid ?? 0; + if (!array_key_exists($key, $this->coursesInCategoryCache)) { + $params = $catid ? ["field" => "category", "value" => $catid] : []; + $this->coursesInCategoryCache[$key] = $this->request("core_course_get_courses_by_field", + $params)["courses"]; + } - return $this->request("core_course_get_courses_by_field", $params)["courses"]; + return $this->coursesInCategoryCache[$key]; } public diff --git a/app/Console/Commands/MoodleSync.php b/app/Console/Commands/MoodleSync.php index 4eb963c..4b38756 100644 --- a/app/Console/Commands/MoodleSync.php +++ b/app/Console/Commands/MoodleSync.php @@ -17,7 +17,7 @@ class MoodleSync extends Command * * @var string */ - protected $signature = 'moodle:sync + protected $signature = 'moodle:sync {user? : CID of a single user to sync}'; /** @@ -56,47 +56,138 @@ public function handle() return 0; } - $this->sync($user); + $items = $this->computeSyncItems($user, $this->resolveMoodleId($user)); + $this->flushItems($items); return 0; } //Syncronize Users $moodleIds = $this->moodle->getAllUserIdMap(); - User::with('visits', 'academyCompetencies.course', 'roles')->chunk(1000, function ($users) use ($moodleIds) { - foreach ($users as $user) { - if ($moodleIds->has($user->cid)) { - $this->sync($user, (int) $moodleIds[$user->cid]); + logger()->info("moodle:sync starting bulk pass: {$moodleIds->count()} Moodle-linked users"); + + $chunkNum = 0; + $totals = ['users' => 0, 'updates' => 0, 'cohorts' => 0, 'roles' => 0, 'enrolments' => 0]; + + User::with('visits', 'academyCompetencies.course', 'roles')->chunk(1000, + function ($users) use ($moodleIds, &$chunkNum, &$totals) { + $chunkNum++; + $updates = $cohorts = $roles = $enrolments = []; + + foreach ($users as $user) { + if (!$moodleIds->has($user->cid)) { + continue; + } + $items = $this->computeSyncItems($user, (int) $moodleIds[$user->cid]); + $updates[] = $items['update']; + $cohorts = array_merge($cohorts, $items['cohorts']); + $roles = array_merge($roles, $items['roles']); + $enrolments = array_merge($enrolments, $items['enrolments']); } - } - }); + + $this->flushBulk($chunkNum, 'update', $updates, fn ($items) => $this->moodle->updateUsersBulk($items)); + $this->flushBulk($chunkNum, 'cohorts', $cohorts, + fn ($items) => $this->moodle->assignCohortsBulk($items)); + $this->flushBulk($chunkNum, 'roles', $roles, fn ($items) => $this->moodle->assignRolesBulk($items)); + $this->flushBulk($chunkNum, 'enrolments', $enrolments, + fn ($items) => $this->moodle->enrolUsersBulk($items)); + + $totals['users'] += count($users); + $totals['updates'] += count($updates); + $totals['cohorts'] += count($cohorts); + $totals['roles'] += count($roles); + $totals['enrolments'] += count($enrolments); + + logger()->info("moodle:sync chunk {$chunkNum}: " . count($users) . " users, " + . count($updates) . " updates, " . count($cohorts) . " cohorts, " + . count($roles) . " roles, " . count($enrolments) . " enrolments"); + }); + + logger()->info("moodle:sync finished bulk pass: {$totals['users']} users seen, " + . "{$totals['updates']} updates, {$totals['cohorts']} cohorts, " + . "{$totals['roles']} roles, {$totals['enrolments']} enrolments across {$chunkNum} chunks"); return 0; } /** - * Synchronize Roles + * Resolve a user's Moodle id for the single-user CLI path (create-or-update decision). * * @param \App\User $user - * @param int|null $knownId Moodle user id, if already known (bulk sync path) — skips the - * redundant existence-check lookup that the single-user CLI path - * still needs to decide create-vs-update. + * + * @return int + * @throws \Exception + */ + private function resolveMoodleId(User $user): int + { + if ($id = $this->moodle->getUserId($user->cid)) { + return $id; + } + + return $this->moodle->createUser($user)[0]["id"]; + } + + /** + * Flush one bulk call for a chunk, logging and rethrowing on failure so a mid-run + * failure points straight at which chunk and which call broke. + * + * @param int $chunkNum + * @param string $kind + * @param array $items + * @param callable $call * * @throws \Exception */ - private function sync(User $user, ?int $knownId = null) + private function flushBulk(int $chunkNum, string $kind, array $items, callable $call) { - //Update or Create - if ($knownId !== null) { - $id = $knownId; - $this->moodle->updateUser($user, $id); - } elseif ($id = $this->moodle->getUserId($user->cid)) { - //Update Information - $this->moodle->updateUser($user, $id); - } else { - //Create User - $id = $this->moodle->createUser($user)[0]["id"]; + if (empty($items)) { + return; + } + + try { + $call($items); + } catch (\Exception $e) { + logger()->error("moodle:sync chunk {$chunkNum}: {$kind} bulk call failed: {$e->getMessage()}"); + throw $e; } + } + + /** + * Flush a single user's computed items immediately (single-user CLI path). + * + * @param array $items + * + * @throws \Exception + */ + private function flushItems(array $items) + { + $this->moodle->updateUsersBulk([$items['update']]); + if (!empty($items['cohorts'])) { + $this->moodle->assignCohortsBulk($items['cohorts']); + } + if (!empty($items['roles'])) { + $this->moodle->assignRolesBulk($items['roles']); + } + if (!empty($items['enrolments'])) { + $this->moodle->enrolUsersBulk($items['enrolments']); + } + } + + /** + * Compute what needs to happen in Moodle for a user — cohorts, roles, and enrolments — + * without making any HTTP calls itself. Callers accumulate these across many users and + * flush them as bulk calls (whole-table sync), or flush a single user's items + * immediately (single-user CLI path). Cohort/role clearing stays per-user since it's a + * direct DB delete (cheap, not the HTTP bottleneck this split targets). + * + * @param \App\User $user + * @param int $id Moodle user id + * + * @return array{update: array, cohorts: array, roles: array, enrolments: array} + * @throws \Exception + */ + private function computeSyncItems(User $user, int $id): array + { $facilities = $user->visits->pluck('facility')->merge(collect($user->facility))->unique(); //Assign Cohorts @@ -119,10 +210,6 @@ private function sync(User $user, ?int $knownId = null) $cohorts[] = $facility . "-V"; //Facility level visitor $cohorts[] = "$facility-" . Helper::ratingShortFromInt($user->rating); //Facility level rating } - if (!empty($cohorts)) { - $this->moodle->assignCohortsBulk(array_map(fn ($cnumber) => ['uid' => $id, 'cnumber' => $cnumber], - $cohorts)); - } //Clear Roles $this->moodle->clearUserRoles($id); @@ -203,11 +290,11 @@ private function sync(User $user, ?int $knownId = null) $roles[] = ['uid' => $id, 'cid' => $this->moodle->getCategoryFromShort($user->facility, true), 'role' => "MTR", 'context' => "coursecat"]; }*/ - if (!empty($roles)) { - $this->moodle->assignRolesBulk($roles); - } - if (!empty($enrolments)) { - $this->moodle->enrolUsersBulk($enrolments); - } + return [ + 'update' => ['id' => $id, 'fname' => $user->fname, 'lname' => $user->lname, 'email' => $user->email], + 'cohorts' => array_map(fn ($cnumber) => ['uid' => $id, 'cnumber' => $cnumber], $cohorts), + 'roles' => $roles, + 'enrolments' => $enrolments, + ]; } }