diff --git a/core/BackgroundJobs/CleanupBackgroundJobsJob.php b/core/BackgroundJobs/CleanupBackgroundJobsJob.php new file mode 100644 index 0000000000000..073fe93e538d5 --- /dev/null +++ b/core/BackgroundJobs/CleanupBackgroundJobsJob.php @@ -0,0 +1,59 @@ +setInterval(60 * 60); + $this->setTimeSensitivity(IJob::TIME_INSENSITIVE); + } + + #[Override] + protected function run($argument): void { + $this->reapCrashedJobs(); + + // TODO Clean oldest jobs + } + + private function reapCrashedJobs(): void { + $currentServerId = Util::getServerId(); + + foreach ($this->jobRuns->runningJobs(1000) as $job) { + if ($job->serverId !== $currentServerId) { + continue; + } + 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 + = ((int)$maxDuration->format('%a') * 24 * 60 * 60 * 1000) + + ((int)$maxDuration->format('%h') * 60 * 60 * 1000) + + ((int)$maxDuration->format('%m') * 60 * 1000) + + ((int)$maxDuration->format('%s') * 1000) + + (int)((int)$maxDuration->format('%f') / 1000); + $this->jobRuns->finished($job->runId, $maxDuration, 0, JobStatus::CRASHED); + } + } + } +} 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/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..255dfcdc30555 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) { @@ -102,24 +101,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/Util.php b/lib/public/Util.php index 9fc9b9e759713..e40a34aec73e4 100644 --- a/lib/public/Util.php +++ b/lib/public/Util.php @@ -140,9 +140,7 @@ public static function addScript(string $application, ?string $file = null, stri // Inject js translations if we load a script for // a specific app that is not core, as those js files // need separate handling - if ($application !== 'core' - && $file !== null - && !str_contains($file, 'l10n')) { + if ($application !== 'core' && $file !== null && !str_contains($file, 'l10n')) { self::addTranslations($application); } @@ -188,12 +186,10 @@ public static function getScripts(): array { * @since 34.0.0 */ private static function scriptOrderValue(string $name): int { - return match($name) { + return match ($name) { 'core/js/common' => 3, 'core/js/main' => 2, - default => str_starts_with($name, 'core/l10n/') - ? 1 // core translations have to be loaded directly after core-main - : 0, // other scripts should preserve their current order + default => str_starts_with($name, 'core/l10n/') ? 1 : 0, // core translations have to be loaded directly after core-main // other scripts should preserve their current order }; } @@ -246,9 +242,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 +273,25 @@ public static function getServerHostName() { return $host_name; } + /** + * Returns configured Server ID or use default fallback + * @since 34.0.0 + */ + 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 +496,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 +531,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..944e6259d3e0c 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,7 @@ public function testGenerator(): void { $this->assertTrue($data->isCli()); // Check serverId - $this->assertEquals(42, $data->getServerId()); + $this->assertEquals(Util::getServerId(), $data->getServerId()); } #[DataProvider('provideSnowflakeData')] @@ -70,12 +66,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 {