From dd7ad91ba28f7ab42103c536c00a8eef2a5e6b9b Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 28 Sep 2026 11:50:32 +0200 Subject: [PATCH] fix(cleanup): delete old rows in batches and report the real count cleanup:database used orderBy()->skip(10)->delete(). Laravel drops the offset in a DELETE, so it deleted all old rows anyway, and the count query with the offset printed 0 on PostgreSQL. Count without the offset, delete in batches of 1000 rows so a large table is not locked by one long DELETE, and write the output through the command so it is captured. Co-Authored-By: Claude Opus 5.5 --- app/Console/Commands/CleanupDatabase.php | 65 ++++++++------------ tests/Feature/CleanupDatabaseCommandTest.php | 36 +++++++++++ 2 files changed, 63 insertions(+), 38 deletions(-) create mode 100644 tests/Feature/CleanupDatabaseCommandTest.php diff --git a/app/Console/Commands/CleanupDatabase.php b/app/Console/Commands/CleanupDatabase.php index 65f686ba61..eac3a33ebf 100644 --- a/app/Console/Commands/CleanupDatabase.php +++ b/app/Console/Commands/CleanupDatabase.php @@ -12,12 +12,14 @@ class CleanupDatabase extends Command protected $description = 'Cleanup database'; + private const DELETE_BATCH_SIZE = 1000; + public function handle() { if ($this->option('yes')) { - echo "Running database cleanup...\n"; + $this->line('Running database cleanup...'); } else { - echo "Running database cleanup in dry-run mode...\n"; + $this->line('Running database cleanup in dry-run mode...'); } if (isCloud()) { // Later on we can increase this to 180 days or dynamically set @@ -25,51 +27,38 @@ class CleanupDatabase extends Command } else { $keep_days = $this->option('keep-days') ?? 60; } - echo "Keep days: $keep_days\n"; - // Cleanup failed jobs table - $failed_jobs = DB::table('failed_jobs')->where('failed_at', '<', now()->subDays(1)); - $count = $failed_jobs->count(); - echo "Delete $count entries from failed_jobs.\n"; - if ($this->option('yes')) { - $failed_jobs->delete(); - } + $this->line("Keep days: $keep_days"); + $this->cleanupTable('failed_jobs', 'failed_at', now()->subDays(1)); - // Cleanup sessions table - $sessions = DB::table('sessions')->where('last_activity', '<', now()->subDays($keep_days)->timestamp); - $count = $sessions->count(); - echo "Delete $count entries from sessions.\n"; - if ($this->option('yes')) { - $sessions->delete(); - } + $this->cleanupTable('sessions', 'last_activity', now()->subDays($keep_days)->timestamp); - // Cleanup activity_log table - $activity_log = DB::table('activity_log')->where('created_at', '<', now()->subDays($keep_days))->orderBy('created_at', 'desc')->skip(10); - $count = $activity_log->count(); - echo "Delete $count entries from activity_log.\n"; - if ($this->option('yes')) { - $activity_log->delete(); - } + $this->cleanupTable('activity_log', 'created_at', now()->subDays($keep_days)); $count = DB::table('audit_events')->where('created_at', '<', now()->subDays(90))->count(); - echo "Delete $count entries from audit_events.\n"; + $this->line("Delete $count entries from audit_events."); if ($this->option('yes')) { AuditEvent::pruneExpired(); } - // Cleanup application_deployment_queues table - $application_deployment_queues = DB::table('application_deployment_queues')->where('created_at', '<', now()->subDays($keep_days))->orderBy('created_at', 'desc')->skip(10); - $count = $application_deployment_queues->count(); - echo "Delete $count entries from application_deployment_queues.\n"; - if ($this->option('yes')) { - $application_deployment_queues->delete(); + $this->cleanupTable('application_deployment_queues', 'created_at', now()->subDays($keep_days)); + + $this->cleanupTable('scheduled_task_executions', 'created_at', now()->subDays($keep_days)); + } + + /** + * Delete the rows older than the given value in batches, so a large table is not locked + * by one long DELETE. + */ + private function cleanupTable(string $table, string $column, mixed $olderThan): void + { + $count = DB::table($table)->where($column, '<', $olderThan)->count(); + $this->line("Delete $count entries from $table."); + if (! $this->option('yes')) { + return; } - // Cleanup scheduled_task_executions table - $scheduled_task_executions = DB::table('scheduled_task_executions')->where('created_at', '<', now()->subDays($keep_days))->orderBy('created_at', 'desc'); - $count = $scheduled_task_executions->count(); - echo "Delete $count entries from scheduled_task_executions.\n"; - if ($this->option('yes')) { - $scheduled_task_executions->delete(); - } + do { + $deleted = DB::table($table)->where($column, '<', $olderThan)->limit(self::DELETE_BATCH_SIZE)->delete(); + } while ($deleted > 0); } } diff --git a/tests/Feature/CleanupDatabaseCommandTest.php b/tests/Feature/CleanupDatabaseCommandTest.php new file mode 100644 index 0000000000..673e6ac9d7 --- /dev/null +++ b/tests/Feature/CleanupDatabaseCommandTest.php @@ -0,0 +1,36 @@ +withProperties(['type' => 'inline', 'status' => 'finished'])->event('inline')->log('[]'); + Activity::query()->whereKey($activity->id)->toBase()->update(['created_at' => now()->subDays($ageInDays)]); + + return $activity; +} + +beforeEach(function () { + $this->old = collect(range(1, 12))->map(fn () => cleanupTestActivity(61)); + $this->recent = collect(range(1, 2))->map(fn () => cleanupTestActivity(5)); +}); + +it('reports the real number of old activities in a dry run and deletes nothing', function () { + $this->artisan('cleanup:database') + ->expectsOutputToContain('Delete 12 entries from activity_log.') + ->assertSuccessful(); + + expect(Activity::query()->count())->toBe(14); +}); + +it('deletes all activities older than the retention and keeps newer ones', function () { + $this->artisan('cleanup:database --yes') + ->expectsOutputToContain('Delete 12 entries from activity_log.') + ->assertSuccessful(); + + expect(Activity::query()->pluck('id')->sort()->values()->all()) + ->toBe($this->recent->pluck('id')->sort()->values()->all()); +});