From bd835726f2e0ed2600c1e426bd1869d6fb1252c8 Mon Sep 17 00:00:00 2001 From: Maxim Muzafarov Date: Wed, 15 Jan 2025 23:49:01 +0100 Subject: [PATCH] Rename conflicting nodetool import --copy-data short option from -p to -cd Patch by Maxim Muzafarov; reviewed by Stefan Miklosovic, Caleb Rackliffe, Bernardo Botella Corbi for CASSANDRA-20214 --- CHANGES.txt | 1 + .../apache/cassandra/db/SSTableImporter.java | 16 ++--- .../cassandra/tools/nodetool/Import.java | 2 +- .../org/apache/cassandra/db/ImportTest.java | 64 ++++++++++++++++--- 4 files changed, 65 insertions(+), 18 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index fbc2301eac..f333ddb532 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.0.21 + * Rename conflicting nodetool import --copy-data short option from -p to -cd (CASSANDRA-20214) * Fix PasswordObfuscator failing to obfuscate certain passwords (CASSANDRA-21113) * Fix negative memtable allocator ownership when an update is shadowed by an existing row deletion (CASSANDRA-21469) * Consider first token of SSTable when calculating SSTable intersection in LeveledScanner (CASSANDRA-21369) diff --git a/src/java/org/apache/cassandra/db/SSTableImporter.java b/src/java/org/apache/cassandra/db/SSTableImporter.java index c7c4f6b010..6efe560daf 100644 --- a/src/java/org/apache/cassandra/db/SSTableImporter.java +++ b/src/java/org/apache/cassandra/db/SSTableImporter.java @@ -381,14 +381,14 @@ public class SSTableImporter public static class Options { - private final Set srcPaths; - private final boolean resetLevel; - private final boolean clearRepaired; - private final boolean verifySSTables; - private final boolean verifyTokens; - private final boolean invalidateCaches; - private final boolean extendedVerify; - private final boolean copyData; + final Set srcPaths; + final boolean resetLevel; + final boolean clearRepaired; + final boolean verifySSTables; + final boolean verifyTokens; + final boolean invalidateCaches; + final boolean extendedVerify; + final boolean copyData; public Options(Set srcPaths, boolean resetLevel, boolean clearRepaired, boolean verifySSTables, boolean verifyTokens, boolean invalidateCaches, boolean extendedVerify, boolean copyData) { diff --git a/src/java/org/apache/cassandra/tools/nodetool/Import.java b/src/java/org/apache/cassandra/tools/nodetool/Import.java index 73fa3144bb..c9a388a982 100644 --- a/src/java/org/apache/cassandra/tools/nodetool/Import.java +++ b/src/java/org/apache/cassandra/tools/nodetool/Import.java @@ -76,7 +76,7 @@ public class Import extends NodeToolCmd private boolean extendedVerify = false; @Option(title = "copy_data", - name = {"-p", "--copy-data"}, + name = {"-cd", "--copy-data"}, description = "Copy data from source directories instead of moving them") private boolean copyData = false; diff --git a/test/unit/org/apache/cassandra/db/ImportTest.java b/test/unit/org/apache/cassandra/db/ImportTest.java index e812d0f724..68a1258ebf 100644 --- a/test/unit/org/apache/cassandra/db/ImportTest.java +++ b/test/unit/org/apache/cassandra/db/ImportTest.java @@ -23,6 +23,7 @@ import java.io.IOException; import java.io.RandomAccessFile; import java.nio.file.Files; import java.nio.file.Path; +import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; import java.util.Iterator; @@ -32,8 +33,8 @@ import java.util.Set; import com.google.common.collect.Lists; import com.google.common.collect.Sets; import org.apache.commons.lang3.StringUtils; -import org.junit.After; import org.junit.Assert; +import org.junit.BeforeClass; import org.junit.Test; import org.apache.cassandra.cache.RowCacheKey; @@ -47,16 +48,28 @@ import org.apache.cassandra.locator.InetAddressAndPort; import org.apache.cassandra.locator.TokenMetadata; import org.apache.cassandra.service.CacheService; import org.apache.cassandra.service.StorageService; +import org.apache.cassandra.tools.ToolRunner; import org.apache.cassandra.utils.FBUtilities; +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; +/** + * @see org.apache.cassandra.tools.nodetool.Import + */ public class ImportTest extends CQLTester { - @After - public void afterTest() + @BeforeClass + public static void setupClass() throws Throwable + { + startJMXServer(); + } + + @Override + public void afterTest() throws Throwable { SSTableReader.resetTidying(); } @@ -78,7 +91,7 @@ public class ImportTest extends CQLTester // copy is true - so importing will be done by copying importSSTables(SSTableImporter.Options.options(backupDir.toString()).copyData(true).build(), 10); // files are left there as they were just copied - Assert.assertNotEquals(0, countFiles(backupDir)); + assertNotEquals(0, countFiles(backupDir)); } private File prepareBasicImporting() throws Throwable @@ -347,9 +360,9 @@ public class ImportTest extends CQLTester // then we moved out 1 sstable, a correct one (in backupdirCorrect). // now import should fail import on backupdir, but import the one in backupdirCorrect. SSTableImporter.Options options = SSTableImporter.Options.options(Sets.newHashSet(backupdir.toString(), backupdirCorrect.toString())).copyData(copy).verifySSTables(verify).build(); - SSTableImporter importer = new SSTableImporter(getCurrentColumnFamilyStore()); - List failedDirectories = importer.importNewSSTables(options); - assertEquals(Collections.singletonList(backupdir.toString()), failedDirectories); + ToolRunner.ToolResult result = assertImportFailed(getCurrentColumnFamilyStore(), options); + assertThat(result.getStderr()).contains("Some directories failed to import, check server logs for details:"); + assertThat(result.getStderr()).contains(backupdir.toString()); UntypedResultSet res = execute("SELECT * FROM %s"); for (UntypedResultSet.Row r : res) { @@ -707,7 +720,7 @@ public class ImportTest extends CQLTester assertEquals(10, execute(String.format("select * from %s.%s", KEYSPACE, table)).size()); // files are left there as they were just copied - Assert.assertNotEquals(0, countFiles(backupDir)); + assertNotEquals(0, countFiles(backupDir)); } finally { @@ -751,7 +764,7 @@ public class ImportTest extends CQLTester assertEquals(10, execute(String.format("select * from %s.%s", KEYSPACE, table)).size()); // files are left there as they were just copied - Assert.assertNotEquals(0, countFiles(backupDir)); + assertNotEquals(0, countFiles(backupDir)); } finally { @@ -759,6 +772,39 @@ public class ImportTest extends CQLTester } } + private static ToolRunner.ToolResult assertImportFailed(ColumnFamilyStore cfs, SSTableImporter.Options options) + { + ToolRunner.ToolResult result = doImportWithNodetool(cfs, options); + assertNotEquals(0, result.getExitCode()); + return result; + } + + private static ToolRunner.ToolResult doImportWithNodetool(ColumnFamilyStore cfs, SSTableImporter.Options options) + { + List args = new ArrayList<>(); + args.add("import"); + if (!options.resetLevel) + args.add("--keep-level"); + if (!options.clearRepaired) + args.add("--keep-repaired"); + if (!options.verifySSTables) + args.add("--no-verify"); + if (!options.verifyTokens) + args.add("--no-tokens"); + if (!options.invalidateCaches) + args.add("--no-invalidate-caches"); + if (options.extendedVerify) + args.add("--extended-verify"); + if (options.copyData) + args.add("-cd"); + + args.add(cfs.keyspace.getName()); + args.add(cfs.getTableName()); + args.addAll(options.srcPaths); + + return ToolRunner.invokeNodetool(args.toArray(new String[0])); + } + private static class MockCFS extends ColumnFamilyStore { public MockCFS(ColumnFamilyStore cfs, Directories dirs)