diff --git a/app/Actions/Database/RestartDatabase.php b/app/Actions/Database/RestartDatabase.php index 6efaf8812f..e1c531182e 100644 --- a/app/Actions/Database/RestartDatabase.php +++ b/app/Actions/Database/RestartDatabase.php @@ -26,9 +26,14 @@ class RestartDatabase */ public function handle(StandaloneRedis|StandalonePostgresql|StandaloneMongodb|StandaloneMysql|StandaloneMariadb|StandaloneKeydb|StandaloneDragonfly|StandaloneClickhouse|StandaloneSqlite $database, ?string $reservation = null) { - $reservation ??= DatabaseOperationReservation::acquire($database->uuid); if ($reservation === null) { - return ResourceStartActivity::DATABASE_OPERATION_IN_PROGRESS_MESSAGE; + $reservation = DatabaseOperationReservation::acquire($database->uuid); + if ($reservation === null) { + return ResourceStartActivity::DATABASE_OPERATION_IN_PROGRESS_MESSAGE; + } + } elseif (! DatabaseOperationReservation::renew($database->uuid, $reservation)) { + // Do not stop the database for a request that expired in the queue. + return StartDatabase::operationInProgressError($database) ?? DatabaseOperationReservation::EXPIRED_MESSAGE; } try { @@ -45,10 +50,17 @@ class RestartDatabase return $prerequisiteError; } StopDatabase::run($database, dockerCleanup: false); + // The stop can take a while; keep the reservation for the start that follows. + DatabaseOperationReservation::renew($database->uuid, $reservation); return StartDatabase::run($database, $reservation); } finally { DatabaseOperationReservation::release($database->uuid, $reservation); } } + + public function asJob(StandaloneRedis|StandalonePostgresql|StandaloneMongodb|StandaloneMysql|StandaloneMariadb|StandaloneKeydb|StandaloneDragonfly|StandaloneClickhouse|StandaloneSqlite $database, ?string $reservation = null): mixed + { + return StartDatabase::runQueued($database, 'Database restart failed.', fn (): mixed => $this->handle($database, $reservation)); + } } diff --git a/app/Actions/Database/StartDatabase.php b/app/Actions/Database/StartDatabase.php index 169d996f9a..75d2bca5ce 100644 --- a/app/Actions/Database/StartDatabase.php +++ b/app/Actions/Database/StartDatabase.php @@ -4,6 +4,7 @@ namespace App\Actions\Database; use App\Enums\ActivityTypes; use App\Enums\ProcessStatus; +use App\Events\DatabaseStatusChanged; use App\Exceptions\DatabaseStartException; use App\Jobs\DatabaseStartJob; use App\Models\StandaloneClickhouse; @@ -37,9 +38,13 @@ class StartDatabase */ public function handle(StandaloneRedis|StandalonePostgresql|StandaloneMongodb|StandaloneMysql|StandaloneMariadb|StandaloneKeydb|StandaloneDragonfly|StandaloneClickhouse|StandaloneSqlite $database, ?string $reservation = null): Activity|string { - $reservation ??= DatabaseOperationReservation::acquire($database->uuid); if ($reservation === null) { - return ResourceStartActivity::DATABASE_OPERATION_IN_PROGRESS_MESSAGE; + $reservation = DatabaseOperationReservation::acquire($database->uuid); + if ($reservation === null) { + return ResourceStartActivity::DATABASE_OPERATION_IN_PROGRESS_MESSAGE; + } + } elseif (! DatabaseOperationReservation::renew($database->uuid, $reservation)) { + return self::expiredReservationError($database); } try { @@ -49,6 +54,11 @@ class StartDatabase } } + public function asJob(StandaloneRedis|StandalonePostgresql|StandaloneMongodb|StandaloneMysql|StandaloneMariadb|StandaloneKeydb|StandaloneDragonfly|StandaloneClickhouse|StandaloneSqlite $database, ?string $reservation = null): Activity|string + { + return self::runQueued($database, 'Database start failed.', fn (): Activity|string => $this->handle($database, $reservation)); + } + private function queueStart(StandaloneRedis|StandalonePostgresql|StandaloneMongodb|StandaloneMysql|StandaloneMariadb|StandaloneKeydb|StandaloneDragonfly|StandaloneClickhouse|StandaloneSqlite $database, string $reservation): Activity|string { $server = $database->destination->server; @@ -69,18 +79,7 @@ class StartDatabase 'last_restart_type' => null, ]); - $activity = activity() - ->withProperties([ - 'server_uuid' => $server->uuid, - 'type' => ActivityTypes::INLINE->value, - 'type_uuid' => $database->uuid, - 'status' => ProcessStatus::QUEUED->value, - 'team_id' => $server->team_id, - 'operation' => ResourceStartActivity::DATABASE_START_OPERATION, - ]) - ->performedOn($database) - ->event(ActivityTypes::INLINE->value) - ->log('[]'); + $activity = self::logQueuedStartActivity($database); if ($activity === null) { return 'Database start could not be queued because activity logging is disabled.'; @@ -107,6 +106,91 @@ class StartDatabase return $activity; } + private static function logQueuedStartActivity(StandaloneRedis|StandalonePostgresql|StandaloneMongodb|StandaloneMysql|StandaloneMariadb|StandaloneKeydb|StandaloneDragonfly|StandaloneClickhouse|StandaloneSqlite $database): ?Activity + { + $server = $database->destination->server; + + return activity() + ->withProperties([ + 'server_uuid' => $server->uuid, + 'type' => ActivityTypes::INLINE->value, + 'type_uuid' => $database->uuid, + 'status' => ProcessStatus::QUEUED->value, + 'team_id' => $server->team_id, + 'operation' => ResourceStartActivity::DATABASE_START_OPERATION, + ]) + ->performedOn($database) + ->event(ActivityTypes::INLINE->value) + ->log('[]'); + } + + /** + * A queued action whose reservation expired or was taken over must not run: a newer start + * may have run in the meantime. If another operation is in progress, that one reports its + * own result; otherwise the caller must record the expired request as failed. + */ + private static function expiredReservationError(StandaloneRedis|StandalonePostgresql|StandaloneMongodb|StandaloneMysql|StandaloneMariadb|StandaloneKeydb|StandaloneDragonfly|StandaloneClickhouse|StandaloneSqlite $database): string + { + return self::operationInProgressError($database) ?? DatabaseOperationReservation::EXPIRED_MESSAGE; + } + + /** + * Run a queued start or restart. The request that queued it has already answered, so an + * error would be lost. Record it as a failed start activity, the same as a failed start + * job, so that the UI shows the error. An error that already has its own start activity + * (created by this run) or that reports another operation in progress is not recorded again. + * + * @param callable(): (Activity|string|null) $operation + */ + public static function runQueued(StandaloneRedis|StandalonePostgresql|StandaloneMongodb|StandaloneMysql|StandaloneMariadb|StandaloneKeydb|StandaloneDragonfly|StandaloneClickhouse|StandaloneSqlite $database, string $failureMessage, callable $operation): mixed + { + $lastStartActivityId = self::latestStartActivityId($database); + + try { + $result = $operation(); + } catch (Throwable $e) { + if (self::latestStartActivityId($database) === $lastStartActivityId) { + self::recordFailedStart($database, $e instanceof DatabaseStartException ? $e->getMessage() : $failureMessage); + } + + throw $e; + } + + if (is_string($result) + && $result !== ResourceStartActivity::DATABASE_OPERATION_IN_PROGRESS_MESSAGE + && self::latestStartActivityId($database) === $lastStartActivityId) { + self::recordFailedStart($database, $result); + } + + return $result; + } + + public static function recordFailedStart(StandaloneRedis|StandalonePostgresql|StandaloneMongodb|StandaloneMysql|StandaloneMariadb|StandaloneKeydb|StandaloneDragonfly|StandaloneClickhouse|StandaloneSqlite $database, string $message): void + { + try { + $activity = self::logQueuedStartActivity($database); + if ($activity !== null) { + ResourceStartActivity::markFailed($activity, $message); + } + } finally { + event(new DatabaseStatusChanged); + } + } + + private static function latestStartActivityId(StandaloneRedis|StandalonePostgresql|StandaloneMongodb|StandaloneMysql|StandaloneMariadb|StandaloneKeydb|StandaloneDragonfly|StandaloneClickhouse|StandaloneSqlite $database): ?int + { + if (blank($database->uuid)) { + return null; + } + + $id = Activity::query() + ->where('properties->type_uuid', $database->uuid) + ->where('properties->operation', ResourceStartActivity::DATABASE_START_OPERATION) + ->max('id'); + + return $id === null ? null : (int) $id; + } + /** * Reserve a start or restart before it is queued, so that a parallel request cannot also * queue one before the queued action has created its activity. Pass the token to the diff --git a/app/Support/DatabaseOperationReservation.php b/app/Support/DatabaseOperationReservation.php index 936f5ab56f..61e33926fd 100644 --- a/app/Support/DatabaseOperationReservation.php +++ b/app/Support/DatabaseOperationReservation.php @@ -20,6 +20,8 @@ class DatabaseOperationReservation { public const TTL_SECONDS = ResourceStartActivity::QUEUED_STALE_AFTER_SECONDS; + public const EXPIRED_MESSAGE = 'The queued start or restart waited too long in the queue and was not run. Start the database again.'; + public static function key(string $databaseUuid): string { return "database-operation-pending:{$databaseUuid}"; @@ -55,6 +57,30 @@ class DatabaseOperationReservation return $holder !== null && $holder !== $token; } + /** + * Confirm that the token still holds the reservation and extend it, so it does not expire + * while the operation runs. False when the reservation expired (the queued action waited + * longer than the TTL) or a newer request took it over: the operation must not run then. + */ + public static function renew(?string $databaseUuid, ?string $token): bool + { + if ($token === null) { + return false; + } + + if (blank($databaseUuid)) { + return true; + } + + if (Cache::get(self::key($databaseUuid)) !== $token) { + return false; + } + + Cache::put(self::key($databaseUuid), $token, self::TTL_SECONDS); + + return true; + } + /** * Release the reservation only if the token still holds it, so that an expired * reservation does not remove the reservation of a newer request. diff --git a/tests/Feature/DatabaseStartConcurrencyTest.php b/tests/Feature/DatabaseStartConcurrencyTest.php index 8f51caccd5..0ecd6f2076 100644 --- a/tests/Feature/DatabaseStartConcurrencyTest.php +++ b/tests/Feature/DatabaseStartConcurrencyTest.php @@ -35,6 +35,7 @@ uses(RefreshDatabase::class); beforeEach(function () { InstanceSettings::forceCreate(['id' => 0]); config()->set('cache.default', 'array'); + Server::flushIdentityMap(); $this->team = Team::factory()->create(); $this->user = User::factory()->create(); @@ -378,6 +379,139 @@ describe('pending operation reservation', function () { ->and(databaseStartActivityCount($this->database->uuid))->toBe(0); }); + it('fails an expired queued start instead of running it late', function () { + Queue::fake(); + Event::fake([DatabaseStatusChanged::class]); + + $this->withHeaders(databaseStartApiHeaders($this->bearerToken)) + ->postJson("/api/v1/databases/{$this->database->uuid}/start") + ->assertOk(); + + $this->travel(DatabaseOperationReservation::TTL_SECONDS + 1)->seconds(); + + expect(runPushedDatabaseAction(StartDatabase::class))->toBe(DatabaseOperationReservation::EXPIRED_MESSAGE) + ->and(databaseOperationReserved($this->database->uuid))->toBeFalse(); + Queue::assertNotPushed(DatabaseStartJob::class); + Event::assertDispatched(DatabaseStatusChanged::class); + + $failed = Activity::query()->where('properties->type_uuid', $this->database->uuid)->sole(); + expect(data_get($failed, 'properties.status'))->toBe(ProcessStatus::ERROR->value) + ->and(data_get($failed, 'properties.operation'))->toBe(ResourceStartActivity::DATABASE_START_OPERATION) + ->and(data_get($failed, 'properties.error'))->toBe(DatabaseOperationReservation::EXPIRED_MESSAGE) + ->and(ResourceStartActivity::latestRunning($this->database->uuid))->toBeNull(); + + $this->withHeaders(databaseStartApiHeaders($this->bearerToken)) + ->postJson("/api/v1/databases/{$this->database->uuid}/start") + ->assertOk(); + }); + + it('does not start the database a second time when an expired queued start runs after a newer start finished', function () { + Queue::fake(); + + $this->withHeaders(databaseStartApiHeaders($this->bearerToken)) + ->postJson("/api/v1/databases/{$this->database->uuid}/start") + ->assertOk(); + + $this->travel(DatabaseOperationReservation::TTL_SECONDS + 1)->seconds(); + $uiActivity = StartDatabase::run($this->database); + expect($uiActivity)->toBeInstanceOf(Activity::class); + $uiActivity->properties = $uiActivity->properties->merge(['status' => ProcessStatus::FINISHED->value]); + $uiActivity->save(); + + expect(runPushedDatabaseAction(StartDatabase::class))->toBe(DatabaseOperationReservation::EXPIRED_MESSAGE); + Queue::assertPushed(DatabaseStartJob::class, 1); + }); + + it('fails an expired queued restart without stopping the database', function () { + Queue::fake(); + StopDatabase::shouldRun()->never(); + + $this->withHeaders(databaseStartApiHeaders($this->bearerToken)) + ->postJson("/api/v1/databases/{$this->database->uuid}/restart") + ->assertOk(); + + $this->travel(DatabaseOperationReservation::TTL_SECONDS + 1)->seconds(); + + expect(runPushedDatabaseAction(RestartDatabase::class))->toBe(DatabaseOperationReservation::EXPIRED_MESSAGE); + Queue::assertNotPushed(DatabaseStartJob::class); + $failed = Activity::query()->where('properties->type_uuid', $this->database->uuid)->sole(); + expect(data_get($failed, 'properties.error'))->toBe(DatabaseOperationReservation::EXPIRED_MESSAGE); + }); + + it('keeps the reservation of a queued restart while the database stops', function () { + Queue::fake(); + + $this->withHeaders(databaseStartApiHeaders($this->bearerToken)) + ->postJson("/api/v1/databases/{$this->database->uuid}/restart") + ->assertOk(); + + $this->travel(DatabaseOperationReservation::TTL_SECONDS - 10)->seconds(); + StopDatabase::shouldRun()->once()->andReturnUsing(function () { + $this->travel(20)->seconds(); + + return 'Database stopped successfully'; + }); + + expect(runPushedDatabaseAction(RestartDatabase::class))->toBeInstanceOf(Activity::class); + Queue::assertPushed(DatabaseStartJob::class, 1); + }); + + it('records a failed start when the queued action cannot start the database', function () { + Queue::fake(); + + $this->withHeaders(databaseStartApiHeaders($this->bearerToken)) + ->postJson("/api/v1/databases/{$this->database->uuid}/start") + ->assertOk(); + + $job = Queue::pushed(JobDecorator::class, fn (JobDecorator $job) => $job->getAction() instanceof StartDatabase)->last(); + $job->getParameters()[0]->destination->server->settings->is_reachable = false; + + expect($job->handle())->toBe('Server is not functional') + ->and(databaseOperationReserved($this->database->uuid))->toBeFalse(); + $failed = Activity::query()->where('properties->type_uuid', $this->database->uuid)->sole(); + expect(data_get($failed, 'properties.status'))->toBe(ProcessStatus::ERROR->value) + ->and(data_get($failed, 'properties.error'))->toBe('Server is not functional'); + }); + + it('records a failed start when the queued restart action throws', function () { + Queue::fake(); + StopDatabase::shouldRun()->once()->andThrow(new RuntimeException('ssh: connection refused')); + + $this->withHeaders(databaseStartApiHeaders($this->bearerToken)) + ->postJson("/api/v1/databases/{$this->database->uuid}/restart") + ->assertOk(); + + expect(fn () => runPushedDatabaseAction(RestartDatabase::class))->toThrow(RuntimeException::class); + + expect(databaseOperationReserved($this->database->uuid))->toBeFalse(); + $failed = Activity::query()->where('properties->type_uuid', $this->database->uuid)->sole(); + expect(data_get($failed, 'properties.status'))->toBe(ProcessStatus::ERROR->value) + ->and(data_get($failed, 'properties.error'))->toBe('Database restart failed.'); + + $this->withHeaders(databaseStartApiHeaders($this->bearerToken)) + ->postJson("/api/v1/databases/{$this->database->uuid}/start") + ->assertOk(); + }); + + it('does not record a second failed start when the queued start already failed its own activity', function () { + Queue::fake(); + + $this->withHeaders(databaseStartApiHeaders($this->bearerToken)) + ->postJson("/api/v1/databases/{$this->database->uuid}/start") + ->assertOk(); + + $job = Queue::pushed(JobDecorator::class, fn (JobDecorator $job) => $job->getAction() instanceof StartDatabase)->last(); + $queue = Queue::partialMock(); + $queue->shouldReceive('connection')->andReturnSelf(); + $queue->shouldReceive('push', 'pushOn')->andThrow(new RuntimeException('redis down')); + + expect(fn () => $job->handle())->toThrow(RuntimeException::class, 'redis down'); + + $activities = Activity::query()->where('properties->type_uuid', $this->database->uuid)->get(); + expect($activities)->toHaveCount(1) + ->and(data_get($activities->first(), 'properties.error'))->toBe('Database start could not be queued.'); + }); + it('does not start from the UI while an API start is still in the queue', function () { Queue::fake();