diff --git a/src/java/org/apache/cassandra/tools/nodetool/Compact.java b/src/java/org/apache/cassandra/tools/nodetool/Compact.java index 8b73caef8350..2aadc51c02b4 100644 --- a/src/java/org/apache/cassandra/tools/nodetool/Compact.java +++ b/src/java/org/apache/cassandra/tools/nodetool/Compact.java @@ -23,16 +23,24 @@ import org.apache.cassandra.tools.NodeProbe; import org.apache.cassandra.tools.nodetool.layout.CassandraUsage; +import picocli.CommandLine.ArgGroup; import picocli.CommandLine.Command; import picocli.CommandLine.Option; import picocli.CommandLine.Parameters; +import static org.apache.cassandra.tools.nodetool.CommandUtils.concatArgs; import static org.apache.cassandra.tools.nodetool.CommandUtils.parseOptionalKeyspace; import static org.apache.cassandra.tools.nodetool.CommandUtils.parseOptionalTables; import static org.apache.commons.lang3.StringUtils.EMPTY; -// TODO CASSANDRA-20793 Types of input aguments shouldn't be mixed in the same command. The keyspace, table and SSTable file arguments should have their own commands. -@Command(name = "compact", description = "Force a (major) compaction on one or more tables or user-defined compaction on given SSTables") +/** + * @deprecated See CASSANDRA-20793. Use {@code compact keyspace}, {@code compact sstables}, + * or {@code compact range} instead. + */ +@Deprecated(since = "7.0") +@Command(name = "compact", + description = "Force a (major) compaction on one or more tables or user-defined compaction on given SSTables", + subcommands = { Compact.Keyspace.class, Compact.SSTables.class, Compact.Range.class }) public class Compact extends AbstractCommand { @CassandraUsage(usage = "[ ...] or ...", @@ -66,6 +74,8 @@ public class Compact extends AbstractCommand @Override public void execute(NodeProbe probe) { + probe.output().out.println("WARNING: nodetool compact is deprecated, use 'compact keyspace', 'compact sstables', or 'compact range' instead."); + final boolean startEndTokenProvided = !(startToken.isEmpty() && endToken.isEmpty()); final boolean partitionKeyProvided = !partitionKey.isEmpty(); final boolean tokenProvided = startEndTokenProvided || partitionKeyProvided; @@ -118,4 +128,149 @@ else if (partitionKeyProvided) } } } + + /** + * Subcommand for compacting specific keyspace tables or a specific partition within a keyspace. + */ + @Command(name = "keyspace", description = "Force a (major) compaction on one or more tables in a keyspace") + public static class Keyspace extends AbstractCommand + { + @CassandraUsage(usage = "[ ...]", + description = "The keyspace followed by one or many tables") + @Parameters(index = "0", description = "The keyspace name", arity = "0..1") + private String keyspaceName; + + @Parameters(index = "1..*", description = "The table names", arity = "0..*") + private List tableNames = new ArrayList<>(); + + @Option(paramLabel = "split_output", names = { "-s", "--split-output" }, description = "Use -s to not create a single big file") + private boolean splitOutput = false; + + @Option(paramLabel = "partition_key", names = { "--partition" }, description = "String representation of the partition key") + private String partitionKey = EMPTY; + + @Option(paramLabel = "jobs", + names = {"-j", "--jobs"}, + description = "Use -j to specify the maximum number of threads to use for parallel compaction. " + + "If not set, up to half the compaction threads will be used. " + + "If set to 0, the major compaction will use all threads and will not permit other compactions to run until it completes (use with caution).") + private Integer parallelism = null; + + @Override + public void execute(NodeProbe probe) + { + if (splitOutput && !partitionKey.isEmpty()) + throw new RuntimeException("Invalid option combination: Can not use split-output with --partition"); + + List args = concatArgs(keyspaceName, tableNames); + List keyspaces = parseOptionalKeyspace(args, probe); + String[] tables = parseOptionalTables(args); + + for (String ks : keyspaces) + { + try + { + if (!partitionKey.isEmpty()) + { + probe.forceKeyspaceCompactionForPartitionKey(ks, partitionKey, tables); + } + else + { + if (parallelism != null) + probe.forceKeyspaceCompaction(splitOutput, parallelism, ks, tables); + else + probe.forceKeyspaceCompaction(splitOutput, ks, tables); + } + } + catch (Exception e) + { + throw new RuntimeException("Error occurred during compaction", e); + } + } + } + } + + /** + * Subcommand for user-defined compaction on specific SSTable files. + */ + @Command(name = "sstables", description = "Force user-defined compaction on given SSTable files") + public static class SSTables extends AbstractCommand + { + @CassandraUsage(usage = "...", + description = "List of SSTable data files for user-defined compaction") + @Parameters(index = "0..*", description = "List of SSTable data files for user-defined compaction", arity = "1..*") + private List sstableFiles = new ArrayList<>(); + + @Override + public void execute(NodeProbe probe) + { + try + { + String userDefinedFiles = String.join(",", sstableFiles); + probe.forceUserDefinedCompaction(userDefinedFiles); + } + catch (Exception e) + { + throw new RuntimeException("Error occurred during user defined compaction", e); + } + } + } + + /** + * Subcommand for token range compaction on one or more tables in a keyspace. + * At least one of --start-token or --end-token must be provided. + */ + @Command(name = "range", description = "Force compaction on a token range for one or more tables in a keyspace") + public static class Range extends AbstractCommand + { + @CassandraUsage(usage = "[ ...]", + description = "The keyspace followed by one or many tables") + @Parameters(index = "0", description = "The keyspace name", arity = "0..1") + private String keyspaceName; + + @Parameters(index = "1..*", description = "The table names", arity = "0..*") + private List tableNames = new ArrayList<>(); + + @ArgGroup(exclusive = false, multiplicity = "1..1") + private TokenRange tokenRange; + + /** + * Token range options for compaction. At least one of --start-token or --end-token must be provided. + */ + static class TokenRange + { + @Option(paramLabel = "start_token", + names = { "-st", "--start-token" }, + description = "Use -st to specify a token at which the compaction range starts (inclusive)") + private String startToken = EMPTY; + + @Option(paramLabel = "end_token", + names = { "-et", "--end-token" }, + description = "Use -et to specify a token at which compaction range ends (inclusive)") + private String endToken = EMPTY; + } + + @Override + public void execute(NodeProbe probe) + { + List args = concatArgs(keyspaceName, tableNames); + List keyspaces = parseOptionalKeyspace(args, probe); + String[] tables = parseOptionalTables(args); + + String startToken = tokenRange != null ? tokenRange.startToken : EMPTY; + String endToken = tokenRange != null ? tokenRange.endToken : EMPTY; + + for (String ks : keyspaces) + { + try + { + probe.forceKeyspaceCompactionForTokenRange(ks, startToken, endToken, tables); + } + catch (Exception e) + { + throw new RuntimeException("Error occurred during compaction", e); + } + } + } + } } diff --git a/test/unit/org/apache/cassandra/tools/nodetool/CompactTest.java b/test/unit/org/apache/cassandra/tools/nodetool/CompactTest.java index 1763147547ff..5f0a25f21577 100644 --- a/test/unit/org/apache/cassandra/tools/nodetool/CompactTest.java +++ b/test/unit/org/apache/cassandra/tools/nodetool/CompactTest.java @@ -26,6 +26,7 @@ import org.apache.cassandra.cql3.CQLTester; import org.apache.cassandra.db.ColumnFamilyStore; import org.apache.cassandra.db.Keyspace; +import org.apache.cassandra.tools.ToolRunner; import static org.apache.cassandra.tools.ToolRunner.invokeNodetool; @@ -38,6 +39,8 @@ public static void setup() throws Throwable startJMXServer(); } + // Deprecated compact command tests (backward compatibility) + @Test public void keyPresent() throws Throwable { @@ -101,4 +104,107 @@ public void keyWrongType() .failure() .errorContains(String.format("Unable to parse partition key 'this_will_not_work' for table %s.%s; Unable to make long from 'this_will_not_work'", keyspace(), currentTable())); } + + @Test + public void compactCommandIsDeprecated() throws Throwable + { + createTable("CREATE TABLE %s (id bigint, value text, PRIMARY KEY ((id)))"); + ToolRunner.ToolResult result = invokeNodetool("compact", keyspace(), currentTable()); + result.assertOnCleanExit(); + Assertions.assertThat(result.getStdout()).contains("WARNING: nodetool compact is deprecated"); + } + + // compact keyspace subcommand tests + + @Test + public void keyPresentWithKeyspaceSubcommand() throws Throwable + { + long key = 42; + createTable("CREATE TABLE %s (id bigint, value text, PRIMARY KEY ((id)))"); + ColumnFamilyStore cfs = Keyspace.open(keyspace()).getColumnFamilyStore(currentTable()); + cfs.disableAutoCompaction(); + for (int i = 0; i < 10; i++) + { + execute("INSERT INTO %s (id, value) VALUES (?, ?)", key, "This is just some text... part " + i); + flush(keyspace()); + } + Assertions.assertThat(cfs.getTracker().getView().liveSSTables()).hasSize(10); + invokeNodetool("compact", "keyspace", "--partition", Long.toString(key), keyspace(), currentTable()).assertOnCleanExit(); + Assertions.assertThat(cfs.getTracker().getView().liveSSTables()).hasSize(1); + } + + @Test + public void keyNotPresentWithKeyspaceSubcommand() throws Throwable + { + long key = 42; + createTable("CREATE TABLE %s (id bigint, value text, PRIMARY KEY ((id)))"); + ColumnFamilyStore cfs = Keyspace.open(keyspace()).getColumnFamilyStore(currentTable()); + cfs.disableAutoCompaction(); + for (int i = 0; i < 10; i++) + { + execute("INSERT INTO %s (id, value) VALUES (?, ?)", key, "This is just some text... part " + i); + flush(keyspace()); + } + Assertions.assertThat(cfs.getTracker().getView().liveSSTables()).hasSize(10); + for (long keyNotFound : Arrays.asList(key - 1, key + 1)) + { + invokeNodetool("compact", "keyspace", "--partition", Long.toString(keyNotFound), keyspace(), currentTable()).assertOnCleanExit(); + Assertions.assertThat(cfs.getTracker().getView().liveSSTables()).hasSize(10); + } + } + + @Test + public void tableNotFoundWithKeyspaceSubcommand() + { + invokeNodetool("compact", "keyspace", "--partition", Long.toString(42), keyspace(), "doesnotexist") + .asserts() + .failure() + .errorContains(String.format("java.lang.IllegalArgumentException: Unknown keyspace/cf pair (%s.doesnotexist)", keyspace())); + } + + @Test + public void keyWrongTypeWithKeyspaceSubcommand() + { + createTable("CREATE TABLE %s (id bigint, value text, PRIMARY KEY ((id)))"); + invokeNodetool("compact", "keyspace", "--partition", "this_will_not_work", keyspace(), currentTable()) + .asserts() + .failure() + .errorContains(String.format("Unable to parse partition key 'this_will_not_work' for table %s.%s; Unable to make long from 'this_will_not_work'", keyspace(), currentTable())); + } + + @Test + public void regularCompactionWithKeyspaceSubcommand() throws Throwable + { + long key = 42; + createTable("CREATE TABLE %s (id bigint, value text, PRIMARY KEY ((id)))"); + ColumnFamilyStore cfs = Keyspace.open(keyspace()).getColumnFamilyStore(currentTable()); + cfs.disableAutoCompaction(); + for (int i = 0; i < 10; i++) + { + execute("INSERT INTO %s (id, value) VALUES (?, ?)", key, "This is just some text... part " + i); + flush(keyspace()); + } + Assertions.assertThat(cfs.getTracker().getView().liveSSTables()).hasSize(10); + invokeNodetool("compact", "keyspace", keyspace(), currentTable()).assertOnCleanExit(); + Assertions.assertThat(cfs.getTracker().getView().liveSSTables()).hasSize(1); + } + + @Test + public void splitOutputAndPartitionKeyFailsWithKeyspaceSubcommand() + { + createTable("CREATE TABLE %s (id bigint, value text, PRIMARY KEY ((id)))"); + invokeNodetool("compact", "keyspace", "--split-output", "--partition", Long.toString(42), keyspace(), currentTable()) + .asserts() + .failure() + .errorContains("Invalid option combination: Can not use split-output with --partition"); + } + + @Test + public void rangeSubcommandRequiresAtLeastOneToken() + { + createTable("CREATE TABLE %s (id bigint, value text, PRIMARY KEY ((id)))"); + invokeNodetool("compact", "range", keyspace(), currentTable()) + .asserts() + .failure(); + } } diff --git a/test/unit/org/apache/cassandra/tools/nodetool/mock/CompactMockTest.java b/test/unit/org/apache/cassandra/tools/nodetool/mock/CompactMockTest.java index 5667ca99192e..f68e61fa383f 100644 --- a/test/unit/org/apache/cassandra/tools/nodetool/mock/CompactMockTest.java +++ b/test/unit/org/apache/cassandra/tools/nodetool/mock/CompactMockTest.java @@ -19,16 +19,20 @@ import java.util.List; +import org.assertj.core.api.Assertions; import org.junit.Test; import org.mockito.Mockito; import org.apache.cassandra.db.compaction.CompactionManagerMBean; import org.apache.cassandra.service.StorageServiceMBean; +import org.apache.cassandra.tools.ToolRunner; import static org.mockito.Mockito.when; public class CompactMockTest extends AbstractNodetoolMock { + // Deprecated compact command tests (backward compatibility) + @Test public void testCompactForceKeyspaceCompactionForPartitionKey() throws Throwable { @@ -74,4 +78,159 @@ public void testCompactForceUserDefinedCompaction() throws Throwable invokeNodetool("compact", "--user-defined", ssTables[0], ssTables[1]).assertOnCleanExit(); Mockito.verify(mock).forceUserDefinedCompaction(String.join(",", ssTables)); } + + @Test + public void testDeprecatedCompactPrintsWarning() throws Throwable + { + String table = "table"; + StorageServiceMBean mock = getMock(STORAGE_SERVICE_MBEAN); + when(mock.getKeyspaces()).thenReturn(List.of(keyspace())); + when(mock.getNonSystemKeyspaces()).thenReturn(List.of(keyspace())); + ToolRunner.ToolResult result = invokeNodetool("compact", keyspace(), table); + result.assertOnCleanExit(); + Assertions.assertThat(result.getStdout()).contains("WARNING: nodetool compact is deprecated"); + } + + // compact keyspace subcommand tests + + @Test + public void testCompactKeyspaceSubcommandForceKeyspaceCompaction() throws Throwable + { + String table = "table"; + StorageServiceMBean mock = getMock(STORAGE_SERVICE_MBEAN); + when(mock.getKeyspaces()).thenReturn(List.of(keyspace())); + when(mock.getNonSystemKeyspaces()).thenReturn(List.of(keyspace())); + invokeNodetool("compact", "keyspace", keyspace(), table).assertOnCleanExit(); + Mockito.verify(mock).forceKeyspaceCompaction(false, keyspace(), table); + } + + @Test + public void testCompactKeyspaceSubcommandWithSplitOutput() throws Throwable + { + String table = "table"; + StorageServiceMBean mock = getMock(STORAGE_SERVICE_MBEAN); + when(mock.getKeyspaces()).thenReturn(List.of(keyspace())); + when(mock.getNonSystemKeyspaces()).thenReturn(List.of(keyspace())); + invokeNodetool("compact", "keyspace", "--split-output", keyspace(), table).assertOnCleanExit(); + Mockito.verify(mock).forceKeyspaceCompaction(true, keyspace(), table); + } + + @Test + public void testCompactKeyspaceSubcommandForPartitionKey() throws Throwable + { + long key = 43; + String table = "table"; + StorageServiceMBean mock = getMock(STORAGE_SERVICE_MBEAN); + when(mock.getKeyspaces()).thenReturn(List.of(keyspace())); + when(mock.getNonSystemKeyspaces()).thenReturn(List.of(keyspace())); + invokeNodetool("compact", "keyspace", "--partition", Long.toString(key), keyspace(), table).assertOnCleanExit(); + Mockito.verify(mock).forceKeyspaceCompactionForPartitionKey(keyspace(), Long.toString(key), table); + } + + @Test + public void testCompactKeyspaceSubcommandSplitOutputAndPartitionKeyFails() throws Throwable + { + long key = 43; + String table = "table"; + StorageServiceMBean mock = getMock(STORAGE_SERVICE_MBEAN); + when(mock.getKeyspaces()).thenReturn(List.of(keyspace())); + when(mock.getNonSystemKeyspaces()).thenReturn(List.of(keyspace())); + invokeNodetool("compact", "keyspace", "--split-output", "--partition", Long.toString(key), keyspace(), table) + .asserts() + .failure() + .errorContains("Invalid option combination: Can not use split-output with --partition"); + } + + @Test + public void testCompactKeyspaceSubcommandWithJobs() throws Throwable + { + String table = "table"; + StorageServiceMBean mock = getMock(STORAGE_SERVICE_MBEAN); + when(mock.getKeyspaces()).thenReturn(List.of(keyspace())); + when(mock.getNonSystemKeyspaces()).thenReturn(List.of(keyspace())); + invokeNodetool("compact", "keyspace", "--jobs", "2", keyspace(), table).assertOnCleanExit(); + Mockito.verify(mock).forceKeyspaceCompaction(false, 2, keyspace(), table); + } + + // compact sstables subcommand tests + + @Test + public void testCompactSSTablesSubcommandForceUserDefinedCompaction() throws Throwable + { + String[] ssTables = new String[] { "ssTable1", "ssTable2" }; + CompactionManagerMBean mock = getMock(COMPACTION_MANAGER_MBEAN); + invokeNodetool("compact", "sstables", ssTables[0], ssTables[1]).assertOnCleanExit(); + Mockito.verify(mock).forceUserDefinedCompaction(String.join(",", ssTables)); + } + + @Test + public void testCompactSSTablesSubcommandSingleFile() throws Throwable + { + String ssTable = "ssTable1"; + CompactionManagerMBean mock = getMock(COMPACTION_MANAGER_MBEAN); + invokeNodetool("compact", "sstables", ssTable).assertOnCleanExit(); + Mockito.verify(mock).forceUserDefinedCompaction(ssTable); + } + + @Test + public void testCompactSSTablesSubcommandRequiresAtLeastOneFile() throws Throwable + { + invokeNodetool("compact", "sstables") + .asserts() + .failure(); + } + + // compact range subcommand tests + + @Test + public void testCompactRangeSubcommandWithBothTokens() throws Throwable + { + long key = 34; + String startToken = Long.toString(key - 1); + String endToken = Long.toString(key + 1); + String table = "table"; + StorageServiceMBean mock = getMock(STORAGE_SERVICE_MBEAN); + when(mock.getKeyspaces()).thenReturn(List.of(keyspace())); + when(mock.getNonSystemKeyspaces()).thenReturn(List.of(keyspace())); + invokeNodetool("compact", "range", "--start-token", startToken, "--end-token", endToken, keyspace(), table).assertOnCleanExit(); + Mockito.verify(mock).forceKeyspaceCompactionForTokenRange(keyspace(), startToken, endToken, table); + } + + @Test + public void testCompactRangeSubcommandWithStartTokenOnly() throws Throwable + { + long key = 34; + String startToken = Long.toString(key - 1); + String table = "table"; + StorageServiceMBean mock = getMock(STORAGE_SERVICE_MBEAN); + when(mock.getKeyspaces()).thenReturn(List.of(keyspace())); + when(mock.getNonSystemKeyspaces()).thenReturn(List.of(keyspace())); + invokeNodetool("compact", "range", "--start-token", startToken, keyspace(), table).assertOnCleanExit(); + Mockito.verify(mock).forceKeyspaceCompactionForTokenRange(keyspace(), startToken, "", table); + } + + @Test + public void testCompactRangeSubcommandWithEndTokenOnly() throws Throwable + { + long key = 34; + String endToken = Long.toString(key + 1); + String table = "table"; + StorageServiceMBean mock = getMock(STORAGE_SERVICE_MBEAN); + when(mock.getKeyspaces()).thenReturn(List.of(keyspace())); + when(mock.getNonSystemKeyspaces()).thenReturn(List.of(keyspace())); + invokeNodetool("compact", "range", "--end-token", endToken, keyspace(), table).assertOnCleanExit(); + Mockito.verify(mock).forceKeyspaceCompactionForTokenRange(keyspace(), "", endToken, table); + } + + @Test + public void testCompactRangeSubcommandRequiresAtLeastOneToken() throws Throwable + { + String table = "table"; + StorageServiceMBean mock = getMock(STORAGE_SERVICE_MBEAN); + when(mock.getKeyspaces()).thenReturn(List.of(keyspace())); + when(mock.getNonSystemKeyspaces()).thenReturn(List.of(keyspace())); + invokeNodetool("compact", "range", keyspace(), table) + .asserts() + .failure(); + } }