[
https://issues.apache.org/jira/browse/CASSANDRA-20793?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18113726#comment-18113726
]
Arvind Kandpal edited comment on CASSANDRA-20793 at 9/10/26 8:52 AM:
---------------------------------------------------------------------
+*Option (a): Keep subcommands + add pre-parse fallback*+
*How it works:*
1. Add a method in NodeTool.java (like
relocateCompactArgsForBackwardCompatibility()) that checks:
- If first arg after compact is keyspace, sstables, or range
- AND it looks like legacy usage (e.g., followed by args that look like
keyspace names, not subcommand options)
- Then insert --legacy flag (or similar) to force routing to the deprecated
handler
2. Compact.java checks for the flag and routes to legacy execute() instead of
subcommand
*Pros:*
- Minimal change to already-written code
- Follows existing NodeTool.java precedent (relocatePrintPortOptions...)
- Tests mostly stay as-is
*Cons:*
- Heuristic-based — hard to distinguish compact keyspace
<actual-keyspace-named-"keyspace"> from compact keyspace <any-other-keyspace>
without checking if the keyspace exists
(expensive JMX call during parsing)
- Fragile: what if someone has a file literally named sstables? The heuristic
might fail
- Doesn't address reviewer's suggestion about combining table + range
- Still doesn't follow the most common Cassandra nodetool pattern (positional
params)
+*Option (b): Replace subcommands with flags*+
How it works:
Redesign Compact to be a single command (no subcommands) with:
{code:java}
@Option(names = {"--keyspace", "-ks"}) — target keyspace (optional, defaults to
all non-system)
@Option(names = {"--table", "-t"}) — one or more tables (repeatable)
@Option(names = {"--sstables"}) — list of SSTable files (mutually exclusive
with keyspace/table)
@Option(names = {"--start-token", "-st"}) + @Option(names = {"--end-token",
"-et"}) — token range
@Option(names = {"--partition", "-p"}) — single partition key (mutually
exclusive with token range)
@Option(names = {"--key", "-k"}) (NEW) — convenience for --partition
@Parameters (positional args) for backward-compatible legacy parsing: if no
flags given and positional args present, parse as [keyspace] [tables...] like
the old command
{code}
*Example new usage:*
{code:java}
nodetool compact --keyspace mykeyspace --table mytable
nodetool compact --keyspace mykeyspace --table mytable --start-token 100
--end-token 200
nodetool compact --sstables /path/to/file1 /path/to/file2
nodetool compact --keyspace mykeyspace --key 42 # new convenience{code}
*Example legacy usage (still works):*
{code:java}
nodetool compact mykeyspace mytable # positional, no flags {code}
*Pros:*
- Eliminates collision entirely — no ambiguity between subcommand names and
keyspace names
- Allows table + range combination: --keyspace ks --table t1 --start-token 100
--end-token 200
- Adds --key convenience for single-key compaction (large-tombstone use case)
- Still backward-compatible via positional arg fallback
- Some precedent exists (--keyspace in Rebuild, ListSnapshots)
- Larger change — need to rewrite Compact.java significantly
- ALL tests need updating to new flag-based syntax (or keep legacy positional
tests)
- Goes against the dominant Cassandra pattern (most commands use positional
params, not flags)
- Deprecation path becomes awkward: positional args work but aren't the "new"
way, yet flags are the new way — which is "correct"?
was (Author: JIRAUSER311141):
+*Option (a): Keep subcommands + add pre-parse fallback*+
*How it works:*
1. Add a method in NodeTool.java (like
relocateCompactArgsForBackwardCompatibility()) that checks:
- If first arg after compact is keyspace, sstables, or range
- AND it looks like legacy usage (e.g., followed by args that look like
keyspace names, not subcommand options)
- Then insert --legacy flag (or similar) to force routing to the deprecated
handler
2. Compact.java checks for the flag and routes to legacy execute() instead of
subcommand
*Pros:*
- Minimal change to already-written code
- Follows existing NodeTool.java precedent (relocatePrintPortOptions...)
- Tests mostly stay as-is
*Cons:*
- Heuristic-based — hard to distinguish compact keyspace
<actual-keyspace-named-"keyspace"> from compact keyspace <any-other-keyspace>
without checking if the keyspace exists
(expensive JMX call during parsing)
- Fragile: what if someone has a file literally named sstables? The heuristic
might fail
- Doesn't address reviewer's suggestion about combining table + range
- Still doesn't follow the most common Cassandra nodetool pattern (positional
params)
+*Option (b): Replace subcommands with flags*+
How it works:
Redesign Compact to be a single command (no subcommands) with:
{code:java}
@Option(names = {"--keyspace", "-ks"}) — target keyspace (optional, defaults to
all non-system)
@Option(names = {"--table", "-t"}) — one or more tables (repeatable)
@Option(names = {"--sstables"}) — list of SSTable files (mutually exclusive
with keyspace/table)
@Option(names = {"--start-token", "-st"}) + @Option(names = {"--end-token",
"-et"}) — token range
@Option(names = {"--partition", "-p"}) — single partition key (mutually
exclusive with token range)
@Option(names = {"--key", "-k"}) (NEW) — convenience for --partition
@Parameters (positional args) for backward-compatible legacy parsing: if no
flags given and positional args present, parse as [keyspace] [tables...] like
the old command
{code}
*Example new usage:*
{code:java}
nodetool compact --keyspace mykeyspace --table mytable
nodetool compact --keyspace mykeyspace --table mytable --start-token 100
--end-token 200
nodetool compact --sstables /path/to/file1 /path/to/file2
nodetool compact --keyspace mykeyspace --key 42 # new convenience{code}
*Example legacy usage (still works):*
{code:java}
nodetool compact mykeyspace mytable # positional, no flags {code}
*Pros:*
- Eliminates collision entirely — no ambiguity between subcommand names and
keyspace names
- Allows table + range combination as reviewer requested: --keyspace ks
--table t1 --start-token 100 --end-token 200
- Adds --key convenience for single-key compaction (large-tombstone use case
reviewer mentioned)
- Still backward-compatible via positional arg fallback
- Some precedent exists (--keyspace in Rebuild, ListSnapshots)
- Larger change — need to rewrite Compact.java significantly
- ALL tests need updating to new flag-based syntax (or keep legacy positional
tests)
- Goes against the dominant Cassandra pattern (most commands use positional
params, not flags)
- Deprecation path becomes awkward: positional args work but aren't the "new"
way, yet flags are the new way — which is "correct"?
> Improve compact nodetool command to comply with standards by splitting into
> separate subcommands
> ------------------------------------------------------------------------------------------------
>
> Key: CASSANDRA-20793
> URL: https://issues.apache.org/jira/browse/CASSANDRA-20793
> Project: Apache Cassandra
> Issue Type: Improvement
> Components: Tool/nodetool
> Reporter: Maxim Muzafarov
> Assignee: Arvind Kandpal
> Priority: Normal
> Time Spent: 1h 50m
> Remaining Estimate: 0h
>
> The current compact command violates picocli best practices by mixing
> different types of arguments in a single command. The command currently
> accepts three distinct types of input:
> # Keyspace/Table compaction: [<keyspace> <tables>...] - for forcing major
> compaction on specific keyspaces and tables
> # SSTable file compaction: <SSTable file>... - for user-defined compaction
> on specific SSTable files
> # Token range compaction: Using --start-token and --end-token options for
> token-based compaction
> Problems with current implementation:
> * The command handles three completely different compaction scenarios
> * Users must understand complex conditional logic to determine which mode to
> use
> * Other nodetool commands use clear subcommands for different operations
> * The execute() method contains complex conditional checks that should be
> handled by picocli's argument validation
> * The usage description "[<keyspace> <tables>...] or <SSTable file>..." is
> confusing
> h3. Proposed Solution
> Split the compact command into three separate subcommands under a main
> compact command:
> # compact keyspace - For keyspace/table compaction
> # compact sstables - For user-defined SSTable compaction
> # compact range - For token range compaction
> All the changes must be backward compatible, this mean the compact command
> must be deprecated first.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
