Classify SQLite disk-pressure errors and stop retrying them - #816
Classify SQLite disk-pressure errors and stop retrying them#816elirangoshen wants to merge 2 commits into
Conversation
Full device disk surfaces as "disk I/O error" (SQLITE_IOERR, shm sizing on open), "unable to open database file" (SQLITE_CANTOPEN), or "cannot rollback - no transaction is active" (failed ROLLBACK masking the original write error). All were UNKNOWN: 5 futile retries per operation plus per-operation alerts. New DISK_PRESSURE class drops the write (cache stays authoritative), skips retries and eviction (neither frees OS-level space), and logs one throttled alert + storage quota snapshot per burst — the snapshot carries free-disk bytes so telemetry can confirm disk pressure as the root cause. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| // Full-disk failures around the database files: SQLITE_IOERR (cannot size the -shm file on reopen, | ||
| // fails reads too), SQLITE_CANTOPEN (cannot create it), and a failed ROLLBACK masking the original | ||
| // write error. None can succeed until the OS frees space. | ||
| if (message.includes('disk i/o error') || message.includes('unable to open database file') || message.includes('cannot rollback - no transaction is active')) { |
There was a problem hiding this comment.
Are you sure that cannot rollback - no transaction is active is 100% related to disk pressure?
There was a problem hiding this comment.
its not 100 percent so I edit the comments to reflect it better
| /** 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', |
There was a problem hiding this comment.
How this is different from CAPACITY?
There was a problem hiding this comment.
their both related to disk full but CAPACITY usually means the onyx keys is full and evict some keys can fix while in DISK_PRESSURE its when os file system is full so there is no way to fix it unless free space.
| 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); | ||
| } |
There was a problem hiding this comment.
The goal is not to send so many logs to VL, right?
This is why we introduced CircuitBreaker.
There was a problem hiding this comment.
They solve different problems. The circuit breaker stops the evict → retry loop for CAPACITY errors. DISK_PRESSURE has no loop to stop — we never retry or evict, the write is just dropped. The only thing left to limit is log volume, and the 60s throttle does that. Putting it through the breaker wouldn't work anyway: the breaker's recovery probe is an eviction, which can't fix a full disk.
2 participants
Details
Fixes the app-side symptom of Expensify/App#87869 — iOS bursts of
NitroSQLiteError: [NativeNitroSQLiteException][SqlExecutionError] disk I/O error(Sentry APP-19Q).Root cause (reproduced locally on an iOS simulator): when the device disk is full at database (re)open, SQLite fails to ftruncate the
OnyxDB-shmWAL-index file to 32KB (errno 28ENOSPC→SQLITE_IOERR_SHMSIZE), after which every read and write returnsdisk I/O erroruntil space frees — then it recovers on its own. Two related shapes of the same condition:unable to open database file(SQLITE_CANTOPEN) — the-shmfile cannot be created at allcannot rollback - no transaction is active— a batch-write failure whose original error was masked by the ROLLBACK itself failing on the same full diskBefore this PR these errors fell in the
UNKNOWNstorage error class: 5 blind retries per operation (futile — the disk is still full) plus a per-operation alert, amplifying the very log storm being reported (~70x amplification seen in prod logs).This PR:
DISK_PRESSUREstorage error class (lib/storage/errors.ts)classifySQLiteErrorroutes the three messages above to it (lib/storage/providers/classifySQLiteError.ts)retryOperationdrops the write (the in-memory cache stays authoritative), skips retries and eviction (neither can free OS-level disk space), and logs a single throttled alert (60s window) plus one storage quota snapshot per burst (lib/OnyxUtils.ts)getFreeDiskStorage()bytes, giving prod telemetry the free-disk data to confirm disk pressure as the root causeRelated Issues
Expensify/App#87869
Linked E/App PR
TBD — companion Expensify/App pin PR not opened yet
Automated Tests
Added to
tests/unit/onyxUtilsTest.ts:it.eachover the three disk-pressure error strings (disk I/O error,unable to open database file,cannot rollback - no transaction is active) asserting the operation is not retriedFull unit suite passes (569 tests).
Manual Tests
Regression smoke (no user-facing change in this PR — it only alters error classification/logging):
Author Checklist
### Related Issuessection above### Linked E/App PRsection above, and verified this change against it (E/App CI passed and manual testing completed)TestssectiontoggleReportand notonIconClick)myBool && <MyComponent />.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)Avataris modified, I verified thatAvataris working as expected in all cases)mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari