Enable FileDB in prod and only clean up storage dirs that are older

assets/featureFlag/prod.json

@@ -45,8 +45,8 @@
             ]
         },
         "FileDb": {
-            "enabled": false,
-            "fleetPercentage": 0
+            "enabled": true,
+            "fleetPercentage": 5
         }
     }
 }

src/datastore/FileStoreFactory.ts

@@ -10,6 +10,7 @@ import { DataStore, DataStoreFactory, PersistedStores, StoreName, TotalMaxDatast
 import { encryptionKey } from './file/Encryption';
 import { KeyedFileStore } from './file/KeyedFileStore';
 import { recordDiscardedData, recordDiskUsage, recordOutOfDiskFailure, StoreOperation } from './Utils';
+import { isOlderVersionDirectory } from './VersionDirectory';
 
 export class FileStoreFactory implements DataStoreFactory {
     private readonly log = LoggerFactory.getLogger('FileStore.Global');
@@ -119,7 +120,7 @@ export class FileStoreFactory implements DataStoreFactory {
         const entries = readdirSync(this.fileDbRoot, { withFileTypes: true });
         for (const entry of entries) {
             try {
-                if (entry.name !== Version) {
+                if (entry.isDirectory() && isOlderVersionDirectory(entry.name, VersionNumber)) {
                     this.telemetry.count('oldVersion.cleanup.count', 1);
                     rmSync(join(this.fileDbRoot, entry.name), { recursive: true, force: true });
                 }

src/datastore/LMDBStoreFactory.ts

@@ -15,6 +15,7 @@ import { LMDBOwnershipTracker } from './lmdb/OwnershipTracker';
 import { stats } from './lmdb/Stats';
 import { encryptionStrategy } from './lmdb/Utils';
 import { recordDiscardedData, recordDiskUsage, recordOutOfDiskFailure, StoreOperation } from './Utils';
+import { isOlderVersionDirectory } from './VersionDirectory';
 
 const MetricsIntervalMs = 60 * 1000;
 const CleanupDelayMs = 2 * 60 * 1000;
@@ -343,12 +344,14 @@ export class LMDBStoreFactory implements DataStoreFactory {
         const entries = readdirSync(this.lmdbDir, { withFileTypes: true });
         for (const entry of entries) {
             try {
-                if (entry.name === Version || entry.name === LMDBOwnershipTracker.DirName) {
-                    continue;
+                if (
+                    entry.isDirectory() &&
+                    isOlderVersionDirectory(entry.name, VersionNumber) &&
+                    entry.name !== LMDBOwnershipTracker.DirName
+                ) {
+                    this.telemetry.count('oldVersion.cleanup.count', 1);
+                    rmSync(join(this.lmdbDir, entry.name), { recursive: true, force: true });
                 }
-
-                this.telemetry.count('oldVersion.cleanup.count', 1);
-                rmSync(join(this.lmdbDir, entry.name), { recursive: true, force: true });
             } catch (error) {
                 this.log.error(error, 'Failed to cleanup old LMDB versions');
                 this.telemetry.count('oldVersion.cleanup.error', 1);

src/datastore/VersionDirectory.ts

@@ -0,0 +1,24 @@
+const VersionDirectoryPattern = /^v(\d+)$/;
+
+/**
+ * True only for a strict `v<number>` directory whose version is below {@link currentVersion}.
+ *
+ * Background cleanup runs from whichever binary happens to be executing, and an older binary must
+ * never delete the store a newer binary is actively using. So this returns false for the current
+ * version, any newer version, non-version names (markers, stray files, malformed names), and any
+ * digit string too large to compare as a safe integer — leaving only strictly older stores eligible
+ * for removal.
+ */
+export function isOlderVersionDirectory(directoryName: string, currentVersion: number): boolean {
+    const match = VersionDirectoryPattern.exec(directoryName);
+    if (match === null) {
+        return false;
+    }
+
+    const version = Number(match[1]);
+    if (!Number.isSafeInteger(version)) {
+        return false;
+    }
+
+    return version < currentVersion;
+}

tst/unit/datastore/FileStore.test.ts

@@ -548,22 +548,27 @@ describe('FileStore', () => {
             expect(factory.timeout.hasRef()).toBe(false);
         });
 
-        it('should cleanup old version directories', () => {
+        it('should remove older version directories but preserve current, newer, and unrelated directories', () => {
             const fileDbRoot = join(testDir, 'filedb');
 
-            // Create old version directories
             mkdirSync(join(fileDbRoot, 'v1'), { recursive: true });
             writeFileSync(join(fileDbRoot, 'v1', 'data.enc'), 'old');
+            mkdirSync(join(fileDbRoot, 'v4'), { recursive: true });
+            writeFileSync(join(fileDbRoot, 'v4', 'data.enc'), 'newer');
+            mkdirSync(join(fileDbRoot, 'backup'), { recursive: true });
+            writeFileSync(join(fileDbRoot, 'v2'), 'not a directory');
 
-            // Current version should exist from factory constructor
+            // Current version exists from the factory constructor
             expect(existsSync(join(fileDbRoot, 'v3'))).toBe(true);
-            expect(existsSync(join(fileDbRoot, 'v1'))).toBe(true);
 
             // Trigger cleanup directly (normally runs after 2min timeout)
-            (fileFactory as any).cleanupOldVersions();
+            (fileFactory as unknown as { cleanupOldVersions(): void }).cleanupOldVersions();
 
             expect(existsSync(join(fileDbRoot, 'v1'))).toBe(false);
             expect(existsSync(join(fileDbRoot, 'v3'))).toBe(true);
+            expect(existsSync(join(fileDbRoot, 'v4'))).toBe(true);
+            expect(existsSync(join(fileDbRoot, 'backup'))).toBe(true);
+            expect(existsSync(join(fileDbRoot, 'v2'))).toBe(true);
         });
 
         it('should handle cleanup when directory does not exist', async () => {

tst/unit/datastore/LMDB.recovery.test.ts

@@ -239,16 +239,22 @@ describe('LMDB fork detection and recovery', () => {
             await factory.close();
         });
 
-        it('should remove old version directories but preserve the markers directory', () => {
+        it('should remove older version directories but preserve current, newer, unrelated, and marker directories', () => {
             const lmdbDir = join(testDir, 'lmdb');
             const markersDir = join(lmdbDir, LMDBOwnershipTracker.DirName);
             fs.mkdirSync(join(lmdbDir, 'v1'), { recursive: true });
+            fs.mkdirSync(join(lmdbDir, 'v7'), { recursive: true });
+            fs.mkdirSync(join(lmdbDir, 'backup'), { recursive: true });
+            fs.writeFileSync(join(lmdbDir, 'v2'), 'not a directory');
 
             (factory as unknown as { cleanupOldVersions(): void }).cleanupOldVersions();
 
             expect(fs.existsSync(join(lmdbDir, 'v1'))).toBe(false);
-            expect(fs.existsSync(markersDir)).toBe(true);
             expect(fs.existsSync(join(lmdbDir, 'v6'))).toBe(true);
+            expect(fs.existsSync(join(lmdbDir, 'v7'))).toBe(true);
+            expect(fs.existsSync(join(lmdbDir, 'backup'))).toBe(true);
+            expect(fs.existsSync(join(lmdbDir, 'v2'))).toBe(true);
+            expect(fs.existsSync(markersDir)).toBe(true);
         });
     });
 });

tst/unit/datastore/VersionDirectory.test.ts

@@ -0,0 +1,41 @@
+import { describe, it, expect } from 'vitest';
+import { isOlderVersionDirectory } from '../../../src/datastore/VersionDirectory';
+
+describe('isOlderVersionDirectory', () => {
+    const currentVersion = 6;
+
+    it('should return true for a strictly older version directory', () => {
+        expect(isOlderVersionDirectory('v1', currentVersion)).toBe(true);
+        expect(isOlderVersionDirectory('v5', currentVersion)).toBe(true);
+    });
+
+    it('should treat v0 as an older version directory', () => {
+        expect(isOlderVersionDirectory('v0', currentVersion)).toBe(true);
+    });
+
+    it('should return false for the current version directory', () => {
+        expect(isOlderVersionDirectory('v6', currentVersion)).toBe(false);
+    });
+
+    it('should return false for a newer version directory', () => {
+        expect(isOlderVersionDirectory('v7', currentVersion)).toBe(false);
+        expect(isOlderVersionDirectory('v100', currentVersion)).toBe(false);
+    });
+
+    it('should return false for non-version directory names', () => {
+        for (const name of ['markers', 'backup', 'lmdb', 'v', 'version1', 'V1', '1', '']) {
+            expect(isOlderVersionDirectory(name, currentVersion)).toBe(false);
+        }
+    });
+
+    it('should return false for names that only partially match the version format', () => {
+        for (const name of ['v1.0', 'v-1', 'v1 ', ' v1', 'v1a', 'av1', 'v1/', 'v_1']) {
+            expect(isOlderVersionDirectory(name, currentVersion)).toBe(false);
+        }
+    });
+
+    it('should return false for a numeric value too large to compare as a safe integer', () => {
+        const overflowing = `v${'9'.repeat(30)}`;
+        expect(isOlderVersionDirectory(overflowing, currentVersion)).toBe(false);
+    });
+});