diff --git a/config/config.sample.php b/config/config.sample.php index 953a47fd19850..c3ab614b4af00 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -3011,4 +3011,12 @@ * Defaults to ``0``. */ 'preview_expiration_days' => 0, + + /** + * Delete job runs older than a certain number of days. + * Less than one day is not allowed. + * + * Defaults to ``60``. + */ + 'background_jobs_expiration_days' => 60, ]; diff --git a/core/BackgroundJobs/CleanupBackgroundJobsJob.php b/core/BackgroundJobs/CleanupBackgroundJobsJob.php new file mode 100644 index 0000000000000..5dc9a372df892 --- /dev/null +++ b/core/BackgroundJobs/CleanupBackgroundJobsJob.php @@ -0,0 +1,85 @@ +setInterval(60 * 60); + $this->setTimeSensitivity(IJob::TIME_SENSITIVE); + } + + #[Override] + protected function run($argument): void { + $this->reapCrashedJobs(); + $this->cleanOldestRuns(); + } + + private function reapCrashedJobs(): void { + $currentServerId = Util::getServerId(); + + foreach ($this->jobRuns->runningJobs(1000) as $job) { + if ($job->serverId !== $currentServerId) { + continue; + } + $output = []; + $result = 0; + exec('ps -p ' . escapeshellarg((string)$job->pid) . ' -o cmd', $output, $result); + if (count($output) === 1 && current($output) === 'CMD' && $result === 1) { + // Process doesn't exists anymore + $maxDuration = (new DateTimeImmutable())->diff($job->startedAt); + $maxDuration + = ($maxDuration->days * 24 * 60 * 60 * 1000) + + ($maxDuration->h * 60 * 60 * 1000) + + ($maxDuration->i * 60 * 1000) + + ($maxDuration->s * 1000) + + (int)($maxDuration->f * 1000); + $this->jobRuns->finished($job->runId, $maxDuration, 0, JobStatus::CRASHED); + $this->logger->warning('No process matching PID {pid} found on server {serverId}. Job {runId} was marked as crashed', [ + 'pid' => $job->pid, + 'serverId' => $job->serverId, + 'runId' => $job->runId, + ]); + } + } + } + + private function cleanOldestRuns(): void { + $daysToKeep = $this->config->getSystemValueInt('background_jobs_expiration_days', 60); + if ($daysToKeep < 1) { + throw new RuntimeException('Invalid number of days'); + } + $cleanBeforeTimestamp = time() - ($daysToKeep * 24 * 3600); + + $cleanedJobs = $this->jobRuns->deleteBefore($cleanBeforeTimestamp); + if ($cleanedJobs > 0) { + $this->logger->info( + 'Cleanup of old background jobs. Number of jobs removed: ' . $cleanedJobs . 'Reason: older than ' . $daysToKeep . ' days.', + ); + } + } +} diff --git a/core/Command/Background/JobsHistory.php b/core/Command/Background/JobsHistory.php index 8837bcc10cee9..81ec71fb52c40 100644 --- a/core/Command/Background/JobsHistory.php +++ b/core/Command/Background/JobsHistory.php @@ -75,7 +75,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int private function formatLine(iterable $jobs): \Generator { $jobsInfo = []; $now = time(); - $currentServerId = $this->config->getSystemValueInt('serverid', -1); + $currentServerId = Util::getServerId(); foreach ($jobs as $job) { $status = match ($job->status) { JobStatus::RUNNING => 'Running', diff --git a/core/Command/Background/RunningJobs.php b/core/Command/Background/RunningJobs.php index 8b63ace09c5cd..ae2dc0b35c332 100644 --- a/core/Command/Background/RunningJobs.php +++ b/core/Command/Background/RunningJobs.php @@ -12,6 +12,7 @@ use OC\BackgroundJob\JobRuns; use OC\Core\Command\Base; use OCP\IConfig; +use OCP\Util; use Override; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; @@ -60,7 +61,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int private function formatLine(iterable $jobs): \Generator { $now = time(); - $currentServerId = $this->config->getSystemValueInt('serverid', -1); + $currentServerId = Util::getServerId(); foreach ($jobs as $job) { yield [ 'Run ID' => $job->runId, diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index fe73bc0443d9a..46cbe0f884fe5 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1310,6 +1310,7 @@ 'OC\\Core\\AppInfo\\ConfigLexicon' => $baseDir . '/core/AppInfo/ConfigLexicon.php', 'OC\\Core\\BackgroundJobs\\BackgroundCleanupUpdaterBackupsJob' => $baseDir . '/core/BackgroundJobs/BackgroundCleanupUpdaterBackupsJob.php', 'OC\\Core\\BackgroundJobs\\CheckForUserCertificates' => $baseDir . '/core/BackgroundJobs/CheckForUserCertificates.php', + 'OC\\Core\\BackgroundJobs\\CleanupBackgroundJobsJob' => $baseDir . '/core/BackgroundJobs/CleanupBackgroundJobsJob.php', 'OC\\Core\\BackgroundJobs\\CleanupLoginFlowV2' => $baseDir . '/core/BackgroundJobs/CleanupLoginFlowV2.php', 'OC\\Core\\BackgroundJobs\\ExpirePreviewsJob' => $baseDir . '/core/BackgroundJobs/ExpirePreviewsJob.php', 'OC\\Core\\BackgroundJobs\\GenerateMetadataJob' => $baseDir . '/core/BackgroundJobs/GenerateMetadataJob.php', @@ -2045,6 +2046,7 @@ 'OC\\Repair' => $baseDir . '/lib/private/Repair.php', 'OC\\RepairException' => $baseDir . '/lib/private/RepairException.php', 'OC\\Repair\\AddBruteForceCleanupJob' => $baseDir . '/lib/private/Repair/AddBruteForceCleanupJob.php', + 'OC\\Repair\\AddCleanupBackgroundJobsJob' => $baseDir . '/lib/private/Repair/AddCleanupBackgroundJobsJob.php', 'OC\\Repair\\AddCleanupDeletedUsersBackgroundJob' => $baseDir . '/lib/private/Repair/AddCleanupDeletedUsersBackgroundJob.php', 'OC\\Repair\\AddCleanupUpdaterBackupsJob' => $baseDir . '/lib/private/Repair/AddCleanupUpdaterBackupsJob.php', 'OC\\Repair\\AddMetadataGenerationJob' => $baseDir . '/lib/private/Repair/AddMetadataGenerationJob.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index f4d201e709a0c..15a95921bdfae 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1351,6 +1351,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Core\\AppInfo\\ConfigLexicon' => __DIR__ . '/../../..' . '/core/AppInfo/ConfigLexicon.php', 'OC\\Core\\BackgroundJobs\\BackgroundCleanupUpdaterBackupsJob' => __DIR__ . '/../../..' . '/core/BackgroundJobs/BackgroundCleanupUpdaterBackupsJob.php', 'OC\\Core\\BackgroundJobs\\CheckForUserCertificates' => __DIR__ . '/../../..' . '/core/BackgroundJobs/CheckForUserCertificates.php', + 'OC\\Core\\BackgroundJobs\\CleanupBackgroundJobsJob' => __DIR__ . '/../../..' . '/core/BackgroundJobs/CleanupBackgroundJobsJob.php', 'OC\\Core\\BackgroundJobs\\CleanupLoginFlowV2' => __DIR__ . '/../../..' . '/core/BackgroundJobs/CleanupLoginFlowV2.php', 'OC\\Core\\BackgroundJobs\\ExpirePreviewsJob' => __DIR__ . '/../../..' . '/core/BackgroundJobs/ExpirePreviewsJob.php', 'OC\\Core\\BackgroundJobs\\GenerateMetadataJob' => __DIR__ . '/../../..' . '/core/BackgroundJobs/GenerateMetadataJob.php', @@ -2086,6 +2087,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Repair' => __DIR__ . '/../../..' . '/lib/private/Repair.php', 'OC\\RepairException' => __DIR__ . '/../../..' . '/lib/private/RepairException.php', 'OC\\Repair\\AddBruteForceCleanupJob' => __DIR__ . '/../../..' . '/lib/private/Repair/AddBruteForceCleanupJob.php', + 'OC\\Repair\\AddCleanupBackgroundJobsJob' => __DIR__ . '/../../..' . '/lib/private/Repair/AddCleanupBackgroundJobsJob.php', 'OC\\Repair\\AddCleanupDeletedUsersBackgroundJob' => __DIR__ . '/../../..' . '/lib/private/Repair/AddCleanupDeletedUsersBackgroundJob.php', 'OC\\Repair\\AddCleanupUpdaterBackupsJob' => __DIR__ . '/../../..' . '/lib/private/Repair/AddCleanupUpdaterBackupsJob.php', 'OC\\Repair\\AddMetadataGenerationJob' => __DIR__ . '/../../..' . '/lib/private/Repair/AddMetadataGenerationJob.php', diff --git a/lib/private/BackgroundJob/JobRuns.php b/lib/private/BackgroundJob/JobRuns.php index 7a19bdf001a0e..a2612f6655eae 100644 --- a/lib/private/BackgroundJob/JobRuns.php +++ b/lib/private/BackgroundJob/JobRuns.php @@ -62,6 +62,18 @@ public function finished(int|string $runId, int $duration, int $memoryPeakUsage, return $result === 1; } + public function deleteBefore(int $timestamp): int { + $beforeSnowflake = $this->snowflakeGenerator->minForTimeId($timestamp); + $beforeSnowflake = '91480652934574081'; + $qb = $this->connection->getQueryBuilder(); + $result = $qb + ->delete(self::TABLE) + ->where($qb->expr()->lt('run_id', $qb->createNamedParameter($beforeSnowflake))) + ->executeStatement(); + + return $result; + } + #[Override] public function runningJobs(int $limit = 200): \Generator { $qb = $this->connection->getQueryBuilder(); diff --git a/lib/private/Repair.php b/lib/private/Repair.php index d25a12ccd9d76..b323997f507a0 100644 --- a/lib/private/Repair.php +++ b/lib/private/Repair.php @@ -9,6 +9,7 @@ namespace OC; use OC\Repair\AddBruteForceCleanupJob; +use OC\Repair\AddCleanupBackgroundJobsJob; use OC\Repair\AddCleanupDeletedUsersBackgroundJob; use OC\Repair\AddCleanupUpdaterBackupsJob; use OC\Repair\AddMetadataGenerationJob; @@ -134,14 +135,14 @@ public function addStep(IRepairStep|string $repairStep, bool $includeExpensive = } } - if (!($s instanceof IRepairStep)) { + if (!$s instanceof IRepairStep) { throw new \Exception("Repair step '$repairStep' is not of type \\OCP\\Migration\\IRepairStep"); } $repairStep = $s; } - if (($repairStep instanceof IRepairStepExpensive) && !$includeExpensive) { + if ($repairStep instanceof IRepairStepExpensive && !$includeExpensive) { $this->debug("Skipping expensive repair step '" . $repairStep::class . "'"); } else { $this->repairSteps[] = $repairStep; @@ -195,6 +196,7 @@ public static function getRepairSteps(bool $includeExpensive = false): array { Server::get(SanitizeAccountProperties::class), Server::get(AddMovePreviewJob::class), Server::get(ConfigKeyMigration::class), + Server::get(AddCleanupBackgroundJobsJob::class), ]; if ($includeExpensive) { diff --git a/lib/private/Repair/AddCleanupBackgroundJobsJob.php b/lib/private/Repair/AddCleanupBackgroundJobsJob.php new file mode 100644 index 0000000000000..41478db990085 --- /dev/null +++ b/lib/private/Repair/AddCleanupBackgroundJobsJob.php @@ -0,0 +1,33 @@ +jobList->add(CleanupBackgroundJobsJob::class); + } +} diff --git a/lib/private/Setup.php b/lib/private/Setup.php index ad1e59135d162..2c67df38acf32 100644 --- a/lib/private/Setup.php +++ b/lib/private/Setup.php @@ -16,6 +16,7 @@ use OC\AppFramework\Bootstrap\Coordinator; use OC\Authentication\Token\PublicKeyTokenProvider; use OC\Authentication\Token\TokenCleanupJob; +use OC\Core\BackgroundJobs\CleanupBackgroundJobsJob; use OC\Core\BackgroundJobs\ExpirePreviewsJob; use OC\Core\BackgroundJobs\GenerateMetadataJob; use OC\Core\BackgroundJobs\PreviewMigrationJob; @@ -532,6 +533,7 @@ public static function installBackgroundJobs(): void { $jobList->add(GenerateMetadataJob::class); $jobList->add(PreviewMigrationJob::class); $jobList->add(ExpirePreviewsJob::class); + $jobList->add(CleanupBackgroundJobsJob::class); } /** diff --git a/lib/private/Snowflake/SnowflakeGenerator.php b/lib/private/Snowflake/SnowflakeGenerator.php index fa07f39858381..92ce350967527 100644 --- a/lib/private/Snowflake/SnowflakeGenerator.php +++ b/lib/private/Snowflake/SnowflakeGenerator.php @@ -10,8 +10,8 @@ namespace OC\Snowflake; use OCP\AppFramework\Utility\ITimeFactory; -use OCP\IConfig; use OCP\Snowflake\ISnowflakeGenerator; +use OCP\Util; use Override; /** @@ -24,7 +24,6 @@ final readonly class SnowflakeGenerator implements ISnowflakeGenerator { public function __construct( private ITimeFactory $timeFactory, - private IConfig $config, private ISequence $sequenceGenerator, ) { } @@ -34,7 +33,7 @@ public function nextId(): string { // Relative time [$seconds, $milliseconds] = $this->getCurrentTime(); - $serverId = $this->getServerId(); // Already 9 bits + $serverId = Util::getServerId(); // Already 9 bits $isCli = (int)$this->isCli(); // 1 bit $sequenceId = $this->sequenceGenerator->nextId($seconds, $milliseconds, $serverId); // 12 bits if ($sequenceId > 0xFFF || $sequenceId === false) { @@ -43,6 +42,23 @@ public function nextId(): string { return $this->nextId(); } + return $this->packSnowflakeId($seconds, $milliseconds, $serverId, $isCli, $sequenceId); + } + + /** + * Return minimal snowflake ID for a given timestamp + * + * Not a real snowflake ID! + * Only use it for comparisons. For example get all snowflake IDs generated before $timestamp + * + * @since 34.0.1 + */ + #[Override] + public function minForTimeId(int $timestamp): string { + return $this->packSnowflakeId($timestamp - self::TS_OFFSET, 0, 0, 0, 0); + } + + private function packSnowflakeId($seconds, $milliseconds, $serverId, $isCli, $sequenceId): string { if (PHP_INT_SIZE === 8) { $firstHalf = $seconds & 0x7FFFFFFF; $secondHalf = (($milliseconds & 0x3FF) << 22) | ($serverId << 13) | ($isCli << 12) | $sequenceId; @@ -102,24 +118,6 @@ private function getCurrentTime(): array { ]; } - /** - * Return configured serverid or generate one if not set - * - * @return int<0,511> - */ - private function getServerId(): int { - $serverid = $this->config->getSystemValueInt('serverid', -1); - if ($serverid < 1) { - // Fallback: generates a server ID based on hostname - // or random bytes if hostname isn't available - /** @var int<0,max> */ - $serverid = hexdec(hash('xxh32', gethostname() ?: random_bytes(8))); - } - - /** @var int<0,511> */ - return $serverid & 0x1FF; - } - private function isCli(): bool { return PHP_SAPI === 'cli'; } diff --git a/lib/public/Snowflake/ISnowflakeGenerator.php b/lib/public/Snowflake/ISnowflakeGenerator.php index 276e5dc52fa22..f3a8a4cabd85f 100644 --- a/lib/public/Snowflake/ISnowflakeGenerator.php +++ b/lib/public/Snowflake/ISnowflakeGenerator.php @@ -42,4 +42,18 @@ interface ISnowflakeGenerator { * @since 33.0 */ public function nextId(): string; + + /** + * Return the smallest possible Snowflake ID for a given timestamp + * + * Not a real snowflake ID! + * Only use it for comparisons. Examples: + * - find all Snowflake IDs generated from a given $timestamp + * Look for `>= minForTimeId($timestamp)` + * - delete all Snowflake IDs generated before a given $timestamp + * Delete where `id < minForTimeId($timestamp)` + * + * @since 34.0.1 + */ + public function minForTimeId(int $timestamp): string; } diff --git a/lib/public/Util.php b/lib/public/Util.php index 9fc9b9e759713..16f0bf1c24481 100644 --- a/lib/public/Util.php +++ b/lib/public/Util.php @@ -246,9 +246,7 @@ public static function addHeader($tag, $attributes, $text = null) { */ public static function linkToAbsolute($app, $file, $args = []) { $urlGenerator = Server::get(IURLGenerator::class); - return $urlGenerator->getAbsoluteURL( - $urlGenerator->linkTo($app, $file, $args) - ); + return $urlGenerator->getAbsoluteURL($urlGenerator->linkTo($app, $file, $args)); } /** @@ -279,6 +277,25 @@ public static function getServerHostName() { return $host_name; } + /** + * Returns configured Server ID or use default fallback + * @since 34.0.1 + */ + public static function getServerId() { + $config = Server::get(IConfig::class); + + $serverid = $config->getSystemValueInt('serverid', -1); + if ($serverid < 1) { + // Fallback: generates a server ID based on hostname + // or random bytes if hostname isn't available + /** @var int<0,max> */ + $serverid = hexdec(hash('xxh32', gethostname() ?: random_bytes(8))); + } + + /** @var int<0,511> */ + return $serverid & 0x1FF; + } + /** * Returns the default email address * @param string $user_part the user part of the address @@ -483,7 +500,7 @@ public static function encodePath(string $component): string { * @since 4.5.0 */ public static function mb_array_change_key_case($input, $case = MB_CASE_LOWER, $encoding = 'UTF-8') { - $case = ($case !== MB_CASE_UPPER) ? MB_CASE_LOWER : MB_CASE_UPPER; + $case = $case !== MB_CASE_UPPER ? MB_CASE_LOWER : MB_CASE_UPPER; $ret = []; foreach ($input as $k => $v) { $ret[mb_convert_case($k, $case, $encoding)] = $v; @@ -518,7 +535,7 @@ public static function freeSpace(string $dir): int|float { $freeSpace = max($freeSpace, 0); return $freeSpace; } else { - return (INF > 0)? INF: PHP_INT_MAX; // work around https://bugs.php.net/bug.php?id=69188 + return INF > 0 ? INF : PHP_INT_MAX; // work around https://bugs.php.net/bug.php?id=69188 } } diff --git a/tests/lib/Snowflake/GeneratorTest.php b/tests/lib/Snowflake/GeneratorTest.php index 1f0153e0d9c7d..aba15235130d3 100644 --- a/tests/lib/Snowflake/GeneratorTest.php +++ b/tests/lib/Snowflake/GeneratorTest.php @@ -14,8 +14,8 @@ use OC\Snowflake\SnowflakeDecoder; use OC\Snowflake\SnowflakeGenerator; use OCP\AppFramework\Utility\ITimeFactory; -use OCP\IConfig; use OCP\Snowflake\ISnowflakeGenerator; +use OCP\Util; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; @@ -25,23 +25,19 @@ */ class GeneratorTest extends TestCase { private SnowflakeDecoder $decoder; - private IConfig&MockObject $config; private ISequence&MockObject $sequence; #[\Override] public function setUp(): void { $this->decoder = new SnowflakeDecoder(); - $this->config = $this->createMock(IConfig::class); - $this->config->method('getSystemValueInt')->with('serverid')->willReturn(42); - $this->sequence = $this->createMock(ISequence::class); $this->sequence->method('isAvailable')->willReturn(true); $this->sequence->method('nextId')->willReturn(421); } public function testGenerator(): void { - $generator = new SnowflakeGenerator(new TimeFactory(), $this->config, $this->sequence); + $generator = new SnowflakeGenerator(new TimeFactory(), $this->sequence); $snowflakeId = $generator->nextId(); $data = $this->decoder->decode($generator->nextId()); @@ -61,7 +57,26 @@ public function testGenerator(): void { $this->assertTrue($data->isCli()); // Check serverId - $this->assertEquals(42, $data->getServerId()); + $this->assertEquals(Util::getServerId(), $data->getServerId()); + } + + public function testMinForTime(): void { + $generator = new SnowflakeGenerator(new TimeFactory(), $this->sequence); + $now = time(); + $snowflakeId = $generator->minForTimeId($now); + $data = $this->decoder->decode($snowflakeId); + + $this->assertIsString($snowflakeId); + + // Check timestamp + $this->assertEquals($now - ISnowflakeGenerator::TS_OFFSET, $data->getSeconds()); + + // Check all other fields are at zero + $this->assertEquals(0, $data->getMilliseconds()); + $this->assertEquals(0, $data->getServerId()); + $this->assertEquals(0, $data->getSequenceId()); + $this->assertFalse($data->isCli()); + $this->assertEquals(0, $data->getServerId()); } #[DataProvider('provideSnowflakeData')] @@ -70,12 +85,12 @@ public function testGeneratorWithFixedTime(string $date, int $expectedSeconds, i $timeFactory = $this->createMock(ITimeFactory::class); $timeFactory->method('now')->willReturn($dt); - $generator = new SnowflakeGenerator($timeFactory, $this->config, $this->sequence); + $generator = new SnowflakeGenerator($timeFactory, $this->sequence); $data = $this->decoder->decode($generator->nextId()); $this->assertEquals($expectedSeconds, $data->getCreatedAt()->format('U') - ISnowflakeGenerator::TS_OFFSET); $this->assertEquals($expectedMilliseconds, (int)$data->getCreatedAt()->format('v')); - $this->assertEquals(42, $data->getServerId()); + $this->assertEquals(Util::getServerId(), $data->getServerId()); } public static function provideSnowflakeData(): array {