zclllyybb commented on issue #65632: URL: https://github.com/apache/doris/issues/65632#issuecomment-4977891885
Breakwater-GitHub-Analysis-Slot: slot_f37e8522720b This content is generated by AI for reference only. I checked this against current `upstream/master` (`82e90efae653496e10a5e20100215bcda25a2132`). The report looks valid: FE backup staging directories under `Config.tmp_dir/backup` can remain after they are no longer useful. Code-level observations: - `BackupHandler.BACKUP_ROOT_DIR` is `Paths.get(Config.tmp_dir, "backup")`. - `BackupJob.saveMetaInfo()` creates a per-job local directory like `backup/repo__<repo_id>/<label>__<create_time>/`, writes the backup meta file and job-info file there, and records `localMetaInfoFilePath` / `localJobInfoFilePath`. - For a remote repository, `BackupJob.uploadMetaAndJobInfoFile()` uploads those two local files, marks the job `FINISHED`, and logs the final job state, but there is no follow-up deletion of `localJobDirPath`. So a successful remote backup can leave its FE-side staging directory behind. - The user-cancel path already attempts to delete `localJobDirPath` on the leader, but replay cancellation is different: `BackupJob.replayCancel()` is a no-op. Since `BackupJob.replayRun()` calls `saveMetaInfo(true)` when replaying `SAVE_META`, another FE can create the same staging files during journal replay and then keep them after a later cancel/finish replay. - Local snapshots intentionally need to keep the files while the snapshot is valid, but expiration and eviction do not currently remove the directory. `BackupJob.getSnapshot()` returns an expired snapshot after `createTime + timeoutMs`; `BackupHandler.removeSnapshot()` only removes the entry from the in-memory `localSnapshots` map when a local snapshot is evicted from the retained job deque. Suggested fix direction: 1. Add one idempotent helper for deleting a backup job's local staging directory and use it consistently. 2. For remote backups, run that cleanup after the finish state has been durably logged. If FE crashes between durable finish and deletion, the orphan cleaner below should still catch it. 3. For local snapshots, keep the directory while the snapshot is valid, then delete it when the snapshot expires or is evicted from `localSnapshots`. 4. Add a periodic/startup orphan scan under `tmp_dir/backup/repo__*` that builds a keep-set from active jobs and valid local snapshots, then deletes only directories that are not referenced, are expired, or belong to finished/cancelled remote jobs. This is important for replay-created directories, FE restarts, and interrupted cleanup. 5. Add FE tests around these state transitions: successful remote backup finish, cancel replay after `SAVE_META`, local snapshot expiration, and local snapshot eviction by `max_backup_restore_job_num_per_db`. Missing information for impact sizing, not for confirming the cleanup gap: a listing of `tmp_dir/backup` on each FE, the backup labels/timestamps, and the relevant FE logs around backup finish/cancel/restart would help quantify how much stale data has accumulated and whether any directories are still referenced by active jobs. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
