diff --git a/API-INTERNAL.md b/API-INTERNAL.md index 54dc85d21..bc8c0b467 100644 --- a/API-INTERNAL.md +++ b/API-INTERNAL.md @@ -2,9 +2,21 @@ # Internal API Reference +## Constants + +
+
DISK_PRESSURE_LOG_INTERVAL_MS
+

Minimum interval between disk-pressure alerts. One disk-pressure burst fails every queued operation +with the identical error, so per-operation logging would amplify the very storm it reports.

+
+
+ ## Functions
+
resetDiskPressureLogThrottle()
+

Test-only: clears the disk-pressure log throttle so each test observes its own alert.

+
getMergeQueue()

Getter - returns the merge queue.

@@ -89,6 +101,9 @@ and alerted (fatal). Retrying here would only re-amplify, so we skip the write q
  • CAPACITY: evicts the least recently accessed evictable key and retries, under a session-level circuit breaker (see lib/StorageCircuitBreaker.ts) that halts the loop once eviction stops making progress or failures storm — the per-operation budget alone cannot stop a session-wide storm.
    • +
  • DISK_PRESSURE: the device disk itself is full (or the database files are unreadable), so neither +retries nor in-DB eviction can free space — the write is dropped (cache stays authoritative) with +a single throttled alert + quota snapshot per burst.
  • UNKNOWN: the provider couldn't classify it — log the full error shape (name + message + provider) once so it's visible, then bounded retry without eviction.
    • @@ -157,6 +172,19 @@ Retries on failure.

    + + +## DISK\_PRESSURE\_LOG\_INTERVAL\_MS +Minimum interval between disk-pressure alerts. One disk-pressure burst fails every queued operation +with the identical error, so per-operation logging would amplify the very storm it reports. + +**Kind**: global constant + + +## resetDiskPressureLogThrottle() +Test-only: clears the disk-pressure log throttle so each test observes its own alert. + +**Kind**: global function ## getMergeQueue() @@ -333,6 +361,9 @@ capacity recovery (eviction) so that a given failure is retried by exactly one l - CAPACITY: evicts the least recently accessed evictable key and retries, under a session-level circuit breaker (see lib/StorageCircuitBreaker.ts) that halts the loop once eviction stops making progress or failures storm — the per-operation budget alone cannot stop a session-wide storm. +- DISK_PRESSURE: the device disk itself is full (or the database files are unreadable), so neither + retries nor in-DB eviction can free space — the write is dropped (cache stays authoritative) with + a single throttled alert + quota snapshot per burst. - UNKNOWN: the provider couldn't classify it — log the full error shape (name + message + provider) once so it's visible, then bounded retry without eviction. diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 4dc1ba2ce..31c50c38e 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -52,6 +52,16 @@ const METHOD = { // Max number of retries for failed storage operations const MAX_STORAGE_OPERATION_RETRY_ATTEMPTS = 5; +/** Minimum interval between disk-pressure alerts. One disk-pressure burst fails every queued operation + * with the identical error, so per-operation logging would amplify the very storm it reports. */ +const DISK_PRESSURE_LOG_INTERVAL_MS = 60000; +let lastDiskPressureLogTime = 0; + +/** Test-only: clears the disk-pressure log throttle so each test observes its own alert. */ +function resetDiskPressureLogThrottle(): void { + lastDiskPressureLogTime = 0; +} + type OnyxMethod = ValueOf; // Key/value store of Onyx key and arrays of values to merge @@ -783,6 +793,9 @@ function reportStorageQuota(error?: Error): Promise { * - CAPACITY: evicts the least recently accessed evictable key and retries, under a session-level * circuit breaker (see lib/StorageCircuitBreaker.ts) that halts the loop once eviction stops making * progress or failures storm — the per-operation budget alone cannot stop a session-wide storm. + * - DISK_PRESSURE: the device disk itself is full (or the database files are unreadable), so neither + * retries nor in-DB eviction can free space — the write is dropped (cache stays authoritative) with + * a single throttled alert + quota snapshot per burst. * - UNKNOWN: the provider couldn't classify it — log the full error shape (name + message + * provider) once so it's visible, then bounded retry without eviction. */ @@ -806,6 +819,19 @@ function retryOperation( return Promise.resolve(); } + // DISK_PRESSURE: the device disk is full, so neither retries nor eviction can succeed until the OS + // frees space. Drop the write (cache stays authoritative) and log one alert + quota snapshot per + // interval — the snapshot's free-disk bytes let telemetry confirm (or rule out) disk pressure. + if (errorClass === StorageErrorClass.DISK_PRESSURE) { + const now = Date.now(); + if (now - lastDiskPressureLogTime < DISK_PRESSURE_LOG_INTERVAL_MS) { + return Promise.resolve(); + } + lastDiskPressureLogTime = now; + Logger.logAlert(`Disk-pressure storage error; skipping retries. provider: ${Storage.getStorageProvider().name}. message: ${error?.message}. onyxMethod: ${onyxMethod.name}.`); + return reportStorageQuota(error); + } + Logger.logInfo( `Failed to save to storage. Error: ${error}. class: ${errorClass}. onyxMethod: ${onyxMethod.name}. retryAttempt: ${currentRetryAttempt}/${MAX_STORAGE_OPERATION_RETRY_ATTEMPTS}`, ); @@ -1816,6 +1842,7 @@ const OnyxUtils = { getCollectionDataAndSendAsObject, remove, reportStorageQuota, + resetDiskPressureLogThrottle, retryOperation, broadcastUpdate, hasPendingMergeForKey, diff --git a/lib/storage/errors.ts b/lib/storage/errors.ts index 635f55428..631b01535 100644 --- a/lib/storage/errors.ts +++ b/lib/storage/errors.ts @@ -15,6 +15,10 @@ const StorageErrorClass = { TRANSIENT: 'transient', /** Quota exceeded / disk full. Owner: operation layer — evict and retry. */ CAPACITY: 'capacity', + /** Filesystem-level failure around the database files (device disk full, or the files cannot be + * created/read). Owner: operation layer — skip retries and eviction (neither can free OS-level + * space) and log one throttled alert + quota snapshot per burst. */ + DISK_PRESSURE: 'diskPressure', /** Non-serializable payload. Never retriable — the same data will always fail. */ INVALID_DATA: 'invalidData', /** Backing-store corruption. Owner: connection layer — budgeted heal, then give up. */ diff --git a/lib/storage/providers/SQLiteProvider.ts b/lib/storage/providers/SQLiteProvider.ts index c03efb66e..013139120 100644 --- a/lib/storage/providers/SQLiteProvider.ts +++ b/lib/storage/providers/SQLiteProvider.ts @@ -325,16 +325,20 @@ const provider: StorageProvider = { throw new Error('Store is not initialized!'); } - return Promise.all([provider.store.executeAsync('PRAGMA page_size;'), provider.store.executeAsync('PRAGMA page_count;'), getFreeDiskStorage()]).then( - ([pageSizeResult, pageCountResult, bytesRemaining]) => { + // The PRAGMAs need the SQLite connection; getFreeDiskStorage() is filesystem-level. Degrade + // bytesUsed to -1 instead of failing, so the free-disk bytes always get logged. + const bytesUsedPromise = Promise.all([provider.store.executeAsync('PRAGMA page_size;'), provider.store.executeAsync('PRAGMA page_count;')]) + .then(([pageSizeResult, pageCountResult]) => { const pageSize = pageSizeResult.rows?.item(0)?.page_size ?? 0; const pageCount = pageCountResult.rows?.item(0)?.page_count ?? 0; - return { - bytesUsed: pageSize * pageCount, - bytesRemaining, - }; - }, - ); + return pageSize * pageCount; + }) + .catch(() => -1); + + return Promise.all([bytesUsedPromise, getFreeDiskStorage()]).then(([bytesUsed, bytesRemaining]) => ({ + bytesUsed, + bytesRemaining, + })); }, }; diff --git a/lib/storage/providers/classifySQLiteError.ts b/lib/storage/providers/classifySQLiteError.ts index 2105764c2..06d4f7d51 100644 --- a/lib/storage/providers/classifySQLiteError.ts +++ b/lib/storage/providers/classifySQLiteError.ts @@ -18,6 +18,10 @@ function classifySQLiteError(error: unknown): ValueOf return StorageErrorClass.CAPACITY; } + if (message.includes('disk i/o error') || message.includes('unable to open database file')) { + return StorageErrorClass.DISK_PRESSURE; + } + return StorageErrorClass.UNKNOWN; } diff --git a/tests/unit/onyxUtilsTest.ts b/tests/unit/onyxUtilsTest.ts index 93899cfdc..0a20a7d21 100644 --- a/tests/unit/onyxUtilsTest.ts +++ b/tests/unit/onyxUtilsTest.ts @@ -748,8 +748,12 @@ describe('OnyxUtils', () => { const diskFullError = new Error('database or disk is full'); const nonRetriableIdbError = Object.assign(new Error('Internal error opening backing store for indexedDB.open.'), {name: 'UnknownError'}); - // The circuit breaker is process-scoped, so reset it between tests to avoid state leaking. - beforeEach(() => StorageCircuitBreaker.reset()); + // The circuit breaker and the disk-pressure log throttle are process-scoped, so reset them + // between tests to avoid state leaking. + beforeEach(() => { + StorageCircuitBreaker.reset(); + OnyxUtils.resetDiskPressureLogThrottle(); + }); it('should retry only one time if the operation is firstly failed and then passed', async () => { StorageMock.setItem = jest.fn(StorageMock.setItem).mockRejectedValueOnce(genericError).mockImplementation(StorageMock.setItem); @@ -808,6 +812,41 @@ describe('OnyxUtils', () => { expect(retryOperationSpy).toHaveBeenCalledTimes(1); }); + it.each([['[NativeNitroSQLiteException][SqlExecutionError] disk I/O error'], ['[NativeNitroSQLiteException][SqlExecutionError] unable to open database file']])( + 'should not retry disk-pressure errors (%s)', + async (message) => { + StorageMock.setItem = jest.fn().mockRejectedValue(new Error(message)); + + await Onyx.set(ONYXKEYS.TEST_KEY, {test: 'data'}); + + // Called once (initial attempt only): retries cannot succeed while the device disk is full. + expect(retryOperationSpy).toHaveBeenCalledTimes(1); + }, + ); + + it('should log a single throttled alert with a quota snapshot for a disk-pressure burst', async () => { + const logAlertSpy = jest.spyOn(Logger, 'logAlert'); + const logInfoSpy = jest.spyOn(Logger, 'logInfo'); + const diskIOError = new Error('[NativeNitroSQLiteException][SqlExecutionError] disk I/O error'); + StorageMock.setItem = jest.fn().mockRejectedValue(diskIOError); + + // A burst: several operations all failing with the identical error. + await Onyx.set(ONYXKEYS.TEST_KEY, {test: 'data'}); + await Onyx.set(ONYXKEYS.TEST_KEY, {test: 'data2'}); + await Onyx.set(ONYXKEYS.TEST_KEY, {test: 'data3'}); + + // One alert for the whole burst (the rest are throttled within DISK_PRESSURE_LOG_INTERVAL_MS)... + const alerts = logAlertSpy.mock.calls.filter((call) => typeof call[0] === 'string' && call[0].startsWith('Disk-pressure storage error')); + expect(alerts).toHaveLength(1); + expect(alerts[0][0]).toBe(`Disk-pressure storage error; skipping retries. provider: MemoryOnlyProvider. message: ${diskIOError.message}. onyxMethod: setWithRetry.`); + // ...paired with exactly one quota snapshot (carries the free-disk bytes on native platforms). + const quotaLogs = logInfoSpy.mock.calls.filter((call) => typeof call[0] === 'string' && call[0].startsWith('Storage Quota Check')); + expect(quotaLogs).toHaveLength(1); + // And no "failed after N retries" alert: the writes were dropped, not retried to exhaustion. + const retryAlerts = logAlertSpy.mock.calls.filter((call) => typeof call[0] === 'string' && call[0].startsWith('Storage operation failed after')); + expect(retryAlerts).toHaveLength(0); + }); + it('should skip retry quietly (info, not alert) for fatal connection-layer errors', async () => { const logAlertSpy = jest.spyOn(Logger, 'logAlert'); const logInfoSpy = jest.spyOn(Logger, 'logInfo'); diff --git a/tests/unit/storage/providers/SQLiteProviderTest.ts b/tests/unit/storage/providers/SQLiteProviderTest.ts index 4d82b2e31..55da98760 100644 --- a/tests/unit/storage/providers/SQLiteProviderTest.ts +++ b/tests/unit/storage/providers/SQLiteProviderTest.ts @@ -453,5 +453,15 @@ describe('SQLiteProvider', () => { expect(after.bytesUsed).toBeGreaterThan(before.bytesUsed); }); + + it('should still report free-disk bytes when the PRAGMAs fail (disk pressure)', async () => { + // During disk pressure the SQLite connection itself fails, but free-disk comes from the + // filesystem — the snapshot must survive with bytesUsed degraded instead of rejecting. + const executeAsyncSpy = jest.spyOn(SQLiteProvider.store!, 'executeAsync').mockRejectedValue(new Error('[NativeNitroSQLiteException][SqlExecutionError] disk I/O error')); + + await expect(SQLiteProvider.getDatabaseSize()).resolves.toEqual({bytesUsed: -1, bytesRemaining: 12345}); + + executeAsyncSpy.mockRestore(); + }); }); });