From 55605f65160d8aa70ac14d6ed68d043ed64c748f Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 28 Oct 2026 23:43:16 +0100 Subject: [PATCH] fix(github-runners): select runner servers with a UNION of server ids An OR of two correlated EXISTS subqueries made Postgres estimate a per-server scan of the runner executions, so JIT compilation took most of the time of the per-minute reconcile query. Select the server ids from both runner tables with one UNION and index executions by status and server. Co-Authored-By: Claude Opus 5.5 --- app/Jobs/ReconcileGithubRunnersJob.php | 24 +++++-- ...ex_for_active_github_runner_executions.php | 41 ++++++++++++ ...concileGithubRunnersDisabledConfigTest.php | 67 +++++++++++++++++++ 3 files changed, 127 insertions(+), 5 deletions(-) create mode 100644 database/migrations/2026_10_28_224219_add_index_for_active_github_runner_executions.php diff --git a/app/Jobs/ReconcileGithubRunnersJob.php b/app/Jobs/ReconcileGithubRunnersJob.php index f3adb513c4..e3447c87aa 100644 --- a/app/Jobs/ReconcileGithubRunnersJob.php +++ b/app/Jobs/ReconcileGithubRunnersJob.php @@ -43,11 +43,7 @@ class ReconcileGithubRunnersJob implements ShouldBeUnique, ShouldQueue public function handle(): void { - // Disabling keeps running jobs, so a disabled server is reconciled until its last runner is done. - Server::query() - ->where(fn (Builder $query) => $query - ->whereHas('githubRunnerConfig', fn (Builder $config) => $config->where('is_enabled', true)) - ->orWhereHas('githubRunnerExecutions', fn (Builder $execution) => $execution->whereIn('status', GithubRunnerStatus::occupying()))) + $this->serversToReconcile() ->with('settings') ->get() ->filter(fn (Server $server) => $server->isFunctional()) @@ -57,6 +53,24 @@ class ReconcileGithubRunnersJob implements ShouldBeUnique, ShouldQueue $this->reconcileQueued(); } + /** + * Servers with enabled runners or with executions that still occupy a runner. Disabling keeps + * running jobs, so a disabled server is reconciled until its last runner is done. The server ids + * come from one UNION: an OR of two EXISTS subqueries makes Postgres estimate a per-server scan + * of the executions and spend most of the query time on JIT compilation. + * + * @return Builder + */ + public function serversToReconcile(): Builder + { + return Server::query()->whereIn('id', GithubRunnerConfig::query() + ->select('server_id') + ->where('is_enabled', true) + ->union(GithubRunnerExecution::query() + ->select('server_id') + ->whereIn('status', GithubRunnerStatus::occupying()))); + } + /** * Removes executions that still occupy a runner of a deleted server (for example a server deleted * without model events), because they block deleting the GitHub App. diff --git a/database/migrations/2026_10_28_224219_add_index_for_active_github_runner_executions.php b/database/migrations/2026_10_28_224219_add_index_for_active_github_runner_executions.php new file mode 100644 index 0000000000..425849e2d7 --- /dev/null +++ b/database/migrations/2026_10_28_224219_add_index_for_active_github_runner_executions.php @@ -0,0 +1,41 @@ +index(['status', 'server_id'], 'idx_github_runner_executions_status_server_id'); + }); + } + } + + public function down(): void + { + if (DB::getDriverName() === 'pgsql') { + DB::statement('DROP INDEX CONCURRENTLY IF EXISTS idx_github_runner_executions_status_server_id'); + + return; + } + + if (Schema::hasIndex('github_runner_executions', 'idx_github_runner_executions_status_server_id')) { + Schema::table('github_runner_executions', function (Blueprint $table) { + $table->dropIndex('idx_github_runner_executions_status_server_id'); + }); + } + } +}; diff --git a/tests/Feature/ReconcileGithubRunnersDisabledConfigTest.php b/tests/Feature/ReconcileGithubRunnersDisabledConfigTest.php index 48cc9a95ce..8bdcda74ef 100644 --- a/tests/Feature/ReconcileGithubRunnersDisabledConfigTest.php +++ b/tests/Feature/ReconcileGithubRunnersDisabledConfigTest.php @@ -114,3 +114,70 @@ it('keeps reconciling a disabled server until its running jobs finish', function Process::assertRan(reconcileDisabledTestCommandFor($server)); expect($running->fresh()->status)->toBe(GithubRunnerStatus::Failed); }); + +function reconcileDisabledTestExecution(GithubApp $githubApp, Server $server, GithubRunnerStatus $status): GithubRunnerExecution +{ + static $jobId = 4000; + + return GithubRunnerExecution::create([ + 'github_app_id' => $githubApp->id, + 'server_id' => $server->id, + 'trigger_workflow_job_id' => ++$jobId, + 'labels' => ['self-hosted', 'coolify'], + 'status' => $status, + ]); +} + +function reconcileDisabledTestSelectedIds(): array +{ + return (new ReconcileGithubRunnersJob)->serversToReconcile()->pluck('id')->all(); +} + +it('selects servers with enabled runners or occupying executions', function () { + $enabled = reconcileDisabledTestServer($this->team, $this->githubApp, enabled: true); + $disabled = reconcileDisabledTestServer($this->team, $this->githubApp, enabled: false); + $disabledWithFinished = reconcileDisabledTestServer($this->team, $this->githubApp, enabled: false); + $executionOnly = Server::factory()->create(['team_id' => $this->team->id]); + $finishedOnly = Server::factory()->create(['team_id' => $this->team->id]); + + foreach ([GithubRunnerStatus::Queued, GithubRunnerStatus::Completed, GithubRunnerStatus::Failed, GithubRunnerStatus::Cancelled, GithubRunnerStatus::TimedOut] as $status) { + reconcileDisabledTestExecution($this->githubApp, $disabledWithFinished, $status); + reconcileDisabledTestExecution($this->githubApp, $finishedOnly, $status); + } + reconcileDisabledTestExecution($this->githubApp, $executionOnly, GithubRunnerStatus::Provisioning); + + expect(reconcileDisabledTestSelectedIds())->toEqualCanonicalizing([$enabled->id, $executionOnly->id]) + ->not->toContain($disabled->id); +}); + +it('selects each server once when several conditions match', function () { + $server = reconcileDisabledTestServer($this->team, $this->githubApp, enabled: true); + foreach (GithubRunnerStatus::occupying() as $status) { + reconcileDisabledTestExecution($this->githubApp, $server, $status); + } + + expect(reconcileDisabledTestSelectedIds())->toBe([$server->id]); +}); + +it('does not select soft-deleted servers', function () { + $server = reconcileDisabledTestServer($this->team, $this->githubApp, enabled: true); + reconcileDisabledTestExecution($this->githubApp, $server, GithubRunnerStatus::Running); + Server::query()->whereKey($server->id)->update(['deleted_at' => now()]); + + expect(reconcileDisabledTestSelectedIds())->toBe([]); +}); + +it('follows configuration and execution status changes', function () { + $server = reconcileDisabledTestServer($this->team, $this->githubApp, enabled: false); + expect(reconcileDisabledTestSelectedIds())->toBe([]); + + $server->githubRunnerConfig->update(['is_enabled' => true]); + expect(reconcileDisabledTestSelectedIds())->toBe([$server->id]); + + $server->githubRunnerConfig->update(['is_enabled' => false]); + $execution = reconcileDisabledTestExecution($this->githubApp, $server, GithubRunnerStatus::Idle); + expect(reconcileDisabledTestSelectedIds())->toBe([$server->id]); + + $execution->update(['status' => GithubRunnerStatus::Completed]); + expect(reconcileDisabledTestSelectedIds())->toBe([]); +});