Repository navigation
fix(db): persist the Keep-latest-backups retention setting (#3834) #3867
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -37,8 +37,44 @@ function parseNonNegativeInt(value: string | undefined, fallback: number) { | |||||||||||||||||||||||||
| return Number.isInteger(parsed) && parsed >= 0 ? parsed : fallback; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // #3834: the "Keep latest backups" UI value is persisted here so it survives a page | ||||||||||||||||||||||||||
| // refresh / the loadStorageHealth() refetch. A dedicated namespace avoids any | ||||||||||||||||||||||||||
| // cross-talk with the databaseSettings key_value store (which rewrites all of its own | ||||||||||||||||||||||||||
| // keys on every update). It is intentionally separate from the orphan | ||||||||||||||||||||||||||
| // `databaseSettings.backup.keepLastNBackups` (default 5) so existing installs keep the | ||||||||||||||||||||||||||
| // historical default of 20 until an operator explicitly changes it here. | ||||||||||||||||||||||||||
| const DB_BACKUP_SETTINGS_NAMESPACE = "dbBackup"; | ||||||||||||||||||||||||||
| const DB_BACKUP_MAX_FILES_KEY = "maxFiles"; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| function getStoredDbBackupMaxFiles(): number | undefined { | ||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||
| const db = getDbInstance(); | ||||||||||||||||||||||||||
| const row = db | ||||||||||||||||||||||||||
| .prepare("SELECT value FROM key_value WHERE namespace = ? AND key = ?") | ||||||||||||||||||||||||||
| .get(DB_BACKUP_SETTINGS_NAMESPACE, DB_BACKUP_MAX_FILES_KEY) as { value?: string } | undefined; | ||||||||||||||||||||||||||
| if (!row?.value) return undefined; | ||||||||||||||||||||||||||
| const parsed = Number.parseInt(JSON.parse(row.value), 10); | ||||||||||||||||||||||||||
| return Number.isInteger(parsed) && parsed > 0 ? parsed : undefined; | ||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||
| return undefined; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /** Persist the operator-chosen "keep latest backups" retention count (#3834). */ | ||||||||||||||||||||||||||
| export function setDbBackupMaxFiles(value: number): void { | ||||||||||||||||||||||||||
| if (!Number.isInteger(value) || value <= 0) return; | ||||||||||||||||||||||||||
| const db = getDbInstance(); | ||||||||||||||||||||||||||
| db.prepare( | ||||||||||||||||||||||||||
| "INSERT OR REPLACE INTO key_value (namespace, key, value) VALUES (?, ?, ?)" | ||||||||||||||||||||||||||
| ).run(DB_BACKUP_SETTINGS_NAMESPACE, DB_BACKUP_MAX_FILES_KEY, JSON.stringify(value)); | ||||||||||||||||||||||||||
|
Comment on lines
+66
to
+69
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Database write operations can fail due to various reasons (e.g., database locked, read-only database, or disk full). Since persisting this setting is a non-critical side effect of the backup cleanup process, wrapping the database write in a
Suggested change
|
||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| export function getDbBackupMaxFiles() { | ||||||||||||||||||||||||||
| return parsePositiveInt(process.env.DB_BACKUP_MAX_FILES, MAX_DB_BACKUPS); | ||||||||||||||||||||||||||
| // Precedence: DB_BACKUP_MAX_FILES env override (ops) → persisted UI value → default. | ||||||||||||||||||||||||||
| if (process.env.DB_BACKUP_MAX_FILES) { | ||||||||||||||||||||||||||
| return parsePositiveInt(process.env.DB_BACKUP_MAX_FILES, MAX_DB_BACKUPS); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| return getStoredDbBackupMaxFiles() ?? MAX_DB_BACKUPS; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| export function getDbBackupRetentionDays() { | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
JSON.parse(row.value)call already returns a parsed number since the value was stored usingJSON.stringify(value)wherevalueis a number. Wrapping it inNumber.parseInt(..., 10)is redundant and can be safely removed.