-
Notifications
You must be signed in to change notification settings - Fork 4k
CASSANDRA-20793: Split nodetool compact into keyspace, sstables, and range subcommands #4922
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 = "[<keyspace> <tables>...] or <SSTable file>...", | ||
|
|
@@ -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) | |
| } | ||
| } | ||
| } | ||
|
|
||
|
Comment on lines
41
to
+131
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
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?