Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
159 changes: 157 additions & 2 deletions src/java/org/apache/cassandra/tools/nodetool/Compact.java
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've set the deprecated version to 7.0 for now. Could you please confirm if this is the correct version or if it should be 5.x?

@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 = "[<keyspace> <tables>...] or <SSTable file>...",
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -118,4 +128,149 @@ else if (partitionKeyProvided)
}
}
}

Comment on lines 41 to +131

@arvindKandpal-ksolves arvindKandpal-ksolves Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see a backward compatibility edge case here. If a user has a keyspace actually named keyspace or range, running the legacy command like nodetool compact keyspace mytable will trigger the new subcommand instead of treating it as the keyspace name. The same issue would happen if a file is named sstables.

Any suggestions on what's the standard way in the codebase to handle this kind of picocli name collision? Should I add some custom check before picocli parses the arguments?

/**
* 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 = "[<keyspace> <tables>...]",
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<String> 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<String> args = concatArgs(keyspaceName, tableNames);
List<String> 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 = "<SSTable file>...",
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<String> 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 = "[<keyspace> <tables>...]",
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<String> 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<String> args = concatArgs(keyspaceName, tableNames);
List<String> 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);
}
}
}
}
}
106 changes: 106 additions & 0 deletions test/unit/org/apache/cassandra/tools/nodetool/CompactTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -38,6 +39,8 @@ public static void setup() throws Throwable
startJMXServer();
}

// Deprecated compact command tests (backward compatibility)

@Test
public void keyPresent() throws Throwable
{
Expand Down Expand Up @@ -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();
}
}
Loading