From cc85dc5b3c94e0a1e536ba8f4d1211625fed7ed2 Mon Sep 17 00:00:00 2001 From: halibobo1205 Date: Thu, 11 Jun 2026 15:33:59 +0800 Subject: [PATCH] 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 | 188 ++++++++++----- .../java/org/tron/plugins/DbMoveTest.java | 223 +++++++++++++++++- 4 files changed, 351 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..e81e90fe1ba 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,107 @@ 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)) { + ProgressBar.wrap(files.parallel(), 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 +239,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 +267,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 +276,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..d36a90aadc3 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,203 @@ 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); + } + } + + @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