From a4045d5a783d53422047afafec330881d180c5db Mon Sep 17 00:00:00 2001 From: halibobo1205 Date: Thu, 11 Jun 2026 15:33:59 +0800 Subject: [PATCH 1/5] fix(toolkit): keep source DB intact when DbMove copy fails 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. --- plugins/README.md | 4 + .../main/java/common/org/tron/plugins/Db.java | 3 + .../java/common/org/tron/plugins/DbMove.java | 193 ++++++++++----- .../java/org/tron/plugins/DbMoveTest.java | 230 +++++++++++++++++- 4 files changed, 363 insertions(+), 67 deletions(-) diff --git a/plugins/README.md b/plugins/README.md index f14e070c01a..48229fef0d1 100644 --- a/plugins/README.md +++ b/plugins/README.md @@ -2,6 +2,10 @@ This package contains a set of tools for TRON, the followings are the documentation for each tool. +NOTE: All `db` tools operate directly on the database files. Before performing a database +operation (archive, convert, copy, lite, mv, root), you must stop the currently running +FullNode service. + ## DB Archive(Requires x86 + LevelDB) DB archive provides the ability to reformat the manifest according to the current `database`, parameters are compatible with the previous `ArchiveManifest`. diff --git a/plugins/src/main/java/common/org/tron/plugins/Db.java b/plugins/src/main/java/common/org/tron/plugins/Db.java index 84654dca934..217c3fbd301 100644 --- a/plugins/src/main/java/common/org/tron/plugins/Db.java +++ b/plugins/src/main/java/common/org/tron/plugins/Db.java @@ -6,6 +6,9 @@ mixinStandardHelpOptions = true, version = "db command 1.0", description = "An rich command set that provides high-level operations for dbs.", + header = "All `db` tools operate directly on the database files.\n" + + "Before performing a database operation,\n" + + "you must stop the currently running FullNode service.\n", subcommands = {CommandLine.HelpCommand.class, DbMove.class, DbArchive.class, diff --git a/plugins/src/main/java/common/org/tron/plugins/DbMove.java b/plugins/src/main/java/common/org/tron/plugins/DbMove.java index a5619d2d7ed..8b10c5fb3d4 100644 --- a/plugins/src/main/java/common/org/tron/plugins/DbMove.java +++ b/plugins/src/main/java/common/org/tron/plugins/DbMove.java @@ -4,17 +4,21 @@ import com.typesafe.config.ConfigFactory; import java.io.File; import java.io.IOException; +import java.io.UncheckedIOException; import java.nio.file.Files; +import java.nio.file.LinkOption; import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.StandardCopyOption; -import java.util.Arrays; +import java.nio.file.attribute.BasicFileAttributes; +import java.util.ArrayList; import java.util.HashSet; import java.util.List; -import java.util.Objects; import java.util.Set; import java.util.concurrent.Callable; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Collectors; +import java.util.stream.Stream; import lombok.extern.slf4j.Slf4j; import me.tongfei.progressbar.ProgressBar; import org.tron.plugins.utils.FileUtils; @@ -76,41 +80,30 @@ public Integer call() throws Exception { printNotExist(); return 0; } - List toBeMove = dbs.stream() - .map(c -> { - try { - return new Property(c.getString(NAME_CONFIG_KEY), - Paths.get(database.toString(), dbPath, c.getString(NAME_CONFIG_KEY)), - Paths.get(c.getString(PATH_CONFIG_KEY), dbPath, c.getString(NAME_CONFIG_KEY))); - } catch (IOException e) { - spec.commandLine().getErr().println(e); - } - return null; - }).filter(Objects::nonNull) - .filter(p -> !p.destination.equals(p.original)).collect(Collectors.toList()); - - if (toBeMove.isEmpty()) { - printNotExist(); - return 0; + List toBeMove = new ArrayList<>(); + for (Config c : dbs) { + try { + toBeMove.add(new Property(c.getString(NAME_CONFIG_KEY), + Paths.get(database.toString(), dbPath, c.getString(NAME_CONFIG_KEY)), + Paths.get(c.getString(PATH_CONFIG_KEY), dbPath, c.getString(NAME_CONFIG_KEY)))); + } catch (IOException e) { + spec.commandLine().getErr().println(e); + return 2; + } + } + boolean allCopied = ProgressBar.wrap(toBeMove.stream(), "copy task") + .allMatch(this::copy); + if (!allCopied) { + cleanupDestinations(toBeMove); + return 1; } - toBeMove = toBeMove.stream() - .filter(property -> { - if (property.destination.toFile().exists()) { - spec.commandLine().getOut().println(String.format("%s already exist,skip.", - property.destination)); - return false; - } else { - return true; - } - }).collect(Collectors.toList()); - if (toBeMove.isEmpty()) { - printNotExist(); - return 0; + boolean allMoved = ProgressBar.wrap(toBeMove.stream(), "link task") + .map(this::replaceSourceWithLink).reduce(Boolean.TRUE, Boolean::logicalAnd); + if (!allMoved) { + return 1; } - ProgressBar.wrap(toBeMove.stream(), "mv task").forEach(this::run); spec.commandLine().getOut().println("move db done."); - } else { printNotExist(); return 0; @@ -118,28 +111,112 @@ public Integer call() throws Exception { return 0; } - private void run(Property p) { - if (p.destination.toFile().mkdirs()) { - ProgressBar.wrap(Arrays.stream(Objects.requireNonNull(p.original.toFile().listFiles())) - .filter(File::isFile).map(File::getName).parallel(), p.name).forEach(file -> { - Path original = Paths.get(p.original.toString(), file); - Path destination = Paths.get(p.destination.toString(), file); - try { - Files.copy(original, destination, - StandardCopyOption.REPLACE_EXISTING); - } catch (IOException e) { - spec.commandLine().getErr().println(e); - } - }); + private boolean copy(Property p) { + if (!p.destination.toFile().mkdirs()) { + spec.commandLine().getErr().println(String.format("%s create failed.", p.destination)); + return false; + } + + AtomicBoolean hasError = new AtomicBoolean(false); + try (Stream files = Files.walk(p.original)) { + // Collect the tree before copying anything: a traversal failure must + // surface while no copy task is in flight, otherwise a task started + // before the failure could recreate the destination after the rollback + // has already deleted it, and the retry would then be rejected. + List sources = files.collect(Collectors.toList()); + ProgressBar.wrap(sources.parallelStream(), p.name).forEach(source -> { + if (hasError.get()) { + return; + } + try { + copyEntry(p, source); + } catch (IOException e) { + hasError.set(true); + spec.commandLine().getErr().println(e); + } + }); + } catch (IOException | UncheckedIOException e) { + hasError.set(true); + spec.commandLine().getErr().println(e); + } + + if (hasError.get()) { + spec.commandLine().getErr().println(String.format( + "%s copy to %s failed, source kept.", + p.original, p.destination)); + return false; + } + return true; + } + + private void copyEntry(Property p, Path source) throws IOException { + BasicFileAttributes attributes = Files.readAttributes( + source, BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS); + Path destination = p.destination.resolve(p.original.relativize(source)); + if (attributes.isDirectory()) { + Files.createDirectories(destination); + } else if (attributes.isRegularFile()) { + Files.createDirectories(destination.getParent()); + Files.copy(source, destination, StandardCopyOption.REPLACE_EXISTING); + } else { + throw new IOException(String.format( + "%s is neither a regular file nor a directory, can not be moved.", source)); + } + } + + private boolean replaceSourceWithLink(Property p) { + try { + if (!FileUtils.deleteDir(p.original.toFile())) { + spec.commandLine().getErr().println(String.format( + "%s delete failed and may be incomplete; the only complete copy is at %s, keep it.", + p.original, p.destination)); + printRecoveryHint(p); + return false; + } + Files.createSymbolicLink(p.original, p.destination); + return true; + } catch (IOException | RuntimeException x) { + spec.commandLine().getErr().println(x); + spec.commandLine().getErr().println(String.format( + "%s move failed; the complete copy is at %s, keep it.", + p.original, p.destination)); + printRecoveryHint(p); + return false; + } + } + + private void printRecoveryHint(Property p) { + spec.commandLine().getErr().println(String.format( + "To recover manually: remove %s if present, then create a symbolic link at %s" + + " pointing to %s.", + p.original, p.original, p.destination)); + } + + private void cleanupDestinations(List properties) { + boolean allCleaned = properties.stream().map(property -> { + File destination = property.destination.toFile(); + if (Files.notExists(destination.toPath(), LinkOption.NOFOLLOW_LINKS)) { + return true; + } try { - if (FileUtils.deleteDir(p.original.toFile())) { - Files.createSymbolicLink(p.original, p.destination); + if (FileUtils.deleteDir(destination)) { + return true; } - } catch (IOException | UnsupportedOperationException x) { - spec.commandLine().getErr().println(x); + } catch (RuntimeException e) { + spec.commandLine().getErr().println(e); } + spec.commandLine().getErr().println(String.format( + "%s cleanup failed; remove the leftover copy before retrying.", + property.destination)); + return false; + }).reduce(Boolean.TRUE, Boolean::logicalAnd); + + if (allCleaned) { + spec.commandLine().getErr().println( + "move db failed; all source databases were kept, please retry."); } else { - spec.commandLine().getErr().println(String.format("%s create failed.", p.destination)); + spec.commandLine().getErr().println( + "move db failed; all source databases were kept, but leftover copies remain."); } } @@ -167,7 +244,7 @@ public Property(String name, Path original, Path destination) throws IOException throw new IOException(original + " is symbolicLink!"); } this.destination = destination.toFile().getCanonicalFile().toPath(); - if (this.destination.toFile().exists()) { + if (!Files.notExists(this.destination, LinkOption.NOFOLLOW_LINKS)) { throw new IOException(this.destination + " already exist!"); } if (this.destination.equals(this.original)) { @@ -195,9 +272,6 @@ public Config convert(String value) throws Exception { if (dbs.isEmpty()) { throw notFind; } - String dbPath = config.hasPath(DB_DIRECTORY_CONFIG_KEY) - ? config.getString(DB_DIRECTORY_CONFIG_KEY) : DEFAULT_DB_DIRECTORY; - dbs = dbs.stream() .filter(c -> c.hasPath(NAME_CONFIG_KEY) && c.hasPath(PATH_CONFIG_KEY)) .collect(Collectors.toList()); @@ -207,13 +281,10 @@ public Config convert(String value) throws Exception { } Set toBeMove = new HashSet<>(); for (Config c : dbs) { - if (!toBeMove.add(new Property(c.getString(NAME_CONFIG_KEY), - Paths.get(database.toString(), dbPath, c.getString(NAME_CONFIG_KEY)), - Paths.get(c.getString(PATH_CONFIG_KEY), dbPath, - c.getString(NAME_CONFIG_KEY))).name)) { + String name = c.getString(NAME_CONFIG_KEY); + if (!toBeMove.add(name)) { throw new IllegalArgumentException( - "DB config has duplicate key:[" + c.getString(NAME_CONFIG_KEY) - + "],please check! "); + "DB config has duplicate key:[" + name + "],please check! "); } } } else { diff --git a/plugins/src/test/java/org/tron/plugins/DbMoveTest.java b/plugins/src/test/java/org/tron/plugins/DbMoveTest.java index ec4f0d545b0..26aeca21b87 100644 --- a/plugins/src/test/java/org/tron/plugins/DbMoveTest.java +++ b/plugins/src/test/java/org/tron/plugins/DbMoveTest.java @@ -2,11 +2,17 @@ import java.io.File; import java.io.IOException; +import java.io.PrintWriter; +import java.io.StringWriter; import java.net.URL; +import java.nio.file.Files; +import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Objects; import lombok.extern.slf4j.Slf4j; import org.junit.After; import org.junit.Assert; +import org.junit.Assume; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -61,6 +67,22 @@ private static String getConfig(String config) { return path == null ? null : path.getPath(); } + /** Create and initialize a RocksDB database folder. */ + private File newDatabase() throws IOException, RocksDBException { + File database = temporaryFolder.newFolder("database"); + init(DbTool.DbType.RocksDB, database.getPath()); + return database; + } + + private static String[] mvArgs(File database, String configPath) { + return new String[] {"db", "mv", "-d", database.getParent(), "-c", configPath}; + } + + /** Run {@code db mv} with a fresh CommandLine and return the exit code. */ + private static int mv(File database, String configPath) { + return new CommandLine(new Toolkit()).execute(mvArgs(database, configPath)); + } + @Test public void testMvForLevelDB() throws RocksDBException, IOException { File database = temporaryFolder.newFolder("database"); @@ -75,14 +97,210 @@ public void testMvForLevelDB() throws RocksDBException, IOException { @Test public void testMvForRocksDB() throws RocksDBException, IOException { - File database = temporaryFolder.newFolder("database"); - init(DbTool.DbType.RocksDB, Paths.get(database.getPath()).toString()); - String[] args = new String[] {"db", "mv", "-d", - database.getParent(), "-c", - getConfig("config.conf")}; + File database = newDatabase(); + Assert.assertEquals(0, mv(database, getConfig("config.conf"))); + Assert.assertEquals(2, mv(database, getConfig("config.conf"))); + } + + @Test + public void testSourceKeptWhenCopyFails() throws RocksDBException, IOException { + File database = newDatabase(); + File accountDir = Paths.get(database.getPath(), ACCOUNT).toFile(); + File marketDir = Paths.get(database.getPath(), DBUtils.MARKET_PAIR_PRICE_TO_ORDER).toFile(); + File victim = Objects.requireNonNull(accountDir.listFiles(File::isFile))[0]; + // Make one source file unreadable so its copy fails part-way through the move. + Assert.assertTrue(victim.setReadable(false, false)); + + String[] args = mvArgs(database, getConfig("config.conf")); CommandLine cli = new CommandLine(new Toolkit()); + StringWriter output = new StringWriter(); + cli.setOut(new PrintWriter(output)); + try { + // Skip when the platform ignores the read bit (e.g. running as root). + Assume.assumeFalse("file still readable (root?), cannot simulate copy failure", + victim.canRead()); + Assert.assertEquals(1, cli.execute(args)); + + // A failed copy must keep every source intact and roll back all destinations. + Assert.assertTrue("source dir must be kept on copy failure", accountDir.exists()); + Assert.assertFalse("source must not be replaced by a symlink", + Files.isSymbolicLink(accountDir.toPath())); + Assert.assertTrue("source file must still exist", victim.exists()); + Assert.assertTrue("other source dirs must not be moved after a copy failure", + marketDir.exists()); + Assert.assertFalse("other source dirs must not be replaced by symlinks", + Files.isSymbolicLink(marketDir.toPath())); + Assert.assertFalse("partial destination must be removed", + Paths.get(OUTPUT_DIRECTORY, "dest", "database", ACCOUNT).toFile().exists()); + Assert.assertFalse("failure must not be reported as success", + output.toString().contains("move db done.")); + } finally { + victim.setReadable(true, false); + } + + // Once the I/O problem is fixed, the unchanged command must be directly retryable. Assert.assertEquals(0, cli.execute(args)); - Assert.assertEquals(2, cli.execute(args)); + Assert.assertTrue(Files.isSymbolicLink(accountDir.toPath())); + Assert.assertTrue(Files.isSymbolicLink(marketDir.toPath())); + Assert.assertEquals("move db done." + System.lineSeparator(), output.toString()); + } + + @Test + public void testOptionOrderConfigFirst() throws RocksDBException, IOException { + File database = newDatabase(); + // '-c' parsed before '-d': path validation must still use the final + // database value, not the stale one visible at conversion time. + String[] args = new String[] {"db", "mv", "-c", + getConfig("config.conf"), "-d", + database.getParent()}; + CommandLine cli = new CommandLine(new Toolkit()); + Assert.assertEquals(0, cli.execute(args)); + Assert.assertTrue(Files.isSymbolicLink( + Paths.get(database.getPath(), ACCOUNT))); + Assert.assertTrue(Files.isSymbolicLink( + Paths.get(database.getPath(), DBUtils.MARKET_PAIR_PRICE_TO_ORDER))); + } + + @Test + public void testInTreeSymlinkRejected() throws RocksDBException, IOException { + File database = newDatabase(); + File accountDir = Paths.get(database.getPath(), ACCOUNT).toFile(); + File outside = temporaryFolder.newFolder("outside"); + File sentinel = new File(outside, "sentinel"); + Assert.assertTrue(sentinel.createNewFile()); + Files.createSymbolicLink( + Paths.get(accountDir.getPath(), "evil-link"), outside.toPath()); + + Assert.assertEquals(1, mv(database, getConfig("config.conf"))); + // The move must fail without touching the source or the symlink target. + Assert.assertTrue("source dir must be kept", accountDir.exists()); + Assert.assertFalse("source must not be replaced by a symlink", + Files.isSymbolicLink(accountDir.toPath())); + Assert.assertTrue("symlink target must never be touched", sentinel.exists()); + Assert.assertFalse("partial destination must be rolled back", + Paths.get(OUTPUT_DIRECTORY, "dest", "database", ACCOUNT).toFile().exists()); + } + + @Test + public void testNestedDirsAndFilesPreserved() throws RocksDBException, IOException { + File database = newDatabase(); + File emptySub = Paths.get(database.getPath(), ACCOUNT, "archive", "sub").toFile(); + Assert.assertTrue(emptySub.mkdirs()); + byte[] payload = {1, 2, 3}; + Files.write(Paths.get(database.getPath(), ACCOUNT, "archive", "keep.dat"), payload); + + Assert.assertEquals(0, mv(database, getConfig("config.conf"))); + Assert.assertTrue("empty nested dirs must be recreated at the destination", + Paths.get(OUTPUT_DIRECTORY, "dest", "database", ACCOUNT, "archive", "sub") + .toFile().isDirectory()); + Assert.assertArrayEquals("files inside sub-directories must be copied", + payload, Files.readAllBytes( + Paths.get(OUTPUT_DIRECTORY, "dest", "database", ACCOUNT, "archive", "keep.dat"))); + } + + @Test + public void testDestinationCreateFails() throws RocksDBException, IOException { + File database = newDatabase(); + File accountDir = Paths.get(database.getPath(), ACCOUNT).toFile(); + File destParent = Paths.get(OUTPUT_DIRECTORY, "dest", "database").toFile(); + Assert.assertTrue(destParent.mkdirs()); + Assert.assertTrue(destParent.setWritable(false, false)); + try { + // Skip when the platform ignores the write bit (e.g. running as root). + Assume.assumeFalse("dir still writable (root?), cannot simulate mkdirs failure", + destParent.canWrite()); + Assert.assertEquals(1, mv(database, getConfig("config.conf"))); + Assert.assertTrue("source dir must be kept", accountDir.exists()); + Assert.assertFalse("source must not be replaced by a symlink", + Files.isSymbolicLink(accountDir.toPath())); + } finally { + destParent.setWritable(true, false); + } + } + + @Test + public void testUnreadableSubdirFailsCopy() throws RocksDBException, IOException { + File database = newDatabase(); + File accountDir = Paths.get(database.getPath(), ACCOUNT).toFile(); + File subDir = new File(accountDir, "subdir"); + Assert.assertTrue(subDir.mkdir()); + Assert.assertTrue(new File(subDir, "data").createNewFile()); + Assert.assertTrue(subDir.setReadable(false, false)); + try { + // Skip when the platform ignores the read bit (e.g. running as root). + Assume.assumeFalse("subdir still readable (root?), cannot simulate traversal failure", + subDir.canRead()); + Assert.assertEquals(1, mv(database, getConfig("config.conf"))); + Assert.assertTrue("source dir must be kept on traversal failure", accountDir.exists()); + Assert.assertFalse("source must not be replaced by a symlink", + Files.isSymbolicLink(accountDir.toPath())); + Assert.assertFalse("partial destination must be rolled back", + Paths.get(OUTPUT_DIRECTORY, "dest", "database", ACCOUNT).toFile().exists()); + } finally { + subDir.setReadable(true, false); + } + + // The rollback must leave a directly retryable state: no copy task still in + // flight when the traversal failed may recreate the destination afterwards. + Assert.assertEquals(0, mv(database, getConfig("config.conf"))); + Assert.assertTrue(Files.isSymbolicLink(accountDir.toPath())); + Assert.assertTrue(Files.isSymbolicLink( + Paths.get(database.getPath(), DBUtils.MARKET_PAIR_PRICE_TO_ORDER))); + } + + @Test + public void testDanglingDestinationLinkRejected() throws RocksDBException, IOException { + File database = newDatabase(); + File destParent = Paths.get(OUTPUT_DIRECTORY, "dest", "database").toFile(); + Assert.assertTrue(destParent.mkdirs()); + // A dangling symlink occupies the destination: File.exists() reports it as + // absent, but mkdirs would fail on it forever. Validation must fail closed. + Path danglingLink = Paths.get(destParent.getPath(), ACCOUNT); + Files.createSymbolicLink(danglingLink, + Paths.get(destParent.getPath(), "no-such-target")); + + Assert.assertEquals(2, mv(database, getConfig("config.conf"))); + Assert.assertTrue("dangling link must be reported, not treated as absent", + Files.isSymbolicLink(danglingLink)); + Assert.assertFalse("nothing may be moved", + Files.isSymbolicLink(Paths.get(database.getPath(), ACCOUNT))); + } + + @Test + public void testRecoveryHintWhenSourceDeleteFails() throws RocksDBException, IOException { + File database = newDatabase(); + File accountDir = Paths.get(database.getPath(), ACCOUNT).toFile(); + Assert.assertTrue(accountDir.setWritable(false, false)); + + StringWriter err = new StringWriter(); + CommandLine cli = new CommandLine(new Toolkit()); + cli.setErr(new PrintWriter(err)); + try { + // Skip when the platform ignores the write bit (e.g. running as root). + Assume.assumeFalse("dir still writable (root?), cannot simulate delete failure", + accountDir.canWrite()); + Assert.assertEquals(1, cli.execute(mvArgs(database, getConfig("config.conf")))); + + // Copy succeeded but finalization failed: source kept, complete copy kept. + Assert.assertTrue("source dir must be kept", accountDir.exists()); + Assert.assertFalse("source must not be replaced by a symlink", + Files.isSymbolicLink(accountDir.toPath())); + File dest = Paths.get(OUTPUT_DIRECTORY, "dest", "database", ACCOUNT).toFile(); + Assert.assertTrue("complete copy must be kept for manual recovery", dest.exists()); + String expectedHint = String.format( + "To recover manually: remove %s if present, then create a symbolic link at %s" + + " pointing to %s.", + accountDir.getCanonicalFile().toPath(), + accountDir.getCanonicalFile().toPath(), + dest.getCanonicalFile().toPath()); + Assert.assertTrue("operator must get exact recovery instructions with real paths", + err.toString().contains(expectedHint)); + // Finalization continues for the remaining dbs. + Assert.assertTrue(Files.isSymbolicLink( + Paths.get(database.getPath(), DBUtils.MARKET_PAIR_PRICE_TO_ORDER))); + } finally { + accountDir.setWritable(true, false); + } } @Test From 9ec25843637ce2f8b634e2238b5fcac1482637ab Mon Sep 17 00:00:00 2001 From: halibobo1205 Date: Tue, 29 Sep 2026 13:05:08 +0800 Subject: [PATCH 2/5] fix(toolkit): reject db mv destinations inside a selected source --- .../java/common/org/tron/plugins/DbMove.java | 17 +++++++ .../java/org/tron/plugins/DbMoveTest.java | 47 +++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/plugins/src/main/java/common/org/tron/plugins/DbMove.java b/plugins/src/main/java/common/org/tron/plugins/DbMove.java index 8b10c5fb3d4..af6c6fd6e53 100644 --- a/plugins/src/main/java/common/org/tron/plugins/DbMove.java +++ b/plugins/src/main/java/common/org/tron/plugins/DbMove.java @@ -91,6 +91,9 @@ public Integer call() throws Exception { return 2; } } + if (hasNestedDestination(toBeMove)) { + return 2; + } boolean allCopied = ProgressBar.wrap(toBeMove.stream(), "copy task") .allMatch(this::copy); if (!allCopied) { @@ -224,6 +227,20 @@ private void printNotExist() { spec.commandLine().getErr().println(NOT_FIND); } + private boolean hasNestedDestination(List properties) { + for (Property source : properties) { + for (Property target : properties) { + if (target.destination.startsWith(source.original)) { + spec.commandLine().getErr().println(String.format( + "destination [%s] can not be inside original [%s], please check!", + target.destination, source.original)); + return true; + } + } + } + return false; + } + static class Property { diff --git a/plugins/src/test/java/org/tron/plugins/DbMoveTest.java b/plugins/src/test/java/org/tron/plugins/DbMoveTest.java index 26aeca21b87..7427cb6e4ed 100644 --- a/plugins/src/test/java/org/tron/plugins/DbMoveTest.java +++ b/plugins/src/test/java/org/tron/plugins/DbMoveTest.java @@ -5,6 +5,7 @@ import java.io.PrintWriter; import java.io.StringWriter; import java.net.URL; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -83,6 +84,23 @@ private static int mv(File database, String configPath) { return new CommandLine(new Toolkit()).execute(mvArgs(database, configPath)); } + private File writeConfig(String fileName, String[]... entries) throws IOException { + StringBuilder content = new StringBuilder("storage {\n properties = [\n"); + for (String[] entry : entries) { + content.append(" {\n name = \"").append(entry[0]) + .append("\",\n path = \"").append(entry[1]).append("\",\n },\n"); + } + content.append(" ]\n}\n"); + File config = temporaryFolder.newFile(fileName); + Files.write(config.toPath(), content.toString().getBytes(StandardCharsets.UTF_8)); + return config; + } + + private static void assertUntouched(File source) { + Assert.assertTrue(source.isDirectory()); + Assert.assertFalse(Files.isSymbolicLink(source.toPath())); + } + @Test public void testMvForLevelDB() throws RocksDBException, IOException { File database = temporaryFolder.newFolder("database"); @@ -303,6 +321,35 @@ public void testRecoveryHintWhenSourceDeleteFails() throws RocksDBException, IOE } } + @Test + public void testDestinationInsideOwnSourceRejected() throws RocksDBException, IOException { + File database = newDatabase(); + File accountDir = Paths.get(database.getPath(), ACCOUNT).toFile(); + File config = writeConfig("self-nested.conf", new String[] {ACCOUNT, accountDir.getPath()}); + + Assert.assertEquals(2, mv(database, config.getPath())); + assertUntouched(accountDir); + Assert.assertFalse(new File(accountDir, "database").exists()); + } + + @Test + public void testDestinationInsideOtherSourceRejected() throws RocksDBException, IOException { + File database = newDatabase(); + File accountDir = Paths.get(database.getPath(), ACCOUNT).toFile(); + File transDir = Paths.get(database.getPath(), TRANS).toFile(); + String[] trans = {TRANS, OUTPUT_DIRECTORY + "/dest"}; + String[] account = {ACCOUNT, new File(transDir, "nested").getPath()}; + File transFirst = writeConfig("trans-first.conf", trans, account); + File accountFirst = writeConfig("account-first.conf", account, trans); + + Assert.assertEquals(2, mv(database, transFirst.getPath())); + Assert.assertEquals(2, mv(database, accountFirst.getPath())); + assertUntouched(accountDir); + assertUntouched(transDir); + Assert.assertFalse(new File(transDir, "nested").exists()); + Assert.assertFalse(Paths.get(OUTPUT_DIRECTORY, "dest").toFile().exists()); + } + @Test public void testDuplicate() throws IOException { File output = temporaryFolder.newFolder(); From e6daa90508d5cce4aa3f7f6ad8788d27626512e5 Mon Sep 17 00:00:00 2001 From: halibobo1205 Date: Tue, 29 Sep 2026 14:45:57 +0800 Subject: [PATCH 3/5] fix(toolkit): roll back db mv copies on runtime failures --- .../src/main/java/common/org/tron/plugins/DbMove.java | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/plugins/src/main/java/common/org/tron/plugins/DbMove.java b/plugins/src/main/java/common/org/tron/plugins/DbMove.java index af6c6fd6e53..edeee594562 100644 --- a/plugins/src/main/java/common/org/tron/plugins/DbMove.java +++ b/plugins/src/main/java/common/org/tron/plugins/DbMove.java @@ -4,7 +4,6 @@ import com.typesafe.config.ConfigFactory; import java.io.File; import java.io.IOException; -import java.io.UncheckedIOException; import java.nio.file.Files; import java.nio.file.LinkOption; import java.nio.file.Path; @@ -115,11 +114,6 @@ public Integer call() throws Exception { } private boolean copy(Property p) { - if (!p.destination.toFile().mkdirs()) { - spec.commandLine().getErr().println(String.format("%s create failed.", p.destination)); - return false; - } - AtomicBoolean hasError = new AtomicBoolean(false); try (Stream files = Files.walk(p.original)) { // Collect the tree before copying anything: a traversal failure must @@ -127,18 +121,19 @@ private boolean copy(Property p) { // before the failure could recreate the destination after the rollback // has already deleted it, and the retry would then be rejected. List sources = files.collect(Collectors.toList()); + Files.createDirectories(p.destination); ProgressBar.wrap(sources.parallelStream(), p.name).forEach(source -> { if (hasError.get()) { return; } try { copyEntry(p, source); - } catch (IOException e) { + } catch (IOException | RuntimeException e) { hasError.set(true); spec.commandLine().getErr().println(e); } }); - } catch (IOException | UncheckedIOException e) { + } catch (IOException | RuntimeException e) { hasError.set(true); spec.commandLine().getErr().println(e); } From f3deb19d4c4a4880331b4efe9e9125109a5c27bb Mon Sep 17 00:00:00 2001 From: halibobo1205 Date: Tue, 29 Sep 2026 15:00:20 +0800 Subject: [PATCH 4/5] test(toolkit): cover db mv rollback and finalize failure paths --- .../java/org/tron/plugins/DbMoveTest.java | 132 +++++++++++++++--- 1 file changed, 111 insertions(+), 21 deletions(-) diff --git a/plugins/src/test/java/org/tron/plugins/DbMoveTest.java b/plugins/src/test/java/org/tron/plugins/DbMoveTest.java index 7427cb6e4ed..eb293c5aa96 100644 --- a/plugins/src/test/java/org/tron/plugins/DbMoveTest.java +++ b/plugins/src/test/java/org/tron/plugins/DbMoveTest.java @@ -9,7 +9,13 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Base64; +import java.util.List; +import java.util.Map; import java.util.Objects; +import java.util.TreeMap; +import java.util.stream.Collectors; +import java.util.stream.Stream; import lombok.extern.slf4j.Slf4j; import org.junit.After; import org.junit.Assert; @@ -17,8 +23,12 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; +import org.mockito.ArgumentMatchers; +import org.mockito.MockedStatic; +import org.mockito.Mockito; import org.rocksdb.RocksDBException; import org.tron.plugins.utils.DBUtils; +import org.tron.plugins.utils.FileUtils; import org.tron.plugins.utils.db.DbTool; import picocli.CommandLine; @@ -101,6 +111,24 @@ private static void assertUntouched(File source) { Assert.assertFalse(Files.isSymbolicLink(source.toPath())); } + private static File destination(String name) { + return Paths.get(OUTPUT_DIRECTORY, "dest", "database", name).toFile(); + } + + private static Map snapshot(File dir) throws IOException { + Path root = dir.toPath(); + List files; + try (Stream paths = Files.walk(root)) { + files = paths.filter(Files::isRegularFile).collect(Collectors.toList()); + } + Map contents = new TreeMap<>(); + for (Path file : files) { + contents.put(root.relativize(file).toString(), + Base64.getEncoder().encodeToString(Files.readAllBytes(file))); + } + return contents; + } + @Test public void testMvForLevelDB() throws RocksDBException, IOException { File database = temporaryFolder.newFolder("database"); @@ -125,8 +153,9 @@ public void testSourceKeptWhenCopyFails() throws RocksDBException, IOException { File database = newDatabase(); File accountDir = Paths.get(database.getPath(), ACCOUNT).toFile(); File marketDir = Paths.get(database.getPath(), DBUtils.MARKET_PAIR_PRICE_TO_ORDER).toFile(); - File victim = Objects.requireNonNull(accountDir.listFiles(File::isFile))[0]; - // Make one source file unreadable so its copy fails part-way through the move. + Map accountBefore = snapshot(accountDir); + Map marketBefore = snapshot(marketDir); + File victim = Objects.requireNonNull(marketDir.listFiles(File::isFile))[0]; Assert.assertTrue(victim.setReadable(false, false)); String[] args = mvArgs(database, getConfig("config.conf")); @@ -134,29 +163,20 @@ public void testSourceKeptWhenCopyFails() throws RocksDBException, IOException { StringWriter output = new StringWriter(); cli.setOut(new PrintWriter(output)); try { - // Skip when the platform ignores the read bit (e.g. running as root). Assume.assumeFalse("file still readable (root?), cannot simulate copy failure", victim.canRead()); Assert.assertEquals(1, cli.execute(args)); - - // A failed copy must keep every source intact and roll back all destinations. - Assert.assertTrue("source dir must be kept on copy failure", accountDir.exists()); - Assert.assertFalse("source must not be replaced by a symlink", - Files.isSymbolicLink(accountDir.toPath())); - Assert.assertTrue("source file must still exist", victim.exists()); - Assert.assertTrue("other source dirs must not be moved after a copy failure", - marketDir.exists()); - Assert.assertFalse("other source dirs must not be replaced by symlinks", - Files.isSymbolicLink(marketDir.toPath())); - Assert.assertFalse("partial destination must be removed", - Paths.get(OUTPUT_DIRECTORY, "dest", "database", ACCOUNT).toFile().exists()); - Assert.assertFalse("failure must not be reported as success", - output.toString().contains("move db done.")); } finally { victim.setReadable(true, false); } + Assert.assertFalse(output.toString().contains("move db done.")); + assertUntouched(accountDir); + assertUntouched(marketDir); + Assert.assertEquals(accountBefore, snapshot(accountDir)); + Assert.assertEquals(marketBefore, snapshot(marketDir)); + Assert.assertFalse(destination(ACCOUNT).exists()); + Assert.assertFalse(destination(DBUtils.MARKET_PAIR_PRICE_TO_ORDER).exists()); - // Once the I/O problem is fixed, the unchanged command must be directly retryable. Assert.assertEquals(0, cli.execute(args)); Assert.assertTrue(Files.isSymbolicLink(accountDir.toPath())); Assert.assertTrue(Files.isSymbolicLink(marketDir.toPath())); @@ -196,7 +216,7 @@ public void testInTreeSymlinkRejected() throws RocksDBException, IOException { Files.isSymbolicLink(accountDir.toPath())); Assert.assertTrue("symlink target must never be touched", sentinel.exists()); Assert.assertFalse("partial destination must be rolled back", - Paths.get(OUTPUT_DIRECTORY, "dest", "database", ACCOUNT).toFile().exists()); + destination(ACCOUNT).exists()); } @Test @@ -253,7 +273,7 @@ public void testUnreadableSubdirFailsCopy() throws RocksDBException, IOException Assert.assertFalse("source must not be replaced by a symlink", Files.isSymbolicLink(accountDir.toPath())); Assert.assertFalse("partial destination must be rolled back", - Paths.get(OUTPUT_DIRECTORY, "dest", "database", ACCOUNT).toFile().exists()); + destination(ACCOUNT).exists()); } finally { subDir.setReadable(true, false); } @@ -303,7 +323,7 @@ public void testRecoveryHintWhenSourceDeleteFails() throws RocksDBException, IOE Assert.assertTrue("source dir must be kept", accountDir.exists()); Assert.assertFalse("source must not be replaced by a symlink", Files.isSymbolicLink(accountDir.toPath())); - File dest = Paths.get(OUTPUT_DIRECTORY, "dest", "database", ACCOUNT).toFile(); + File dest = destination(ACCOUNT); Assert.assertTrue("complete copy must be kept for manual recovery", dest.exists()); String expectedHint = String.format( "To recover manually: remove %s if present, then create a symbolic link at %s" @@ -350,6 +370,76 @@ public void testDestinationInsideOtherSourceRejected() throws RocksDBException, Assert.assertFalse(Paths.get(OUTPUT_DIRECTORY, "dest").toFile().exists()); } + @Test + public void testLeftoverReportedWhenCleanupFails() throws RocksDBException, IOException { + File database = newDatabase(); + File accountDir = Paths.get(database.getPath(), ACCOUNT).toFile(); + File marketDir = Paths.get(database.getPath(), DBUtils.MARKET_PAIR_PRICE_TO_ORDER).toFile(); + File accountDest = destination(ACCOUNT).getCanonicalFile(); + Map accountBefore = snapshot(accountDir); + Map marketBefore = snapshot(marketDir); + File victim = Objects.requireNonNull(marketDir.listFiles(File::isFile))[0]; + Assert.assertTrue(victim.setReadable(false, false)); + + CommandLine cli = new CommandLine(new Toolkit()); + StringWriter output = new StringWriter(); + StringWriter error = new StringWriter(); + cli.setOut(new PrintWriter(output)); + cli.setErr(new PrintWriter(error)); + try (MockedStatic fileUtils = + Mockito.mockStatic(FileUtils.class, Mockito.CALLS_REAL_METHODS)) { + Assume.assumeFalse("file still readable (root?), cannot simulate copy failure", + victim.canRead()); + fileUtils.when(() -> FileUtils.deleteDir(ArgumentMatchers.argThat(accountDest::equals))) + .thenReturn(false); + Assert.assertEquals(1, cli.execute(mvArgs(database, getConfig("config.conf")))); + } finally { + victim.setReadable(true, false); + } + Assert.assertFalse(output.toString().contains("move db done.")); + assertUntouched(accountDir); + assertUntouched(marketDir); + Assert.assertEquals(accountBefore, snapshot(accountDir)); + Assert.assertEquals(marketBefore, snapshot(marketDir)); + Assert.assertTrue(accountDest.isDirectory()); + Assert.assertFalse(destination(DBUtils.MARKET_PAIR_PRICE_TO_ORDER).exists()); + Assert.assertTrue(error.toString().contains(accountDest + " cleanup failed")); + Assert.assertTrue(error.toString().contains("leftover copies remain")); + } + + @Test + public void testRecoveryHintWhenLinkCreationFails() throws RocksDBException, IOException { + File database = newDatabase(); + File accountDir = Paths.get(database.getPath(), ACCOUNT).toFile().getCanonicalFile(); + File marketDir = Paths.get(database.getPath(), DBUtils.MARKET_PAIR_PRICE_TO_ORDER).toFile(); + File accountDest = destination(ACCOUNT).getCanonicalFile(); + Map accountBefore = snapshot(accountDir); + + CommandLine cli = new CommandLine(new Toolkit()); + StringWriter output = new StringWriter(); + StringWriter error = new StringWriter(); + cli.setOut(new PrintWriter(output)); + cli.setErr(new PrintWriter(error)); + try (MockedStatic fileUtils = + Mockito.mockStatic(FileUtils.class, Mockito.CALLS_REAL_METHODS)) { + fileUtils.when(() -> FileUtils.deleteDir(ArgumentMatchers.argThat(accountDir::equals))) + .thenAnswer(invocation -> { + boolean deleted = (boolean) invocation.callRealMethod(); + Files.createFile(accountDir.toPath()); + return deleted; + }); + Assert.assertEquals(1, cli.execute(mvArgs(database, getConfig("config.conf")))); + } + Assert.assertFalse(output.toString().contains("move db done.")); + Assert.assertEquals(accountBefore, snapshot(accountDest)); + Assert.assertTrue(error.toString().contains( + accountDir + " move failed; the complete copy is at " + accountDest + ", keep it.")); + Assert.assertTrue(error.toString().contains(String.format( + "To recover manually: remove %s if present, then create a symbolic link at %s" + + " pointing to %s.", accountDir, accountDir, accountDest))); + Assert.assertTrue(Files.isSymbolicLink(marketDir.toPath())); + } + @Test public void testDuplicate() throws IOException { File output = temporaryFolder.newFolder(); From 5d351377ffa1135f5e44ca93241faf67839ce996 Mon Sep 17 00:00:00 2001 From: halibobo1205 Date: Tue, 29 Sep 2026 15:00:20 +0800 Subject: [PATCH 5/5] fix(toolkit): show the stop-the-node note in db mv help --- plugins/src/main/java/common/org/tron/plugins/Db.java | 8 +++++--- plugins/src/main/java/common/org/tron/plugins/DbMove.java | 1 + plugins/src/test/java/org/tron/plugins/DbMoveTest.java | 4 ++++ 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/plugins/src/main/java/common/org/tron/plugins/Db.java b/plugins/src/main/java/common/org/tron/plugins/Db.java index 217c3fbd301..3565962b7c7 100644 --- a/plugins/src/main/java/common/org/tron/plugins/Db.java +++ b/plugins/src/main/java/common/org/tron/plugins/Db.java @@ -6,9 +6,7 @@ mixinStandardHelpOptions = true, version = "db command 1.0", description = "An rich command set that provides high-level operations for dbs.", - header = "All `db` tools operate directly on the database files.\n" - + "Before performing a database operation,\n" - + "you must stop the currently running FullNode service.\n", + header = Db.STOP_NODE_HEADER, subcommands = {CommandLine.HelpCommand.class, DbMove.class, DbArchive.class, @@ -20,4 +18,8 @@ commandListHeading = "%nCommands:%n%nThe most commonly used db commands are:%n" ) public class Db { + + static final String STOP_NODE_HEADER = "All db tools operate directly on the database files.\n" + + "Before performing a database operation,\n" + + "you must stop the currently running FullNode service.\n"; } diff --git a/plugins/src/main/java/common/org/tron/plugins/DbMove.java b/plugins/src/main/java/common/org/tron/plugins/DbMove.java index edeee594562..93bea0cbc52 100644 --- a/plugins/src/main/java/common/org/tron/plugins/DbMove.java +++ b/plugins/src/main/java/common/org/tron/plugins/DbMove.java @@ -26,6 +26,7 @@ @Slf4j(topic = "move") @Command(name = "mv", aliases = "move", + header = Db.STOP_NODE_HEADER, description = "Move db to pre-set new path . For example HDD,reduce storage expenses.") public class DbMove implements Callable { diff --git a/plugins/src/test/java/org/tron/plugins/DbMoveTest.java b/plugins/src/test/java/org/tron/plugins/DbMoveTest.java index eb293c5aa96..cddeda6a271 100644 --- a/plugins/src/test/java/org/tron/plugins/DbMoveTest.java +++ b/plugins/src/test/java/org/tron/plugins/DbMoveTest.java @@ -455,6 +455,10 @@ public void testHelp() { String[] args = new String[] {"db", "mv", "-h"}; CommandLine cli = new CommandLine(new Toolkit()); Assert.assertEquals(0, cli.execute(args)); + CommandLine db = cli.getSubcommands().get("db"); + Assert.assertFalse(db.getUsageMessage().contains("`db`")); + Assert.assertTrue(db.getSubcommands().get("mv").getUsageMessage() + .contains("you must stop the currently running FullNode service")); } @Test