fix(toolkit): keep source DB intact when DbMove copy fails - #6946
halibobo1205 wants to merge 1 commit into
Conversation
Prevent the prior flow from deleting source databases after logging and ignoring per-file copy errors. Preserve every source until all configured databases copy successfully. Roll back partial destinations, handle recursive database contents, and return a non-zero status on failure. Finalize symlinks only after the copy phase completes, report completion once, and cover rollback and direct retry behavior.
cc85dc5 to
a4045d5
Compare
|
|
||
| private boolean replaceSourceWithLink(Property p) { | ||
| try { | ||
| if (!FileUtils.deleteDir(p.original.toFile())) { |
There was a problem hiding this comment.
Great fix for the copy-failure case. One adjacent gap worth flagging while you're in here: FileUtils.deleteDir (plugins utils/FileUtils.java:73-84) still recurses with java.io.File, and isDirectory() follows symlinks — so a directory symlink gets traversed and its target's contents are deleted.
This composes with this PR: after db mv, the original path is a symlink to the moved DB (DbMove.java:176). If a later db lite merge deletes the archive dbs under that original path (DbLite.backupArchiveDbs calls FileUtils.deleteDir per db), it wipes the already-migrated DB on the new disk — two legitimate operations, chained data loss, no attacker needed.
Suggested fix: replace the recursion with Files.walkFileTree + NOFOLLOW_LINKS (delete the link itself, never follow). The copy phase already uses NOFOLLOW_LINKS (:154/:198), so this would make the delete side consistent. The same pattern also exists in common/.../FileUtil.java:114-126 and the toolkit copy in tron-docker's tools/toolkit — happy to split that into a separate PR if you prefer.
2 participants
Closes #6940
What does this PR do?
Makes
db mva fail-safe two-phase migration: validate first (exit 2, nothing written), then copy every database (recursive, link-aware, fail-fast), and only after all copies succeed replace sources with symlinks. Any copy failure keeps all sources, removes the created destinations, and exits 1 — the same command can simply be re-run. Finalization failures keep the complete copy and print recovery instructions.move db done./ exit 0 only on full success. Also documents the stop-the-node precondition inplugins/README.md.Why are these changes required?
The previous implementation ignored per-file copy errors, then deleted the source and symlinked it to an incomplete destination while still reporting success — any I/O fault (disk full, permissions, bad sector) meant silent, unrecoverable data loss (#6940).
This PR has been tested by:
DbMoveTestgrown to 15 cases (failure/rollback/retry, recursive copy, symlink and permission faults, exit codes).Follow up
None.
Extra details
Design writeup with sequence diagram and non-goals: see #6940.