From d3d7afde6cc8eca1d29f0e257bc423081a99c72e Mon Sep 17 00:00:00 2001 From: Manu Date: Wed, 27 May 2026 19:24:29 +0200 Subject: [PATCH 1/8] SMB: ensure test file cleanup on failures Always attempt to remove the SMB connection test file and local temp file in a finally block. Move client and tmpPath initialization outside the try so deleteFile/unlink are always accessible, remove the fragile remoteFileCreated flag, and unconditionally call client.deleteFile(destination) with errors ignored. Add unit tests that verify cleanup runs when sendFile throws and that deleteFile is retried when it initially fails. Update changelog to document the bug fix and new tests. --- docs/changelog.md | 19 ++++++++++++++ src/lib/adapters/storage/smb.ts | 34 ++++++++++++------------- tests/unit/adapters/storage/smb.test.ts | 24 +++++++++++++++++ 3 files changed, 60 insertions(+), 17 deletions(-) diff --git a/docs/changelog.md b/docs/changelog.md index 4d025fed..602bfd18 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -2,6 +2,25 @@ All notable changes to DBackup are documented here. +## vNEXT +*Release: In Progress* + +### ๐Ÿ› Bug Fixes + +- **SMB**: Fixed orphaned `.connection-test-*` files occasionally surviving on SMB shares. The previous fix used a `remoteFileCreated` flag, but if `sendFile` threw after the file was already written on the server (e.g. a network hiccup before the final ACK arrived), the flag was never set and no cleanup was attempted. The `finally` block now always calls `deleteFile` unconditionally, covering that edge case. + +### ๐Ÿงช Tests + +- **SMB**: Added two `test()` unit tests: verifies `deleteFile` is called in the `finally` block even when `sendFile` throws, and verifies the cleanup retry when the first explicit `deleteFile` fails with a server-side error. + +### ๐Ÿณ Docker + +- **Image**: `skyfay/dbackup:vNEXT` +- **Also tagged as**: `latest`, `vNEXT` +- **CI Image**: `skyfay/dbackup:ci` +- **Platforms**: linux/amd64, linux/arm64 + + ## v2.4.0 - Database Explorer Browser, Drill-down Data Viewer, and Bug Fixes *Released: May 25, 2026* diff --git a/src/lib/adapters/storage/smb.ts b/src/lib/adapters/storage/smb.ts index 9c89755d..6f97c606 100644 --- a/src/lib/adapters/storage/smb.ts +++ b/src/lib/adapters/storage/smb.ts @@ -201,10 +201,12 @@ export const SMBAdapter: StorageAdapter = { async test(config: SMBConfig): Promise<{ success: boolean; message: string }> { const testFileName = `.connection-test-${Date.now()}`; - try { - const client = createClient(config); - const destination = resolvePath(config, testFileName); + const tmpPath = path.join(os.tmpdir(), testFileName); + // Initialized outside try/catch so they are always accessible in the finally block. + const client = createClient(config); + const destination = resolvePath(config, testFileName); + try { // Ensure pathPrefix directory exists if set if (config.pathPrefix) { try { @@ -215,27 +217,25 @@ export const SMBAdapter: StorageAdapter = { } // Create a temp file to upload - const tmpPath = path.join(os.tmpdir(), testFileName); await fs.writeFile(tmpPath, "Connection Test"); - let remoteFileCreated = false; - try { - // 1. Write Test - await client.sendFile(tmpPath, destination); - remoteFileCreated = true; + // 1. Write Test + await client.sendFile(tmpPath, destination); - // 2. Delete Test - await client.deleteFile(destination); - remoteFileCreated = false; + // 2. Delete Test + await client.deleteFile(destination); - return { success: true, message: "Connection successful (Write/Delete verified)" }; - } finally { - if (remoteFileCreated) await client.deleteFile(destination).catch(() => {}); - await fs.unlink(tmpPath).catch(() => {}); - } + return { success: true, message: "Connection successful (Write/Delete verified)" }; } catch (error: unknown) { const message = error instanceof Error ? error.message : String(error); return { success: false, message: `SMB Connection failed: ${message}` }; + } finally { + // Always attempt remote cleanup. This guards against the edge case where + // sendFile partially succeeded (file exists on the SMB share) but then + // threw before the explicit deleteFile call was reached. "File not found" + // errors from the redundant delete in the success path are silently ignored. + await client.deleteFile(destination).catch(() => {}); + await fs.unlink(tmpPath).catch(() => {}); } }, }; diff --git a/tests/unit/adapters/storage/smb.test.ts b/tests/unit/adapters/storage/smb.test.ts index 2b8c72a1..7bf9fb75 100644 --- a/tests/unit/adapters/storage/smb.test.ts +++ b/tests/unit/adapters/storage/smb.test.ts @@ -271,6 +271,30 @@ describe("SMBAdapter", () => { expect(result.success).toBe(false); expect(result.message).toContain("ECONNREFUSED"); }); + + it("attempts remote cleanup when sendFile throws (partial upload guard)", async () => { + // Simulate: file was created on the SMB share but sendFile threw before + // the client received the final ACK (e.g. network hiccup mid-transfer). + mockSendFile.mockRejectedValue(new Error("connection reset")); + + await SMBAdapter.test!(config); + + // deleteFile must be called in the finally block even though sendFile threw, + // so orphaned .connection-test-* files cannot accumulate on the share. + expect(mockDeleteFile).toHaveBeenCalled(); + }); + + it("does not leave orphaned files when deleteFile throws after successful upload", async () => { + // sendFile succeeds, deleteFile fails on first attempt (server-side error), + // but the finally block retries it unconditionally. + mockDeleteFile.mockRejectedValueOnce(new Error("NT_STATUS_SHARING_VIOLATION")); + + const result = await SMBAdapter.test!(config); + + expect(result.success).toBe(false); + // finally block must have retried deleteFile after the explicit call failed + expect(mockDeleteFile).toHaveBeenCalledTimes(2); + }); }); // ===== resolvePath without pathPrefix ===== From 152093876d5d86a11d054ed7d56849ec1e2e26d5 Mon Sep 17 00:00:00 2001 From: Manu Date: Wed, 27 May 2026 19:30:29 +0200 Subject: [PATCH 2/8] MSSQL: prevent INT overflow in size queries Cast MSSQL size values to BIGINT before multiplying to avoid "Arithmetic overflow error converting expression to data type int" for large databases. Updated queries in src/lib/adapters/database/mssql/browser.ts (CAST a.total_pages AS BIGINT) and src/lib/adapters/database/mssql/connection.ts (CAST mf.size AS BIGINT) and added a changelog entry in docs/changelog.md describing the fix. --- docs/changelog.md | 1 + src/lib/adapters/database/mssql/browser.ts | 2 +- src/lib/adapters/database/mssql/connection.ts | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/changelog.md b/docs/changelog.md index 602bfd18..703fa1b6 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -7,6 +7,7 @@ All notable changes to DBackup are documented here. ### ๐Ÿ› Bug Fixes +- **MSSQL**: Fixed "Arithmetic overflow error converting expression to data type int" crash in the Database Explorer and Restore database list for large databases. The size queries now cast page counts to `BIGINT` before multiplying by 8192, preventing `INT` overflow on databases larger than ~2 GB. - **SMB**: Fixed orphaned `.connection-test-*` files occasionally surviving on SMB shares. The previous fix used a `remoteFileCreated` flag, but if `sendFile` threw after the file was already written on the server (e.g. a network hiccup before the final ACK arrived), the flag was never set and no cleanup was attempted. The `finally` block now always calls `deleteFile` unconditionally, covering that edge case. ### ๐Ÿงช Tests diff --git a/src/lib/adapters/database/mssql/browser.ts b/src/lib/adapters/database/mssql/browser.ts index e47e649b..6dfd1b9d 100644 --- a/src/lib/adapters/database/mssql/browser.ts +++ b/src/lib/adapters/database/mssql/browser.ts @@ -22,7 +22,7 @@ export async function getTables(config: MSSQLConfig, database: string): Promise< t.TABLE_NAME AS name, t.TABLE_TYPE AS table_type, COALESCE(SUM(p.rows), 0) AS row_count, - COALESCE(SUM(a.total_pages) * 8 * 1024, 0) AS size_bytes + COALESCE(SUM(CAST(a.total_pages AS BIGINT)) * 8 * 1024, 0) AS size_bytes FROM [${dbId}].INFORMATION_SCHEMA.TABLES t LEFT JOIN [${dbId}].sys.tables st ON st.name = t.TABLE_NAME LEFT JOIN [${dbId}].sys.indexes i ON i.object_id = st.object_id AND i.type <= 1 diff --git a/src/lib/adapters/database/mssql/connection.ts b/src/lib/adapters/database/mssql/connection.ts index 65f119b2..b6a5d111 100644 --- a/src/lib/adapters/database/mssql/connection.ts +++ b/src/lib/adapters/database/mssql/connection.ts @@ -158,7 +158,7 @@ export async function getDatabasesWithStats(config: MSSQLConfig): Promise 4 From 453f0eea7e7d7a8858e070619f0de9512c1a792b Mon Sep 17 00:00:00 2001 From: Manu Date: Wed, 27 May 2026 19:40:55 +0200 Subject: [PATCH 3/8] Rename retention abbreviation from GVS to GFS Standardize the Grandfather-Father-Son retention abbreviation to GFS across the codebase and docs. Updated documentation, README, UI strings, and unit test descriptions to use "GFS" instead of "GVS", and adjusted function/documentation references accordingly. Added a Prisma migration to rename the built-in retention template from "Smart GVS (7/4/12/2)" to "Smart GFS (7/4/12/2)" (conditional update by id/name). This change is purely a terminology correction for clarity and consistency. --- .github/copilot-instructions.md | 2 +- README.md | 2 +- docs/changelog.md | 4 ++++ docs/developer-guide/advanced/retention.md | 12 ++++++------ docs/developer-guide/architecture.md | 6 +++--- docs/developer-guide/core/services.md | 2 +- docs/index.md | 2 +- docs/roadmap.md | 2 +- docs/user-guide/destinations/index.md | 2 +- docs/user-guide/first-steps.md | 2 +- docs/user-guide/getting-started.md | 2 +- docs/user-guide/jobs/retention.md | 6 +++--- .../migration.sql | 7 +++++++ .../settings/templates/retention-policy-list.tsx | 4 ++-- tests/unit/services/retention-service.test.ts | 4 ++-- 15 files changed, 35 insertions(+), 24 deletions(-) create mode 100644 prisma/migrations/20260527000001_fix_gfs_retention_name/migration.sql diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 1806a80e..73ff02db 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -24,7 +24,7 @@ All business logic lives here. Server Actions must delegate to Services. ``` job-service.ts โ†’ CRUD for backup jobs backup-service.ts โ†’ Triggers runJob() -retention-service.ts โ†’ GVS (Grandfather-Father-Son) algorithm +retention-service.ts โ†’ GFS (Grandfather-Father-Son) algorithm restore-service.ts โ†’ Restore orchestration ``` diff --git a/README.md b/README.md index 006e7bb8..30f4376f 100644 --- a/README.md +++ b/README.md @@ -97,7 +97,7 @@ Whether you're running a single MySQL database or managing multiple PostgreSQL, ### โฐ Scheduling & Retention - **Cron-based Scheduling** - Flexible job scheduling with a visual Schedule Picker (Simple Mode + Cron Mode) -- **GVS Retention Policies** - Grandfather-Father-Son rotation with per-destination retention settings +- **GFS Retention Policies** - Grandfather-Father-Son rotation with per-destination retention settings - **Automated Config Backups** - Self-backup of the entire DBackup configuration to any storage adapter ### ๐Ÿ‘ฅ Access Control & Security diff --git a/docs/changelog.md b/docs/changelog.md index 703fa1b6..6e47cdd7 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -10,6 +10,10 @@ All notable changes to DBackup are documented here. - **MSSQL**: Fixed "Arithmetic overflow error converting expression to data type int" crash in the Database Explorer and Restore database list for large databases. The size queries now cast page counts to `BIGINT` before multiplying by 8192, preventing `INT` overflow on databases larger than ~2 GB. - **SMB**: Fixed orphaned `.connection-test-*` files occasionally surviving on SMB shares. The previous fix used a `remoteFileCreated` flag, but if `sendFile` threw after the file was already written on the server (e.g. a network hiccup before the final ACK arrived), the flag was never set and no cleanup was attempted. The `finally` block now always calls `deleteFile` unconditionally, covering that edge case. +### ๐ŸŽจ Improvements + +- **Terminology**: Corrected the retention algorithm abbreviation from "GVS" to the standard "GFS" (Grandfather-Father-Son) across all documentation, UI labels, test descriptions, and the built-in retention template name via a new database migration. + ### ๐Ÿงช Tests - **SMB**: Added two `test()` unit tests: verifies `deleteFile` is called in the `finally` block even when `sendFile` throws, and verifies the cleanup retry when the first explicit `deleteFile` fails with a server-side error. diff --git a/docs/developer-guide/advanced/retention.md b/docs/developer-guide/advanced/retention.md index 06e432a9..e1caf58d 100644 --- a/docs/developer-guide/advanced/retention.md +++ b/docs/developer-guide/advanced/retention.md @@ -1,6 +1,6 @@ # Retention System -The Retention System automatically manages backup storage by implementing smart rotation policies based on the Grandfather-Father-Son (GVS) algorithm. +The Retention System automatically manages backup storage by implementing smart rotation policies based on the Grandfather-Father-Son (GFS) algorithm. ## Overview @@ -10,11 +10,11 @@ DBackup supports three retention modes: | :--- | :--- | | **None** | Keep all backups (no deletion) | | **Simple** | Keep the last N backups | -| **Smart (GVS)** | Grandfather-Father-Son strategy | +| **Smart (GFS)** | Grandfather-Father-Son strategy | -## Grandfather-Father-Son (GVS) +## Grandfather-Father-Son (GFS) -The GVS algorithm keeps backups at decreasing frequencies as they age: +The GFS algorithm keeps backups at decreasing frequencies as they age: ``` Today โ†โ”€โ”€โ”€โ”€ Daily โ”€โ”€โ”€โ”€โ†’ Weekly โ”€โ”€โ”€โ”€โ†’ Monthly โ”€โ”€โ”€โ”€โ†’ Yearly @@ -108,7 +108,7 @@ export const RetentionService = { keep = sorted.slice(0, config.simple!.keepCount); break; case "SMART": - keep = this.applyGVS(sorted, config.smart!); + keep = this.applyGFS(sorted, config.smart!); break; } @@ -122,7 +122,7 @@ export const RetentionService = { }; }, - applyGVS(files: FileInfo[], config: SmartConfig): FileInfo[] { + applyGFS(files: FileInfo[], config: SmartConfig): FileInfo[] { const keep = new Set(); // Daily buckets diff --git a/docs/developer-guide/architecture.md b/docs/developer-guide/architecture.md index ad1ab420..73d95f77 100644 --- a/docs/developer-guide/architecture.md +++ b/docs/developer-guide/architecture.md @@ -128,7 +128,7 @@ src/services/ โ”œโ”€โ”€ job-service.ts # Job CRUD โ”œโ”€โ”€ backup-service.ts # Backup triggering โ”œโ”€โ”€ restore-service.ts # Restore orchestration -โ”œโ”€โ”€ retention-service.ts # GVS algorithm +โ”œโ”€โ”€ retention-service.ts # GFS algorithm โ”œโ”€โ”€ encryption-service.ts # Key management โ”œโ”€โ”€ user-service.ts # User management โ””โ”€โ”€ oidc-provider-service.ts @@ -205,7 +205,7 @@ Executes backups through discrete steps: โ”‚ Completion โ”‚โ—€โ”€โ”€โ”‚ Retention โ”‚โ—€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”˜ โ”‚ โ”‚ โ”‚ โ”‚ โ”‚ โ€ข Cleanup โ”‚ โ”‚ โ€ข Apply โ”‚ -โ”‚ โ€ข Notify โ”‚ โ”‚ GVS โ”‚ +โ”‚ โ€ข Notify โ”‚ โ”‚ GFS โ”‚ โ”‚ โ€ข Finalize โ”‚ โ”‚ โ€ข Delete โ”‚ โ””โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”˜ โ””โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”˜ ``` @@ -286,7 +286,7 @@ Runner Pipeline: โ”‚ โ””โ”€โ”€ Cleanup, notify, update status โ”‚ โ””โ”€โ”€ stepRetention() - โ””โ”€โ”€ Apply GVS, delete old backups + โ””โ”€โ”€ Apply GFS, delete old backups ``` ## Security Architecture diff --git a/docs/developer-guide/core/services.md b/docs/developer-guide/core/services.md index b3c22bbe..b76a0509 100644 --- a/docs/developer-guide/core/services.md +++ b/docs/developer-guide/core/services.md @@ -9,7 +9,7 @@ src/services/ โ”œโ”€โ”€ job-service.ts # CRUD for backup jobs โ”œโ”€โ”€ backup-service.ts # Trigger backups โ”œโ”€โ”€ restore-service.ts # Restore orchestration -โ”œโ”€โ”€ retention-service.ts # GVS algorithm +โ”œโ”€โ”€ retention-service.ts # GFS algorithm โ”œโ”€โ”€ encryption-service.ts # Encryption profiles โ”œโ”€โ”€ integrity-service.ts # SHA-256 checksum verification โ”œโ”€โ”€ notification-log-service.ts # Notification log recording & queries diff --git a/docs/index.md b/docs/index.md index 148efdc5..170731a3 100644 --- a/docs/index.md +++ b/docs/index.md @@ -37,7 +37,7 @@ features: details: Each backup job can target multiple storage destinations simultaneously for redundancy or off-site copies. - icon: ๐Ÿ“… title: Scheduling & Retention - details: Cron-based job scheduling with GVS (Grandfather-Father-Son) retention policies for automatic rotation. + details: Cron-based job scheduling with GFS (Grandfather-Father-Son) retention policies for automatic rotation. - icon: ๐Ÿ”” title: Notifications details: 9+ notification adapters including Discord, Slack, Teams, Telegram, Gotify, ntfy, Webhook, SMS, and Email (SMTP). diff --git a/docs/roadmap.md b/docs/roadmap.md index 2555a4b8..18635333 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -169,7 +169,7 @@ For a full list of completed features, see the [Changelog](/changelog). - โœ… GZIP and Brotli compression - โœ… S3, SFTP, Local, WebDAV, SMB, FTP/FTPS storage adapters - โœ… Discord and Email notifications -- โœ… Cron-based scheduling with GVS retention +- โœ… Cron-based scheduling with GFS retention - โœ… RBAC permission system - โœ… SSO/OIDC authentication (Authentik, PocketID, Generic) - โœ… TOTP and Passkey 2FA diff --git a/docs/user-guide/destinations/index.md b/docs/user-guide/destinations/index.md index 8fc9349f..a5531874 100644 --- a/docs/user-guide/destinations/index.md +++ b/docs/user-guide/destinations/index.md @@ -57,7 +57,7 @@ The `.meta.json` file stores compression, encryption metadata (IV, auth tag, pro Destinations work with retention policies to automatically clean up old backups: - **Simple**: Keep last N backups -- **Smart (GVS)**: Grandfather-Father-Son rotation +- **Smart (GFS)**: Grandfather-Father-Son rotation See [Retention Policies](/user-guide/jobs/retention) for details. - [FTP / FTPS](/user-guide/destinations/ftp) diff --git a/docs/user-guide/first-steps.md b/docs/user-guide/first-steps.md index 42ad397a..35952ede 100644 --- a/docs/user-guide/first-steps.md +++ b/docs/user-guide/first-steps.md @@ -95,7 +95,7 @@ In the **General** tab, use the Schedule picker: In the **Destinations** tab, expand a destination row and configure its retention policy: - **Simple**: Keep last N backups -- **Smart (GVS)**: Grandfather-Father-Son rotation +- **Smart (GFS)**: Grandfather-Father-Son rotation ## Step 4: Run Your First Backup diff --git a/docs/user-guide/getting-started.md b/docs/user-guide/getting-started.md index 67ce30ed..9776bd11 100644 --- a/docs/user-guide/getting-started.md +++ b/docs/user-guide/getting-started.md @@ -13,7 +13,7 @@ DBackup is a self-hosted web application for automating database backups. It sup - **Multi-Destination Jobs**: A single job can upload to multiple storage destinations simultaneously - **Backup Encryption**: AES-256-GCM encryption with an Encryption Vault, key rotation, and offline Recovery Kits - **Compression**: Built-in GZIP and Brotli compression -- **Scheduling & Retention**: Cron-based scheduling with GVS (Grandfather-Father-Son) retention policies +- **Scheduling & Retention**: Cron-based scheduling with GFS (Grandfather-Father-Son) retention policies - **Notifications**: 9+ adapters including Discord, Slack, Teams, Telegram, Email, and more - **Storage Monitoring**: Per-destination alerts for usage spikes, storage limit warnings, and missing backups - **Restore**: Browse backup history, verify checksums, and restore directly to a database including database remapping diff --git a/docs/user-guide/jobs/retention.md b/docs/user-guide/jobs/retention.md index f7e3f109..87b12450 100644 --- a/docs/user-guide/jobs/retention.md +++ b/docs/user-guide/jobs/retention.md @@ -10,7 +10,7 @@ Retention policies prevent unlimited storage growth by automatically deleting ol | :--- | :--- | :--- | | **None** | Keep all backups | Manual management | | **Simple** | Keep last N backups | Fixed rotation | -| **Smart (GVS)** | Grandfather-Father-Son | Long-term archival | +| **Smart (GFS)** | Grandfather-Father-Son | Long-term archival | ## Per-Destination Retention @@ -55,7 +55,7 @@ With `Keep Count: 5`: - Frequent backups with short retention - Simple rotation needs -## Smart Retention (GVS) +## Smart Retention (GFS) Grandfather-Father-Son is an intelligent retention strategy that keeps: - Recent backups (daily) @@ -259,7 +259,7 @@ With compression (70% reduction): ### Wrong Backups Deleted -The GVS algorithm keeps the **oldest** backup in each time bucket. This is intentional: +The GFS algorithm keeps the **oldest** backup in each time bucket. This is intentional: - Weekly: Keeps backup from start of week - Monthly: Keeps backup from start of month diff --git a/prisma/migrations/20260527000001_fix_gfs_retention_name/migration.sql b/prisma/migrations/20260527000001_fix_gfs_retention_name/migration.sql new file mode 100644 index 00000000..a266cb35 --- /dev/null +++ b/prisma/migrations/20260527000001_fix_gfs_retention_name/migration.sql @@ -0,0 +1,7 @@ +-- Fix: Rename built-in retention template from "Smart GVS" to "Smart GFS" (correct Grandfather-Father-Son abbreviation) +UPDATE "RetentionPolicy" +SET "name" = 'Smart GFS (7/4/12/2)', + "description" = 'Grandfather-Father-Son: keep 7 daily, 4 weekly, 12 monthly, 2 yearly backups.', + "updatedAt" = CURRENT_TIMESTAMP +WHERE "id" = 'retention-gvs-default' + AND "name" = 'Smart GVS (7/4/12/2)'; diff --git a/src/components/settings/templates/retention-policy-list.tsx b/src/components/settings/templates/retention-policy-list.tsx index c11841e4..290920c9 100644 --- a/src/components/settings/templates/retention-policy-list.tsx +++ b/src/components/settings/templates/retention-policy-list.tsx @@ -108,7 +108,7 @@ export function RetentionPolicyList() { return `Simple - keep ${parsed.simple?.keepCount ?? "?"} backups`; if (parsed.mode === "SMART") { const s = parsed.smart; - return `Smart GVS (${s?.daily ?? 0}/${s?.weekly ?? 0}/${s?.monthly ?? 0}/${s?.yearly ?? 0})`; + return `Smart GFS (${s?.daily ?? 0}/${s?.weekly ?? 0}/${s?.monthly ?? 0}/${s?.yearly ?? 0})`; } } catch { // ignore @@ -358,7 +358,7 @@ export function RetentionPolicyDialog({ id="rp-name" value={name} onChange={(e) => setName(e.target.value)} - placeholder="e.g. Smart GVS Production" + placeholder="e.g. Smart GFS Production" />
diff --git a/tests/unit/services/retention-service.test.ts b/tests/unit/services/retention-service.test.ts index b9f83d8d..56745813 100644 --- a/tests/unit/services/retention-service.test.ts +++ b/tests/unit/services/retention-service.test.ts @@ -96,11 +96,11 @@ describe('RetentionService', () => { }); }); - describe('Smart Policy (GVS)', () => { + describe('Smart Policy (GFS)', () => { it('should correctly handle daily, weekly, monthly, and yearly retention', () => { const now = new Date('2026-01-24T12:00:00Z'); - // Construct specific GVS scenarios rather than a loop + // Construct specific GFS scenarios rather than a loop const dates: Date[] = [ // DAILY: Days 0-6 (7 backups) now, From fc58f4fae8751933b631919b624dc25cb5993fc3 Mon Sep 17 00:00:00 2001 From: Manu Date: Wed, 27 May 2026 20:16:16 +0200 Subject: [PATCH 4/8] Fix SMART/GFS retention tier overlap (#101) Ensure SMART/GFS tiers do not collapse onto the same newest backup by refactoring retention logic into non-overlapping tiers. RetentionService.applySmartPolicy now uses a generic applyTier helper that picks unique UTC bucket representatives per tier (daily, weekly, monthly, yearly) using date-fns-tz formatInTimeZone. Added retentionPolicyName and retentionPolicySource to DestinationContext and populated them in stepInitialize; stepRetention now logs the applied policy with template/source details. Updated unit tests to cover the regression and logging, and updated changelog entries. --- docs/changelog.md | 3 + src/lib/runner/steps/01-initialize.ts | 9 ++ src/lib/runner/steps/05-retention.ts | 17 ++- src/lib/runner/types.ts | 2 + src/services/backup/retention-service.ts | 104 ++++++++++-------- tests/unit/runner/steps/05-retention.test.ts | 21 ++++ tests/unit/services/retention-service.test.ts | 48 ++++---- 7 files changed, 127 insertions(+), 77 deletions(-) diff --git a/docs/changelog.md b/docs/changelog.md index 6e47cdd7..70170390 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -9,14 +9,17 @@ All notable changes to DBackup are documented here. - **MSSQL**: Fixed "Arithmetic overflow error converting expression to data type int" crash in the Database Explorer and Restore database list for large databases. The size queries now cast page counts to `BIGINT` before multiplying by 8192, preventing `INT` overflow on databases larger than ~2 GB. - **SMB**: Fixed orphaned `.connection-test-*` files occasionally surviving on SMB shares. The previous fix used a `remoteFileCreated` flag, but if `sendFile` threw after the file was already written on the server (e.g. a network hiccup before the final ACK arrived), the flag was never set and no cleanup was attempted. The `finally` block now always calls `deleteFile` unconditionally, covering that edge case. +- **retention**: Fixed SMART/GFS retention tier overlap that could collapse Daily, Weekly, and Monthly selection onto the same newest backup and delete too aggressively. Weekly and larger tiers now retain additional older period representatives instead of reusing already selected buckets. ([#101](https://github.com/Skyfay/DBackup/issues/101)) ### ๐ŸŽจ Improvements - **Terminology**: Corrected the retention algorithm abbreviation from "GVS" to the standard "GFS" (Grandfather-Father-Son) across all documentation, UI labels, test descriptions, and the built-in retention template name via a new database migration. +- **history**: Improved retention execution logs to include the applied retention template name, including whether the policy came from an explicitly assigned template or the system default template. ### ๐Ÿงช Tests - **SMB**: Added two `test()` unit tests: verifies `deleteFile` is called in the `finally` block even when `sendFile` throws, and verifies the cleanup retry when the first explicit `deleteFile` fails with a server-side error. +- **retention**: Added SMART/GFS regression coverage for non-overlapping tier selection and runner log coverage for template-name visibility in retention history. ([#101](https://github.com/Skyfay/DBackup/issues/101)) ### ๐Ÿณ Docker diff --git a/src/lib/runner/steps/01-initialize.ts b/src/lib/runner/steps/01-initialize.ts index e88f0eda..e2b24a54 100644 --- a/src/lib/runner/steps/01-initialize.ts +++ b/src/lib/runner/steps/01-initialize.ts @@ -67,20 +67,27 @@ export async function stepInitialize(ctx: RunnerContext) { } let retention: RetentionConfiguration = { mode: 'NONE' }; + let retentionPolicyName: string | undefined; + let retentionPolicySource: DestinationContext['retentionPolicySource'] = 'none'; try { if (dest.retentionPolicyId) { // Policy template takes priority over the legacy per-destination retention JSON const policy = await prisma.retentionPolicy.findUnique({ where: { id: dest.retentionPolicyId } }); if (policy?.config) { retention = JSON.parse(policy.config as string); + retentionPolicyName = policy.name; + retentionPolicySource = 'template'; } } else if (dest.retention && dest.retention !== '{}') { retention = JSON.parse(dest.retention); + retentionPolicySource = 'legacy'; } else { // No per-destination policy and no legacy config - fall back to the system default retention policy const defaultPolicy = await prisma.retentionPolicy.findFirst({ where: { isDefault: true } }); if (defaultPolicy?.config) { retention = JSON.parse(defaultPolicy.config as string); + retentionPolicyName = defaultPolicy.name; + retentionPolicySource = 'default'; } } } catch { @@ -93,6 +100,8 @@ export async function stepInitialize(ctx: RunnerContext) { adapter, config: await resolveAdapterConfig(dest.config) as any, retention, + retentionPolicyName, + retentionPolicySource, priority: dest.priority, adapterId: dest.config.adapterId, }; diff --git a/src/lib/runner/steps/05-retention.ts b/src/lib/runner/steps/05-retention.ts index 47dfd07f..399ebf5f 100644 --- a/src/lib/runner/steps/05-retention.ts +++ b/src/lib/runner/steps/05-retention.ts @@ -45,7 +45,22 @@ async function applyRetentionForDestination(ctx: RunnerContext, dest: Destinatio return 0; } - ctx.log(`${destLabel} Retention: Applying policy ${policy.mode}...`); + const policyDetails = (() => { + if (dest.retentionPolicyName) { + if (dest.retentionPolicySource === 'default') { + return `${policy.mode} (default template: ${dest.retentionPolicyName})`; + } + return `${policy.mode} (template: ${dest.retentionPolicyName})`; + } + + if (dest.retentionPolicySource === 'legacy') { + return `${policy.mode} (legacy inline policy)`; + } + + return policy.mode; + })(); + + ctx.log(`${destLabel} Retention: Applying policy ${policyDetails}...`); if (!dest.adapter.list) { ctx.log(`${destLabel} Retention warning: Storage adapter does not support listing files. Skipped.`); diff --git a/src/lib/runner/types.ts b/src/lib/runner/types.ts index d7aeaebc..966be163 100644 --- a/src/lib/runner/types.ts +++ b/src/lib/runner/types.ts @@ -19,6 +19,8 @@ export interface DestinationContext { adapter: StorageAdapter; config: Record; // decrypted adapter config retention: RetentionConfiguration; + retentionPolicyName?: string; + retentionPolicySource?: 'template' | 'default' | 'legacy' | 'none'; priority: number; adapterId: string; uploadResult?: { diff --git a/src/services/backup/retention-service.ts b/src/services/backup/retention-service.ts index 258bb8ab..b95c9b59 100644 --- a/src/services/backup/retention-service.ts +++ b/src/services/backup/retention-service.ts @@ -1,6 +1,6 @@ import { FileInfo } from '@/lib/core/interfaces'; import { RetentionConfiguration } from '@/lib/core/retention'; -import { format, getISOWeek, getYear } from 'date-fns'; +import { formatInTimeZone } from 'date-fns-tz'; type FileWithReasons = { file: FileInfo; @@ -56,57 +56,67 @@ export class RetentionService { private static applySmartPolicy(files: FileWithReasons[], policy: NonNullable) { const { daily, weekly, monthly, yearly } = policy; - // Track used slots - const usedDays = new Set(); - const usedWeeks = new Set(); - const usedMonths = new Set(); - const usedYears = new Set(); + // SMART/GFS is applied as non-overlapping tiers. + // Daily picks newest unique days first. + // Weekly/Monthly/Yearly then pick additional representatives from older buckets. + this.applyTier( + files, + daily, + (date) => formatInTimeZone(date, 'UTC', 'yyyy-MM-dd'), + 'Daily' + ); + + this.applyTier( + files, + weekly, + (date) => formatInTimeZone(date, 'UTC', "RRRR-'W'II"), + 'Weekly' + ); + + this.applyTier( + files, + monthly, + (date) => formatInTimeZone(date, 'UTC', 'yyyy-MM'), + 'Monthly' + ); + + this.applyTier( + files, + yearly, + (date) => formatInTimeZone(date, 'UTC', 'yyyy'), + 'Yearly' + ); + } + + private static applyTier( + files: FileWithReasons[], + limit: number, + getBucketKey: (date: Date) => string, + reasonPrefix: string + ) { + if (limit <= 0) return; + + const usedBuckets = new Set(); + // Existing keeps from earlier tiers reserve their bucket in this tier. for (const entry of files) { - const date = entry.file.lastModified; - - // Keys based on local time or UTC? Ideally UTC strictly, but date-fns objects work well. - // Using standard formats - const dayKey = format(date, 'yyyy-MM-dd'); - const weekKey = `${getYear(date)}-W${getISOWeek(date)}`; - const monthKey = format(date, 'yyyy-MM'); - const yearKey = format(date, 'yyyy'); - - // 1. Daily Check - if (usedDays.size < daily) { - if (!usedDays.has(dayKey)) { - entry.keep = true; - entry.reasons.push(`Daily (${dayKey})`); - usedDays.add(dayKey); - } - } + if (!entry.keep) continue; + usedBuckets.add(getBucketKey(entry.file.lastModified)); + } - // 2. Weekly Check - if (usedWeeks.size < weekly) { - if (!usedWeeks.has(weekKey)) { - entry.keep = true; - entry.reasons.push(`Weekly (${weekKey})`); - usedWeeks.add(weekKey); - } - } + let keptInTier = 0; + for (const entry of files) { + if (entry.keep) continue; - // 3. Monthly Check - if (usedMonths.size < monthly) { - if (!usedMonths.has(monthKey)) { - entry.keep = true; - entry.reasons.push(`Monthly (${monthKey})`); - usedMonths.add(monthKey); - } - } + const bucketKey = getBucketKey(entry.file.lastModified); + if (usedBuckets.has(bucketKey)) continue; - // 4. Yearly Check - if (usedYears.size < yearly) { - if (!usedYears.has(yearKey)) { - entry.keep = true; - entry.reasons.push(`Yearly (${yearKey})`); - usedYears.add(yearKey); - } - } + entry.keep = true; + entry.reasons.push(`${reasonPrefix} (${bucketKey})`); + usedBuckets.add(bucketKey); + keptInTier++; + + if (keptInTier >= limit) break; } } } diff --git a/tests/unit/runner/steps/05-retention.test.ts b/tests/unit/runner/steps/05-retention.test.ts index ce059156..dcae5946 100644 --- a/tests/unit/runner/steps/05-retention.test.ts +++ b/tests/unit/runner/steps/05-retention.test.ts @@ -151,6 +151,27 @@ describe('stepRetention', () => { expect(ctx.log).toHaveBeenCalledWith(expect.stringContaining('Keeping 1, Deleting 1')); }); + it('logs the selected retention template name when applying policy', async () => { + const { RetentionService } = await import('@/services/backup/retention-service'); + (RetentionService.calculateRetention as ReturnType).mockReturnValue({ + keep: [], + delete: [], + }); + + const dest = makeDestination({ + retention: { mode: 'SMART', smart: { daily: 1, weekly: 1, monthly: 1, yearly: 0 } } as any, + retentionPolicyName: 'Default GFS', + retentionPolicySource: 'template', + }); + const ctx = makeCtx({ destinations: [dest] }); + + await stepRetention(ctx); + + expect(ctx.log).toHaveBeenCalledWith( + expect.stringContaining('Retention: Applying policy SMART (template: Default GFS)...') + ); + }); + it('logs an error but does not throw when a delete fails', async () => { const { RetentionService } = await import('@/services/backup/retention-service'); (RetentionService.calculateRetention as ReturnType).mockReturnValue({ diff --git a/tests/unit/services/retention-service.test.ts b/tests/unit/services/retention-service.test.ts index 56745813..6921c06a 100644 --- a/tests/unit/services/retention-service.test.ts +++ b/tests/unit/services/retention-service.test.ts @@ -97,7 +97,7 @@ describe('RetentionService', () => { }); describe('Smart Policy (GFS)', () => { - it('should correctly handle daily, weekly, monthly, and yearly retention', () => { + it('keeps additional backups from older tiers instead of collapsing to the same newest file', () => { const now = new Date('2026-01-24T12:00:00Z'); // Construct specific GFS scenarios rather than a loop @@ -153,22 +153,9 @@ describe('RetentionService', () => { expect(result.keep.map(f => f.lastModified.getTime())).toContain(dates[i].getTime()); } - // 2. Verify Weekly (1-4 weeks back kept) - // Note: - // - Daily backups cover Jan 24 (W04) through Jan 18 (W03). - // - W04 slot is taken by Jan 24 (Day 0). - // - W03 slot is taken by Jan 18 (Day 6). - // - Capacity is 4. So we have room for W02 and W01. - - // subWeeks(now, 1) is Jan 17 (W03). Superseded by Jan 18 (Day 6) which is newer. - // subWeeks(now, 2) is Jan 10 (W02). Should be kept. - // subWeeks(now, 3) is Jan 3 (W01). Should be kept. - // subWeeks(now, 4) is Dec 27 (W52). Dropped because 4 slots (W04, W03, W02, W01) are full. - + // 2. Verify Weekly (older unique weeks are retained additionally) expect(result.keep.map(f => f.lastModified.getTime())).toContain(subWeeks(now, 2).getTime()); expect(result.keep.map(f => f.lastModified.getTime())).toContain(subWeeks(now, 3).getTime()); - - // Verify subWeeks(now, 1) is NOT explicitly kept (it was superseded) expect(result.keep.map(f => f.lastModified.getTime())).not.toContain(subWeeks(now, 1).getTime()); // 3. Verify Monthly @@ -182,23 +169,26 @@ describe('RetentionService', () => { expect(result.keep.map(f => f.lastModified.getTime())).toContain(subMonths(now, 6).getTime()); expect(result.keep.map(f => f.lastModified.getTime())).toContain(subMonths(now, 12).getTime()); - // 4. Verify Yearly - // Slots: - // 1. 2026 (Jan 24) - // 2. 2025 (subMonths 1 - Dec 24 is newest 2025 file) - // 3. 2024 (subMonths 14 - Nov 24 is newest 2024 file) + // 4. Verify the policy keeps multiple representatives across tiers + expect(result.keep.length).toBeGreaterThan(7); + }); - // subMonths(14) is Nov 2024. It takes the 2024 slot. - expect(result.keep.map(f => f.lastModified.getTime())).toContain(subMonths(now, 14).getTime()); + it('keeps weekly representatives from older weeks when daily slots are limited', () => { + const now = new Date('2026-05-27T00:06:00Z'); + const files = createMockFiles( + Array.from({ length: 14 }, (_, i) => subDays(now, i)) + ); - // subYears(2) is Jan 2024. It is OLDER than Nov 2024. So it is dropped for the Yearly slot. - // And Monthly slots (6) are full with 2025/2026 stuff. - const deletedTimes = result.delete.map(f => f.lastModified.getTime()); - expect(deletedTimes).toContain(subYears(now, 2).getTime()); + const policy: RetentionConfiguration = { + mode: 'SMART', + smart: { daily: 1, weekly: 1, monthly: 3, yearly: 0 } + }; - // 5. Verify Deletions (Noise) - // subYears(3) is 2023. Yearly capacity 3 (26, 25, 24). So 2023 is dropped. - expect(deletedTimes).toContain(subYears(now, 3).getTime()); + const result = RetentionService.calculateRetention(files, policy); + + // Daily keeps the newest day and weekly keeps an additional older week. + expect(result.keep).toHaveLength(2); + expect(result.keep.map(f => f.lastModified.getTime())).toContain(now.getTime()); }); it('should prioritize keeping the newest backup when intervals overlap', () => { From 4c64dde7f92e573b36276f27ea1c48561e33ee57 Mon Sep 17 00:00:00 2001 From: Manu Date: Wed, 27 May 2026 20:28:10 +0200 Subject: [PATCH 5/8] Fix file descriptor leaks in storage & tar-utils (#100) Close and destroy read streams to prevent FD leaks that left temporary .tar files holding disk blocks indefinitely. Updated tar-utils to keep createReadStream instances in variables and destroy them on error, and wrapped stream usage in FTP, S3, SFTP and OneDrive adapters with try/finally blocks that call fileStream.destroy(). Also updated changelog to document the fix (addresses #100). This ensures uploaded/processed temporary files are released promptly and avoids ghost disk usage. --- docs/changelog.md | 1 + src/lib/adapters/database/common/tar-utils.ts | 31 ++++++++++++--- src/lib/adapters/storage/ftp.ts | 7 +++- src/lib/adapters/storage/onedrive.ts | 38 ++++++++++--------- src/lib/adapters/storage/s3.ts | 9 +++-- src/lib/adapters/storage/sftp.ts | 23 ++++++----- 6 files changed, 73 insertions(+), 36 deletions(-) diff --git a/docs/changelog.md b/docs/changelog.md index 70170390..fe560745 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -10,6 +10,7 @@ All notable changes to DBackup are documented here. - **MSSQL**: Fixed "Arithmetic overflow error converting expression to data type int" crash in the Database Explorer and Restore database list for large databases. The size queries now cast page counts to `BIGINT` before multiplying by 8192, preventing `INT` overflow on databases larger than ~2 GB. - **SMB**: Fixed orphaned `.connection-test-*` files occasionally surviving on SMB shares. The previous fix used a `remoteFileCreated` flag, but if `sendFile` threw after the file was already written on the server (e.g. a network hiccup before the final ACK arrived), the flag was never set and no cleanup was attempted. The `finally` block now always calls `deleteFile` unconditionally, covering that edge case. - **retention**: Fixed SMART/GFS retention tier overlap that could collapse Daily, Weekly, and Monthly selection onto the same newest backup and delete too aggressively. Weekly and larger tiers now retain additional older period representatives instead of reusing already selected buckets. ([#101](https://github.com/Skyfay/DBackup/issues/101)) +- **S3 / SFTP / FTP / OneDrive**: Fixed file descriptor leak that caused deleted temporary `.tar` backup files to hold their disk blocks open indefinitely. After a backup upload completed, the `ReadStream` passed to the upload client was never explicitly closed. The kernel only frees the inode once all file descriptors are released, so the space was never reclaimed until the container restarted - leading to 100+ GB of ghost disk usage in production. Each affected adapter now calls `fileStream.destroy()` in a `finally` block. Additionally fixed the same FD-leak pattern in `tar-utils.ts` (`createMultiDbTar`, `extractMultiDbTar`, `extractSelectedDatabases`, `readTarManifest`): anonymous `createReadStream` calls are now stored as variables and destroyed in error handlers. ([#100](https://github.com/Skyfay/DBackup/issues/100)) ### ๐ŸŽจ Improvements diff --git a/src/lib/adapters/database/common/tar-utils.ts b/src/lib/adapters/database/common/tar-utils.ts index 0e22ac88..a892a7b5 100644 --- a/src/lib/adapters/database/common/tar-utils.ts +++ b/src/lib/adapters/database/common/tar-utils.ts @@ -89,7 +89,10 @@ export async function createMultiDbTar( // Stream file contents to tar entry const fileStream = createReadStream(file.path); await new Promise((resolve, reject) => { - fileStream.on("error", reject); + fileStream.on("error", (err) => { + fileStream.destroy(); + reject(err); + }); fileStream.on("end", () => { entry.end(); resolve(); @@ -179,13 +182,19 @@ export async function extractMultiDbTar( files: extractedFiles, }); }); + const readStream = createReadStream(sourcePath); /* v8 ignore start */ extractor.on("error", (err) => { + readStream.destroy(); + reject(err); + }); + readStream.on("error", (err) => { + extractor.destroy(err); reject(err); }); /* v8 ignore end */ - createReadStream(sourcePath).pipe(extractor); + readStream.pipe(extractor); }); } @@ -261,13 +270,19 @@ export async function extractSelectedDatabases( }); }); + const readStream = createReadStream(sourcePath); /* v8 ignore start */ extractor.on("error", (err) => { + readStream.destroy(); + reject(err); + }); + readStream.on("error", (err) => { + extractor.destroy(err); reject(err); }); /* v8 ignore end */ - createReadStream(sourcePath).pipe(extractor); + readStream.pipe(extractor); }); } @@ -320,6 +335,7 @@ export async function readTarManifest(filePath: string): Promise { const extractor = extract(); let manifestFound = false; + const readStream = createReadStream(filePath); extractor.on("entry", (header, stream, next) => { if (header.name === MANIFEST_FILENAME) { @@ -355,14 +371,19 @@ export async function readTarManifest(filePath: string): Promise { + readStream.destroy(); resolve(null); }); extractor.on("close", () => { - // Handle early close from destroy() + readStream.destroy(); }); - createReadStream(filePath).pipe(extractor); + readStream.on("error", () => { + extractor.destroy(); + resolve(null); + }); + readStream.pipe(extractor); }); } diff --git a/src/lib/adapters/storage/ftp.ts b/src/lib/adapters/storage/ftp.ts index 7f12bbb1..19fe37d9 100644 --- a/src/lib/adapters/storage/ftp.ts +++ b/src/lib/adapters/storage/ftp.ts @@ -94,7 +94,12 @@ export const FTPAdapter: StorageAdapter = { } }); - await client.uploadFrom(createReadStream(localPath), destination); + const fileStream = createReadStream(localPath); + try { + await client.uploadFrom(fileStream, destination); + } finally { + fileStream.destroy(); + } client.trackProgress(); diff --git a/src/lib/adapters/storage/onedrive.ts b/src/lib/adapters/storage/onedrive.ts index 98ffdbfa..e059e97c 100644 --- a/src/lib/adapters/storage/onedrive.ts +++ b/src/lib/adapters/storage/onedrive.ts @@ -233,24 +233,28 @@ export const OneDriveAdapter: StorageAdapter = { const fileStream = createReadStream(localPath, { highWaterMark: UPLOAD_CHUNK_SIZE }); let offset = 0; - for await (const chunk of fileStream) { - const buffer = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk); - const end = offset + buffer.length - 1; - - await fetch(uploadUrl, { - method: "PUT", - headers: { - "Content-Range": `bytes ${offset}-${end}/${fileSize}`, - "Content-Length": String(buffer.length), - }, - body: buffer, - }); - - offset += buffer.length; - - if (onProgress) { - onProgress(Math.min(99, Math.round((offset / fileSize) * 100))); + try { + for await (const chunk of fileStream) { + const buffer = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk); + const end = offset + buffer.length - 1; + + await fetch(uploadUrl, { + method: "PUT", + headers: { + "Content-Range": `bytes ${offset}-${end}/${fileSize}`, + "Content-Length": String(buffer.length), + }, + body: buffer, + }); + + offset += buffer.length; + + if (onProgress) { + onProgress(Math.min(99, Math.round((offset / fileSize) * 100))); + } } + } finally { + fileStream.destroy(); } } diff --git a/src/lib/adapters/storage/s3.ts b/src/lib/adapters/storage/s3.ts index 07381609..e4f0b88b 100644 --- a/src/lib/adapters/storage/s3.ts +++ b/src/lib/adapters/storage/s3.ts @@ -44,11 +44,10 @@ async function s3Upload(internalConfig: S3InternalConfig, localPath: string, rem const client = S3ClientFactory.create(internalConfig); const targetKey = S3ClientFactory.getTargetKey(internalConfig, remotePath); - try { - if (onLog) onLog(`Starting S3 upload to bucket: ${internalConfig.bucket}, key: ${targetKey}`, 'info', 'storage'); - - const fileStream = createReadStream(localPath); + if (onLog) onLog(`Starting S3 upload to bucket: ${internalConfig.bucket}, key: ${targetKey}`, 'info', 'storage'); + const fileStream = createReadStream(localPath); + try { const parallelUploads3 = new Upload({ client: client, params: { @@ -73,6 +72,8 @@ async function s3Upload(internalConfig: S3InternalConfig, localPath: string, rem log.error("S3 upload failed", { bucket: internalConfig.bucket, targetKey }, wrapError(error)); if (onLog && error instanceof Error) onLog(`S3 upload failed: ${error.message}`, 'error', 'storage', error instanceof Error ? error.stack : undefined); return false; + } finally { + fileStream.destroy(); } } diff --git a/src/lib/adapters/storage/sftp.ts b/src/lib/adapters/storage/sftp.ts index 51599610..de7cca18 100644 --- a/src/lib/adapters/storage/sftp.ts +++ b/src/lib/adapters/storage/sftp.ts @@ -82,16 +82,21 @@ export const SFTPAdapter: StorageAdapter = { // Note: ssh2-sftp-client default 'step' progress might not be granualr enough for small files, but works. // However, the signature is (total_transferred, chunk, total). - await sftp.put(createReadStream(localPath), destination, { - step: (total_transferred: any, _chunk: any, _total: any) => { - if (onProgress && totalSize > 0) { - // total param in callback is total bytes to transfer, which is known if we pass it, but put() with stream might not know it unless we checked. - // We use our known totalSize. - const percent = Math.round((total_transferred / totalSize) * 100); - onProgress(percent); + const fileStream = createReadStream(localPath); + try { + await sftp.put(fileStream, destination, { + step: (total_transferred: any, _chunk: any, _total: any) => { + if (onProgress && totalSize > 0) { + // total param in callback is total bytes to transfer, which is known if we pass it, but put() with stream might not know it unless we checked. + // We use our known totalSize. + const percent = Math.round((total_transferred / totalSize) * 100); + onProgress(percent); + } } - } - } as any); + } as any); + } finally { + fileStream.destroy(); + } if (onLog) onLog(`SFTP upload completed successfully`, 'info', 'storage'); return true; From 72ebfeb479cd0f703b91ec26e44cb258a0371f03 Mon Sep 17 00:00:00 2001 From: Manu Date: Wed, 27 May 2026 20:31:51 +0200 Subject: [PATCH 6/8] MSSQL SSH: reuse SFTP session to avoid channel limit Cache and reuse the SFTP session in the MSSQL SSH transfer adapter instead of opening a new SFTP channel on every operation. Add a sftpSession field, return the cached session from getSftp(), and explicitly end() the session in end() before disconnecting. Also update the changelog to document the bug fix (avoids exhausting SSH server channel limits such as OpenSSH's default 10, which caused "Channel open failure: open failed" and aborted backups when processing many databases). --- docs/changelog.md | 1 + src/lib/adapters/database/mssql/ssh-transfer.ts | 11 ++++++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/docs/changelog.md b/docs/changelog.md index fe560745..cb647d08 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -7,6 +7,7 @@ All notable changes to DBackup are documented here. ### ๐Ÿ› Bug Fixes +- **MSSQL SSH**: Fixed backup abort and missing remote cleanup when backing up more than ~10 databases via SSH file transfer. `getSftp()` was opening a new SFTP subsystem channel on every call (`download`, `upload`, `deleteRemote`, `exists`). SSH servers cap concurrent channels (OpenSSH default: 10), so channel 11+ was rejected mid-download with "Channel open failure: open failed". The SFTP session is now cached and reused for the lifetime of the SSH connection, and `end()` explicitly closes the session before disconnecting. - **MSSQL**: Fixed "Arithmetic overflow error converting expression to data type int" crash in the Database Explorer and Restore database list for large databases. The size queries now cast page counts to `BIGINT` before multiplying by 8192, preventing `INT` overflow on databases larger than ~2 GB. - **SMB**: Fixed orphaned `.connection-test-*` files occasionally surviving on SMB shares. The previous fix used a `remoteFileCreated` flag, but if `sendFile` threw after the file was already written on the server (e.g. a network hiccup before the final ACK arrived), the flag was never set and no cleanup was attempted. The `finally` block now always calls `deleteFile` unconditionally, covering that edge case. - **retention**: Fixed SMART/GFS retention tier overlap that could collapse Daily, Weekly, and Monthly selection onto the same newest backup and delete too aggressively. Weekly and larger tiers now retain additional older period representatives instead of reusing already selected buckets. ([#101](https://github.com/Skyfay/DBackup/issues/101)) diff --git a/src/lib/adapters/database/mssql/ssh-transfer.ts b/src/lib/adapters/database/mssql/ssh-transfer.ts index fde7ca0d..8364be59 100644 --- a/src/lib/adapters/database/mssql/ssh-transfer.ts +++ b/src/lib/adapters/database/mssql/ssh-transfer.ts @@ -20,6 +20,7 @@ const log = logger.child({ adapter: "mssql", module: "ssh-transfer" }); export class MssqlSshTransfer { private client: Client; private connected = false; + private sftpSession: SFTPWrapper | null = null; constructor() { this.client = new Client(); @@ -211,20 +212,28 @@ export class MssqlSshTransfer { */ public end(): void { if (this.connected) { + if (this.sftpSession) { + this.sftpSession.end(); + this.sftpSession = null; + } this.client.end(); this.connected = false; } } /** - * Get SFTP subsystem from the SSH connection + * Get SFTP subsystem from the SSH connection. + * Reuses an existing session - opening a new channel per call exhausts + * the SSH server's channel limit when backing up many databases. */ private getSftp(): Promise { + if (this.sftpSession) return Promise.resolve(this.sftpSession); return new Promise((resolve, reject) => { this.client.sftp((err, sftp) => { if (err) { reject(new Error(`Failed to initialize SFTP: ${err.message}`)); } else { + this.sftpSession = sftp; resolve(sftp); } }); From db7c28a1d6fd1577d0fefb10054066701e71afd1 Mon Sep 17 00:00:00 2001 From: Manu Date: Wed, 27 May 2026 20:37:28 +0200 Subject: [PATCH 7/8] Update changelog and add GFS migration guard Streamline docs/changelog.md entries into concise bullet lines, correct the retention term from GVS to GFS across changelog text, and update test and improvement notes for SMB, retention, storage, and history. Modify prisma/migrations/20260527000001_fix_gfs_retention_name/migration.sql to add a guard (NOT EXISTS) and comment so the built-in retention rename to 'Smart GFS (7/4/12/2)' skips if that target name already exists, preventing a unique-constraint failure. --- docs/changelog.md | 18 +++++++++--------- .../migration.sql | 7 ++++++- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/docs/changelog.md b/docs/changelog.md index cb647d08..89ffc9e9 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -7,21 +7,21 @@ All notable changes to DBackup are documented here. ### ๐Ÿ› Bug Fixes -- **MSSQL SSH**: Fixed backup abort and missing remote cleanup when backing up more than ~10 databases via SSH file transfer. `getSftp()` was opening a new SFTP subsystem channel on every call (`download`, `upload`, `deleteRemote`, `exists`). SSH servers cap concurrent channels (OpenSSH default: 10), so channel 11+ was rejected mid-download with "Channel open failure: open failed". The SFTP session is now cached and reused for the lifetime of the SSH connection, and `end()` explicitly closes the session before disconnecting. -- **MSSQL**: Fixed "Arithmetic overflow error converting expression to data type int" crash in the Database Explorer and Restore database list for large databases. The size queries now cast page counts to `BIGINT` before multiplying by 8192, preventing `INT` overflow on databases larger than ~2 GB. -- **SMB**: Fixed orphaned `.connection-test-*` files occasionally surviving on SMB shares. The previous fix used a `remoteFileCreated` flag, but if `sendFile` threw after the file was already written on the server (e.g. a network hiccup before the final ACK arrived), the flag was never set and no cleanup was attempted. The `finally` block now always calls `deleteFile` unconditionally, covering that edge case. -- **retention**: Fixed SMART/GFS retention tier overlap that could collapse Daily, Weekly, and Monthly selection onto the same newest backup and delete too aggressively. Weekly and larger tiers now retain additional older period representatives instead of reusing already selected buckets. ([#101](https://github.com/Skyfay/DBackup/issues/101)) -- **S3 / SFTP / FTP / OneDrive**: Fixed file descriptor leak that caused deleted temporary `.tar` backup files to hold their disk blocks open indefinitely. After a backup upload completed, the `ReadStream` passed to the upload client was never explicitly closed. The kernel only frees the inode once all file descriptors are released, so the space was never reclaimed until the container restarted - leading to 100+ GB of ghost disk usage in production. Each affected adapter now calls `fileStream.destroy()` in a `finally` block. Additionally fixed the same FD-leak pattern in `tar-utils.ts` (`createMultiDbTar`, `extractMultiDbTar`, `extractSelectedDatabases`, `readTarManifest`): anonymous `createReadStream` calls are now stored as variables and destroyed in error handlers. ([#100](https://github.com/Skyfay/DBackup/issues/100)) +- **mssql**: Fixed backup abort and missing remote cleanup when SSH-transferring more than ~10 databases due to SSH channel exhaustion - the SFTP session is now cached and reused instead of opening a new channel per operation. +- **mssql**: Fixed "Arithmetic overflow" crash in Database Explorer and Restore for databases larger than ~2 GB. +- **smb**: Fixed `.connection-test-*` probe files not being deleted when `sendFile` throws after the remote file was already created. +- **retention**: Fixed SMART/GFS tier overlap that incorrectly mapped multiple tiers to the same backup, causing over-aggressive deletion. ([#101](https://github.com/Skyfay/DBackup/issues/101)) +- **storage**: Fixed file descriptor leak causing deleted `.tar` temp files to hold disk blocks until container restart, affecting S3, SFTP, FTP, OneDrive, and tar-utils streams. ([#100](https://github.com/Skyfay/DBackup/issues/100)) ### ๐ŸŽจ Improvements -- **Terminology**: Corrected the retention algorithm abbreviation from "GVS" to the standard "GFS" (Grandfather-Father-Son) across all documentation, UI labels, test descriptions, and the built-in retention template name via a new database migration. -- **history**: Improved retention execution logs to include the applied retention template name, including whether the policy came from an explicitly assigned template or the system default template. +- **retention**: Corrected the retention algorithm abbreviation from "GVS" to "GFS" across UI, documentation, and the built-in retention template. +- **history**: Retention execution logs now include the applied retention template name. ### ๐Ÿงช Tests -- **SMB**: Added two `test()` unit tests: verifies `deleteFile` is called in the `finally` block even when `sendFile` throws, and verifies the cleanup retry when the first explicit `deleteFile` fails with a server-side error. -- **retention**: Added SMART/GFS regression coverage for non-overlapping tier selection and runner log coverage for template-name visibility in retention history. ([#101](https://github.com/Skyfay/DBackup/issues/101)) +- **smb**: Added unit tests for `finally`-block cleanup when `sendFile` throws and for cleanup retry when the delete itself fails. +- **retention**: Added regression tests for GFS non-overlapping tier selection and template-name visibility in retention history. ([#101](https://github.com/Skyfay/DBackup/issues/101)) ### ๐Ÿณ Docker diff --git a/prisma/migrations/20260527000001_fix_gfs_retention_name/migration.sql b/prisma/migrations/20260527000001_fix_gfs_retention_name/migration.sql index a266cb35..ec2e854c 100644 --- a/prisma/migrations/20260527000001_fix_gfs_retention_name/migration.sql +++ b/prisma/migrations/20260527000001_fix_gfs_retention_name/migration.sql @@ -1,7 +1,12 @@ -- Fix: Rename built-in retention template from "Smart GVS" to "Smart GFS" (correct Grandfather-Father-Son abbreviation) +-- Guard: skip if the target name already exists (user may have created a policy with that name) +-- to avoid a unique-constraint failure on the "name" column. UPDATE "RetentionPolicy" SET "name" = 'Smart GFS (7/4/12/2)', "description" = 'Grandfather-Father-Son: keep 7 daily, 4 weekly, 12 monthly, 2 yearly backups.', "updatedAt" = CURRENT_TIMESTAMP WHERE "id" = 'retention-gvs-default' - AND "name" = 'Smart GVS (7/4/12/2)'; + AND "name" = 'Smart GVS (7/4/12/2)' + AND NOT EXISTS ( + SELECT 1 FROM "RetentionPolicy" WHERE "name" = 'Smart GFS (7/4/12/2)' + ); From d90341b9ccf1e9766c608c269b071cb18be23101 Mon Sep 17 00:00:00 2001 From: Manu Date: Wed, 27 May 2026 21:06:14 +0200 Subject: [PATCH 8/8] Bump version to 2.4.1 and fix FTP/SFTP tests Bump project and docs versions to 2.4.1 (package.json, docs/package.json, api-docs/openapi.yaml, public/openapi.yaml) and update changelog/Docker tags for the release. Fix broken FTP/SFTP unit tests by adding destroy: vi.fn() to fs.createReadStream mocks so adapters that call fileStream.destroy() in finally blocks no longer throw TypeError. --- api-docs/openapi.yaml | 2 +- docs/changelog.md | 9 +++++---- docs/package.json | 2 +- package.json | 2 +- public/openapi.yaml | 2 +- tests/unit/adapters/storage/ftp.test.ts | 4 ++-- tests/unit/adapters/storage/sftp.test.ts | 4 ++-- 7 files changed, 13 insertions(+), 12 deletions(-) diff --git a/api-docs/openapi.yaml b/api-docs/openapi.yaml index fb4da384..fb10e9b5 100644 --- a/api-docs/openapi.yaml +++ b/api-docs/openapi.yaml @@ -1,7 +1,7 @@ openapi: 3.1.0 info: title: DBackup API - version: 2.4.0 + version: 2.4.1 description: | REST API for DBackup - a self-hosted database backup automation platform with encryption, compression, and smart retention. diff --git a/docs/changelog.md b/docs/changelog.md index 89ffc9e9..05deeba1 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -2,8 +2,8 @@ All notable changes to DBackup are documented here. -## vNEXT -*Release: In Progress* +## v2.4.1 - Multiple Bug Fixes across MSSQL, SMB, Retention, and Storage Adapters +*Released: May 27, 2026* ### ๐Ÿ› Bug Fixes @@ -22,11 +22,12 @@ All notable changes to DBackup are documented here. - **smb**: Added unit tests for `finally`-block cleanup when `sendFile` throws and for cleanup retry when the delete itself fails. - **retention**: Added regression tests for GFS non-overlapping tier selection and template-name visibility in retention history. ([#101](https://github.com/Skyfay/DBackup/issues/101)) +- **ftp/sftp**: Fixed broken upload unit tests by adding missing `destroy: vi.fn()` to the `createReadStream` mock - the adapter calls `fileStream.destroy()` in the `finally` block, which threw a TypeError without this mock method. ### ๐Ÿณ Docker -- **Image**: `skyfay/dbackup:vNEXT` -- **Also tagged as**: `latest`, `vNEXT` +- **Image**: `skyfay/dbackup:v2.4.1` +- **Also tagged as**: `latest`, `v2` - **CI Image**: `skyfay/dbackup:ci` - **Platforms**: linux/amd64, linux/arm64 diff --git a/docs/package.json b/docs/package.json index 88fc6bd1..0373c4a8 100644 --- a/docs/package.json +++ b/docs/package.json @@ -1,6 +1,6 @@ { "name": "dbackup-docs", - "version": "2.4.0", + "version": "2.4.1", "private": true, "scripts": { "dev": "vitepress dev", diff --git a/package.json b/package.json index 58ae7eb6..90b5bae0 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "dbackup", - "version": "2.4.0", + "version": "2.4.1", "private": true, "scripts": { "dev": "next dev", diff --git a/public/openapi.yaml b/public/openapi.yaml index a76ebcd5..98da3e9e 100644 --- a/public/openapi.yaml +++ b/public/openapi.yaml @@ -1,7 +1,7 @@ openapi: 3.1.0 info: title: DBackup API - version: 2.4.0 + version: 2.4.1 description: | REST API for DBackup - a self-hosted database backup automation platform with encryption, compression, and smart retention. diff --git a/tests/unit/adapters/storage/ftp.test.ts b/tests/unit/adapters/storage/ftp.test.ts index 084edfe5..4cf13149 100644 --- a/tests/unit/adapters/storage/ftp.test.ts +++ b/tests/unit/adapters/storage/ftp.test.ts @@ -33,10 +33,10 @@ vi.mock("basic-ftp", () => { }); vi.mock("fs", () => ({ - createReadStream: vi.fn(() => ({ pipe: vi.fn() })), + createReadStream: vi.fn(() => ({ pipe: vi.fn(), destroy: vi.fn() })), createWriteStream: vi.fn(() => ({ on: vi.fn(), end: vi.fn() })), default: { - createReadStream: vi.fn(() => ({ pipe: vi.fn() })), + createReadStream: vi.fn(() => ({ pipe: vi.fn(), destroy: vi.fn() })), createWriteStream: vi.fn(() => ({ on: vi.fn(), end: vi.fn() })), }, })); diff --git a/tests/unit/adapters/storage/sftp.test.ts b/tests/unit/adapters/storage/sftp.test.ts index c6d35444..1c91f3d9 100644 --- a/tests/unit/adapters/storage/sftp.test.ts +++ b/tests/unit/adapters/storage/sftp.test.ts @@ -33,9 +33,9 @@ vi.mock("ssh2-sftp-client", () => { }); vi.mock("fs", () => ({ - createReadStream: vi.fn(() => ({ pipe: vi.fn() })), + createReadStream: vi.fn(() => ({ pipe: vi.fn(), destroy: vi.fn() })), default: { - createReadStream: vi.fn(() => ({ pipe: vi.fn() })), + createReadStream: vi.fn(() => ({ pipe: vi.fn(), destroy: vi.fn() })), }, promises: { stat: mockFsStat,