From 7316ef25b8f6a23ef3e797b5e00d7f43dd1bed4e Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Wed, 30 Sep 2026 17:22:11 +0200 Subject: [PATCH 1/3] fix(snowflake): pass sequence ID arguments in the declared order ISequence::nextId() takes the server id, the seconds and the milliseconds, but the generator passed the seconds, the milliseconds and the server id, so the sequences were keyed on the wrong values. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Carl Schwan --- lib/private/Snowflake/SnowflakeGenerator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/Snowflake/SnowflakeGenerator.php b/lib/private/Snowflake/SnowflakeGenerator.php index a73253d246c7f..18861e817bc36 100644 --- a/lib/private/Snowflake/SnowflakeGenerator.php +++ b/lib/private/Snowflake/SnowflakeGenerator.php @@ -41,7 +41,7 @@ public function nextId(?DateTimeImmutable $timestamp = null): string { $serverId = $this->serverInfo->getServerId(); $isCli = (int)$this->isCli(); // 1 bit - $sequenceId = $this->sequenceGenerator->nextId($seconds, $milliseconds, $serverId); // 12 bits + $sequenceId = $this->sequenceGenerator->nextId($serverId, $seconds, $milliseconds); // 12 bits if ($sequenceId > 0xFFF || $sequenceId === false) { // Throttle a bit, wait for next millisecond usleep(1000); From fb5d9aed0bf4303d75f58041abafa8e972f3b5f5 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Wed, 30 Sep 2026 17:22:12 +0200 Subject: [PATCH 2/3] perf(snowflake): use fixed slots and drop fsync in FileSequence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Store one slot per second of the TTL window and per millisecond in each lock file, so generating an id only reads and writes 8 bytes instead of decoding, filtering and rewriting a JSON map of the last 30 seconds. The fsync is not needed as the files only coordinate processes running at the same time. Generating an id went from 660µs to 22µs with the temporary directory on disk, and from 24µs to 11µs on tmpfs. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Carl Schwan --- lib/private/Snowflake/FileSequence.php | 50 +++++++++--------------- tests/lib/Snowflake/FileSequenceTest.php | 37 ++++++++++++++++-- 2 files changed, 53 insertions(+), 34 deletions(-) diff --git a/lib/private/Snowflake/FileSequence.php b/lib/private/Snowflake/FileSequence.php index 463dfd762e195..22af1914bc087 100644 --- a/lib/private/Snowflake/FileSequence.php +++ b/lib/private/Snowflake/FileSequence.php @@ -19,9 +19,11 @@ class FileSequence implements ISequence { /** Lock file directory **/ public const LOCK_FILE_DIRECTORY = 'sfi_file_sequence'; /** Lock filename format **/ - private const string LOCK_FILE_FORMAT = 'seq-%03d.lock'; - /** Delete sequences after SEQUENCE_TTL seconds **/ + private const string LOCK_FILE_FORMAT = 'seq-%03d.slots'; + /** Reuse the slot of a sequence after SEQUENCE_TTL seconds **/ private const int SEQUENCE_TTL = 30; + /** Size of a slot: the seconds and the last sequence ID, both as unsigned 32-bit integers **/ + private const int SLOT_SIZE = 8; private string $workDir; @@ -69,42 +71,28 @@ public function nextId(int $serverId, int $seconds, int $milliseconds): int { throw new \Exception('Unable to acquire lock on sequence ID file: ' . $filePath); } - // Read content - $content = (string)fgets($fp); - $locks = $content === '' - ? [] - : json_decode($content, true, 3, JSON_THROW_ON_ERROR); - - // Generate new ID - if (isset($locks[$seconds])) { - if (isset($locks[$seconds][$milliseconds])) { - ++$locks[$seconds][$milliseconds]; - } else { - $locks[$seconds][$milliseconds] = 0; + // Each file holds one slot per second of the TTL window and per millisecond mapped to it + $slot = (($seconds % self::SEQUENCE_TTL + self::SEQUENCE_TTL) % self::SEQUENCE_TTL) * intdiv(1000, self::NB_FILES) + + intdiv($milliseconds, self::NB_FILES); + $slotSecondsKey = $seconds & 0xFFFFFFFF; + fseek($fp, $slot * self::SLOT_SIZE); + $data = fread($fp, self::SLOT_SIZE); + $sequenceId = 0; + if (is_string($data) && strlen($data) === self::SLOT_SIZE) { + ['seconds' => $slotSeconds, 'sequence' => $slotSequenceId] = unpack('Nseconds/Nsequence', $data); + if ($slotSeconds === $slotSecondsKey) { + $sequenceId = $slotSequenceId + 1; } - } else { - $locks[$seconds] = [ - $milliseconds => 0 - ]; } - // Clean old sequence IDs - $cleanBefore = $seconds - self::SEQUENCE_TTL; - $locks = array_filter($locks, static function ($key) use ($cleanBefore) { - return $key >= $cleanBefore; - }, ARRAY_FILTER_USE_KEY); - - // Write data - ftruncate($fp, 0); - $content = json_encode($locks, JSON_THROW_ON_ERROR); - rewind($fp); - fwrite($fp, $content); - fsync($fp); + // No fsync needed, the file only coordinates processes running at the same time + fseek($fp, $slot * self::SLOT_SIZE); + fwrite($fp, pack('NN', $slotSecondsKey, $sequenceId)); // Release lock fclose($fp); - return $locks[$seconds][$milliseconds]; + return $sequenceId; } private function getFilePath(int $fileId): string { diff --git a/tests/lib/Snowflake/FileSequenceTest.php b/tests/lib/Snowflake/FileSequenceTest.php index bec2dab7b7ce5..8b2d69179c072 100644 --- a/tests/lib/Snowflake/FileSequenceTest.php +++ b/tests/lib/Snowflake/FileSequenceTest.php @@ -23,18 +23,49 @@ public function setUp():void { parent::setUp(); $tempManager = $this->createMock(ITempManager::class); - $this->path = sys_get_temp_dir(); + $this->path = sys_get_temp_dir() . '/' . uniqid('file_sequence_test_'); + mkdir($this->path); $tempManager->method('getTempBaseDir')->willReturn($this->path); $this->sequence = new FileSequence($tempManager); } #[\Override] public function tearDown():void { - $lockDirectory = $this->path . '/' . FileSequence::LOCK_FILE_DIRECTORY; - foreach (glob($lockDirectory . '/*') as $file) { + foreach (glob($this->path . '/' . FileSequence::LOCK_FILE_DIRECTORY . '*/*') as $file) { unlink($file); } + foreach (glob($this->path . '/' . FileSequence::LOCK_FILE_DIRECTORY . '*') as $directory) { + rmdir($directory); + } + rmdir($this->path); parent::tearDown(); } + + public function testSequenceIncrementsWithinTheSameMillisecond(): void { + $this->assertSame(0, $this->sequence->nextId(42, 1000, 500)); + $this->assertSame(1, $this->sequence->nextId(42, 1000, 500)); + $this->assertSame(2, $this->sequence->nextId(42, 1000, 500)); + } + + public function testSequenceStartsAtZeroForEachMillisecond(): void { + $this->assertSame(0, $this->sequence->nextId(42, 1000, 500)); + // Same lock file, different slot + $this->assertSame(0, $this->sequence->nextId(42, 1000, 520)); + $this->assertSame(0, $this->sequence->nextId(42, 1001, 500)); + // Same slot, reused after the TTL window + $this->assertSame(0, $this->sequence->nextId(42, 1030, 500)); + } + + public function testSequenceKeepsEarlierMillisecondsWithinTheWindow(): void { + $this->assertSame(0, $this->sequence->nextId(42, 1000, 500)); + $this->assertSame(0, $this->sequence->nextId(42, 1001, 500)); + $this->assertSame(1, $this->sequence->nextId(42, 1000, 500)); + } + + public function testSequenceWithSecondsBeforeTheEpoch(): void { + $this->assertSame(0, $this->sequence->nextId(42, -5, 10)); + $this->assertSame(1, $this->sequence->nextId(42, -5, 10)); + $this->assertSame(0, $this->sequence->nextId(42, 25, 10)); + } } From e69b8ed7cbef90199f5abff9fa03d2c4870e4ac6 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 1 Oct 2026 10:12:44 +0200 Subject: [PATCH 3/3] fix(snowflake): Reject time before epoch This allow to simplify a bit the code, also add a small description of the algorithm. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Carl Schwan --- lib/private/Snowflake/FileSequence.php | 21 +++++++++++++++----- lib/private/Snowflake/ISequence.php | 2 ++ lib/private/Snowflake/SnowflakeGenerator.php | 3 +++ lib/public/Snowflake/ISnowflakeGenerator.php | 1 + tests/lib/Snowflake/FileSequenceTest.php | 6 ------ tests/lib/Snowflake/GeneratorTest.php | 7 +++++++ 6 files changed, 29 insertions(+), 11 deletions(-) diff --git a/lib/private/Snowflake/FileSequence.php b/lib/private/Snowflake/FileSequence.php index 22af1914bc087..067bf39134993 100644 --- a/lib/private/Snowflake/FileSequence.php +++ b/lib/private/Snowflake/FileSequence.php @@ -13,6 +13,19 @@ use OCP\ITempManager; use Override; +/** + * Sequence IDs shared between processes through lock files in the temporary directory. + * + * Each lock file is a table of counters. To get a sequence ID, the file chosen by the + * millisecond (ms % NB_FILES) is locked and the slot of that second and millisecond is + * read: ((seconds % SEQUENCE_TTL) * (1000 / NB_FILES) + ms / NB_FILES). A slot holds the + * seconds it was last used for and the last sequence ID handed out. If the seconds match, + * the next sequence ID is that one plus one, otherwise it starts at 0. The result is + * written back and the file unlocked. + * + * A slot is reused SEQUENCE_TTL seconds later, the seconds mismatch then resets its counter. + * No fsync is needed, as the files only coordinate processes running at the same time. + */ class FileSequence implements ISequence { /** Number of files to use */ private const int NB_FILES = 20; @@ -72,22 +85,20 @@ public function nextId(int $serverId, int $seconds, int $milliseconds): int { } // Each file holds one slot per second of the TTL window and per millisecond mapped to it - $slot = (($seconds % self::SEQUENCE_TTL + self::SEQUENCE_TTL) % self::SEQUENCE_TTL) * intdiv(1000, self::NB_FILES) - + intdiv($milliseconds, self::NB_FILES); - $slotSecondsKey = $seconds & 0xFFFFFFFF; + $slot = ($seconds % self::SEQUENCE_TTL) * intdiv(1000, self::NB_FILES) + intdiv($milliseconds, self::NB_FILES); fseek($fp, $slot * self::SLOT_SIZE); $data = fread($fp, self::SLOT_SIZE); $sequenceId = 0; if (is_string($data) && strlen($data) === self::SLOT_SIZE) { ['seconds' => $slotSeconds, 'sequence' => $slotSequenceId] = unpack('Nseconds/Nsequence', $data); - if ($slotSeconds === $slotSecondsKey) { + if ($slotSeconds === $seconds) { $sequenceId = $slotSequenceId + 1; } } // No fsync needed, the file only coordinates processes running at the same time fseek($fp, $slot * self::SLOT_SIZE); - fwrite($fp, pack('NN', $slotSecondsKey, $sequenceId)); + fwrite($fp, pack('NN', $seconds, $sequenceId)); // Release lock fclose($fp); diff --git a/lib/private/Snowflake/ISequence.php b/lib/private/Snowflake/ISequence.php index e5e1f6e414d32..22c5f52a840dc 100644 --- a/lib/private/Snowflake/ISequence.php +++ b/lib/private/Snowflake/ISequence.php @@ -20,6 +20,8 @@ public function isAvailable(): bool; /** * Returns next sequence ID for current time and server + * + * @param non-negative-int $seconds seconds since the Snowflake epoch */ public function nextId(int $serverId, int $seconds, int $milliseconds): int|false; } diff --git a/lib/private/Snowflake/SnowflakeGenerator.php b/lib/private/Snowflake/SnowflakeGenerator.php index 18861e817bc36..a720a19c79595 100644 --- a/lib/private/Snowflake/SnowflakeGenerator.php +++ b/lib/private/Snowflake/SnowflakeGenerator.php @@ -37,6 +37,9 @@ public function nextId(?DateTimeImmutable $timestamp = null): string { // Relative time $seconds = $timestamp->getTimestamp() - self::TS_OFFSET; + if ($seconds < 0) { + throw new \InvalidArgumentException('Snowflake IDs cannot be generated for a time before ' . date(DATE_ATOM, self::TS_OFFSET)); + } $milliseconds = (int)$timestamp->format('v'); $serverId = $this->serverInfo->getServerId(); diff --git a/lib/public/Snowflake/ISnowflakeGenerator.php b/lib/public/Snowflake/ISnowflakeGenerator.php index 475d376788313..d64bb02a1cb13 100644 --- a/lib/public/Snowflake/ISnowflakeGenerator.php +++ b/lib/public/Snowflake/ISnowflakeGenerator.php @@ -42,6 +42,7 @@ interface ISnowflakeGenerator { * * @param ?DateTimeImmutable $timestamp Generate the Snowflake ID for a specific time. This should only be used in very special cases. * @return non-empty-string + * @throws \InvalidArgumentException if the timestamp is before 2025-10-01, as such a time cannot be represented * * @since 33.0 */ diff --git a/tests/lib/Snowflake/FileSequenceTest.php b/tests/lib/Snowflake/FileSequenceTest.php index 8b2d69179c072..f58c8a8213bca 100644 --- a/tests/lib/Snowflake/FileSequenceTest.php +++ b/tests/lib/Snowflake/FileSequenceTest.php @@ -62,10 +62,4 @@ public function testSequenceKeepsEarlierMillisecondsWithinTheWindow(): void { $this->assertSame(0, $this->sequence->nextId(42, 1001, 500)); $this->assertSame(1, $this->sequence->nextId(42, 1000, 500)); } - - public function testSequenceWithSecondsBeforeTheEpoch(): void { - $this->assertSame(0, $this->sequence->nextId(42, -5, 10)); - $this->assertSame(1, $this->sequence->nextId(42, -5, 10)); - $this->assertSame(0, $this->sequence->nextId(42, 25, 10)); - } } diff --git a/tests/lib/Snowflake/GeneratorTest.php b/tests/lib/Snowflake/GeneratorTest.php index a3d60f7b93efe..733612d53b52b 100644 --- a/tests/lib/Snowflake/GeneratorTest.php +++ b/tests/lib/Snowflake/GeneratorTest.php @@ -66,6 +66,13 @@ public function testGenerator(): void { $this->assertEquals($this->serverInfo->getServerId(), $data->getServerId()); } + public function testGeneratorRejectsTimestampBeforeEpoch(): void { + $generator = new SnowflakeGenerator(new TimeFactory(), $this->sequence, $this->serverInfo); + + $this->expectException(\InvalidArgumentException::class); + $generator->nextId(new \DateTimeImmutable('2025-09-30 23:59:59 UTC')); + } + public function testMinForTime(): void { $generator = new SnowflakeGenerator(new TimeFactory(), $this->sequence, $this->serverInfo); $now = time();