zclllyybb commented on issue #65798: URL: https://github.com/apache/doris/issues/65798#issuecomment-5017982195
Breakwater-GitHub-Analysis-Slot: slot_100ef8e9b801 This content is generated by AI for reference only. I checked the current `upstream/master` and `upstream/branch-3.1` FE backup paths. The report looks valid and is a real local-snapshot correctness issue, not only a local path naming issue. Code evidence: - `BackupHandler.localSnapshots` is a single FE-memory `Map<String, BackupJob>`, and both `master` and `branch-3.1` key it only by the bare label. - A completed local backup calls `addSnapshot(label, this)` from the `BackupJob` finish path, so a later completed local backup with the same label can replace the earlier map entry even if it belongs to another database. - `FrontendServiceImpl.getSnapshotImpl()` requires `request.db` and uses it for the privilege check, but then resolves the local snapshot with only `request.label_name` via `Env.getCurrentEnv().getBackupHandler().getSnapshot(label)`. - The removal/eviction path is also label-only: when old local backup jobs are removed from a per-database history list, `BackupHandler.removeSnapshot(label)` removes the global map entry for that bare label. So the expected failure mode is plausible from the current code: two databases can have local backups with the same label at SQL scope, but the FE local snapshot cache cannot distinguish them. This can cause wrong snapshot retrieval, overwrite of the previous local snapshot entry, or removal of another database's snapshot entry. Suggested fix direction: - Change the local snapshot key from `label` to a database-scoped key, preferably `(dbId, label)` because `BackupJob` already stores `dbId` and `request.db` can be resolved to a database id before lookup. - Update `addSnapshot`, `removeSnapshot`, and `getSnapshot` together, including the finish path, image/replay reconstruction path, and history eviction path. Keeping only `getSnapshot` scoped is not enough if insert/remove remain label-only. - Keep this scoped to the local in-memory snapshot map; remote repository snapshot naming and timestamped local staging directories are separate mechanisms. - Add coverage for creating local backups with the same label in `db1` and `db2`, then retrieving both snapshots separately. A second useful case is eviction/removal of an old local backup in one database while another database has the same label. No additional logs are needed to justify the bug from code inspection. A focused reproduction or regression test would still be useful to lock down the intended SQL/RPC behavior before merging the fix. -- 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]
