fix(backup): attach reconnecting restore subscriptions to the in-flight job instead of re-running the restore - #4968
Open
Siumauricio wants to merge 1 commit into
Open
fix(backup): attach reconnecting restore subscriptions to the in-flight job instead of re-running the restore#4968Siumauricio wants to merge 1 commit into
Siumauricio wants to merge 1 commit into
Conversation
…ht job instead of re-running the restore
Comment on lines
+592
to
+599
| const jobKey = [ | ||
| input.backupType, | ||
| input.databaseId, | ||
| input.databaseType, | ||
| input.databaseName, | ||
| input.backupFile, | ||
| input.destinationId, | ||
| ].join("|"); |
Contributor
There was a problem hiding this comment.
Because databaseName and backupFile accept pipe characters, distinct restore requests can produce the same delimiter-joined key; the second request then skips its own restore and replays the first job's logs.
Suggested change
| const jobKey = [ | |
| input.backupType, | |
| input.databaseId, | |
| input.databaseType, | |
| input.databaseName, | |
| input.backupFile, | |
| input.destinationId, | |
| ].join("|"); | |
| const jobKey = JSON.stringify([ | |
| input.backupType, | |
| input.databaseId, | |
| input.databaseType, | |
| input.databaseName, | |
| input.backupFile, | |
| input.destinationId, | |
| ]); |
Knowledge Base Used: Backups and Schedules
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3388
restoreBackupWithLogsstarts the restore inside the subscription handler, and the websocket client (createWSClientwithretryDelayMs: 3000) re-subscribes automatically whenever the connection drops. For large backups the socket sits idle for minutes (proxies like Cloudflare cut idle connections), so every reconnect re-invoked the handler and launched another fullpg_restore/mariadbrun — the concurrent duplicate restores reported in the issue.Restores are now tracked in an in-flight job map keyed by backup type, service, database name, backup file and destination. The first subscription starts the restore; any re-subscription with the same key attaches to the running job and replays its log buffer instead of starting a new process. Jobs are evicted 5s after completion so a genuine re-run later starts fresh.
Greptile Summary
The PR prevents reconnecting WebSocket subscriptions from launching duplicate database restores by retaining in-flight jobs and replaying their logs.
Confidence Score: 4/5
The PR appears safe to merge, with a non-blocking key-serialization edge case that can conflate distinct restore requests containing pipe characters.
The reconnect deduplication works in the supported single-process server architecture, but the unescaped delimiter-based key is ambiguous for two unrestricted input fields.
Files Needing Attention: apps/dokploy/server/api/routers/backup.ts
Reviews (1): Last reviewed commit: "fix(backup): attach reconnecting restore..." | Re-trigger Greptile
Context used: