mirror of
https://github.com/coollabsio/coolify.git
synced 2026-11-06 13:10:37 -04:00
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
7324b5ca7b
commit
55605f6516
@@ -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<Server>
|
||||
*/
|
||||
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.
|
||||
|
||||
+41
@@ -0,0 +1,41 @@
|
||||
<?php
|
||||
|
||||
use Illuminate\Database\Migrations\Migration;
|
||||
use Illuminate\Database\Schema\Blueprint;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
use Illuminate\Support\Facades\Schema;
|
||||
|
||||
return new class extends Migration
|
||||
{
|
||||
public $withinTransaction = false;
|
||||
|
||||
public function up(): void
|
||||
{
|
||||
if (DB::getDriverName() === 'pgsql') {
|
||||
DB::statement('CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_github_runner_executions_status_server_id ON 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->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');
|
||||
});
|
||||
}
|
||||
}
|
||||
};
|
||||
@@ -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([]);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user