Skip to content

Commit c9fe399

Browse files
MmuzafMmuzaf
committed
Move -pp/--print-port from NodetoolCommand to per-command Mixin
Patch by Maxim Muzafarov; reviewed by Stefan Miklosovic for CASSANDRA-20790
1 parent ce983cb commit c9fe399

File tree

209 files changed

+703
-841
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

209 files changed

+703
-841
lines changed

‎src/java/org/apache/cassandra/tools/NodeTool.java‎

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,10 @@
2323
import java.lang.reflect.Field;
2424
import java.text.SimpleDateFormat;
2525
import java.util.ArrayList;
26+
import java.util.Arrays;
2627
import java.util.Date;
2728
import java.util.List;
29+
import java.util.Set;
2830

2931
import javax.inject.Inject;
3032
import javax.management.InstanceNotFoundException;
@@ -46,6 +48,8 @@
4648

4749
import static com.google.common.base.Throwables.getStackTraceAsString;
4850
import static org.apache.cassandra.io.util.File.WriteMode.APPEND;
51+
import static org.apache.cassandra.tools.nodetool.PrintPortMixin.PRINT_PORT_LONG;
52+
import static org.apache.cassandra.tools.nodetool.PrintPortMixin.PRINT_PORT_SHORT;
4953
import static org.apache.cassandra.utils.LocalizeString.toUpperCaseLocalized;
5054

5155
public class NodeTool
@@ -57,6 +61,20 @@ public class NodeTool
5761

5862
private static final String HISTORYFILE = "nodetool.history";
5963

64+
/**
65+
* Set of subcommand names that accept the {@code -pp/--print-port} options.
66+
* These are the commands that declare the option via @Mixin and thus require
67+
* relocating the option for backward compatibility in order to be recognized
68+
* by picocli when specified before the subcommand name.
69+
* <p>
70+
* Both calls such as {@code ./nodetool --print-port status}, and
71+
* {@code ./nodetool status --print-port} should work as expected.
72+
*/
73+
private static final Set<String> COMMANDS_SUPPORTING_PRINT_PORT_OPTION = Set.of("status", "ring", "netstats",
74+
"gossipinfo", "getendpoints",
75+
"failuredetector", "describering",
76+
"describecluster");
77+
6078
private final INodeProbeFactory nodeProbeFactory;
6179
private final Output output;
6280

@@ -112,7 +130,7 @@ public int execute(String... args)
112130
.setPosixClusteredShortOptionsAllowed(false);
113131

114132
printHistory(args);
115-
return commandLine.execute(args);
133+
return commandLine.execute(relocatePrintPortOptionsForBackwardCompatibility(args));
116134
}
117135
catch (ConfigurationException e)
118136
{
@@ -220,6 +238,66 @@ protected void err(Throwable e)
220238
output.err.println(getStackTraceAsString(e));
221239
}
222240

241+
/**
242+
* Rewrites global {@code -pp/--print-port} options that have been moved
243+
* to subcommands via @Mixin for backward compatibility. When a user types:
244+
* <pre>
245+
* nodetool -pp status
246+
* nodetool --print-port status -r
247+
* </pre>
248+
* this method rewrites them to:
249+
* <pre>
250+
* nodetool status -pp
251+
* nodetool status -r --print-port
252+
* </pre>
253+
* so that picocli assigns the option to the subcommand that declares it.
254+
* <p>
255+
* Options that appear after the subcommand name are left untouched:
256+
* <pre>
257+
* nodetool status -pp -> unchanged
258+
* nodetool status -pp -r -> unchanged
259+
* </pre>
260+
*/
261+
static String[] relocatePrintPortOptionsForBackwardCompatibility(String[] args)
262+
{
263+
if (args == null || args.length < 2)
264+
return args;
265+
266+
Set<String> relocatable = Set.of(PRINT_PORT_SHORT, PRINT_PORT_LONG);
267+
268+
int subcommandIdx = -1;
269+
for (int i = 0; i < args.length; i++)
270+
{
271+
if (COMMANDS_SUPPORTING_PRINT_PORT_OPTION.contains(args[i]))
272+
{
273+
subcommandIdx = i;
274+
break;
275+
}
276+
}
277+
278+
if (subcommandIdx < 0)
279+
return args;
280+
281+
List<String> before = new ArrayList<>();
282+
List<String> toRelocate = new ArrayList<>();
283+
for (int i = 0; i < subcommandIdx; i++)
284+
{
285+
if (relocatable.contains(args[i]))
286+
toRelocate.add(args[i]);
287+
else
288+
before.add(args[i]);
289+
}
290+
291+
if (toRelocate.isEmpty())
292+
return args;
293+
294+
List<String> result = new ArrayList<>(before);
295+
result.addAll(Arrays.asList(args).subList(subcommandIdx, args.length));
296+
result.addAll(toRelocate);
297+
298+
return result.toArray(new String[0]);
299+
}
300+
223301
private enum CliLayout
224302
{
225303
AIRLINE,

‎src/java/org/apache/cassandra/tools/nodetool/DescribeCluster.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,18 @@
3131
import org.apache.cassandra.tools.NodeProbe;
3232

3333
import picocli.CommandLine.Command;
34+
import picocli.CommandLine.Mixin;
3435

3536
@Command(name = "describecluster", description = "Print the name, snitch, partitioner and schema version of a cluster")
36-
public class DescribeCluster extends WithPortDisplayAbstractCommand
37+
public class DescribeCluster extends AbstractCommand
3738
{
3839
private boolean resolveIp = false;
3940
private String keyspace = null;
4041
private Collection<String> joiningNodes, leavingNodes, movingNodes, liveNodes, unreachableNodes;
4142

43+
@Mixin
44+
private PrintPortMixin printPortMixin = new PrintPortMixin();
45+
4246
@Override
4347
public void execute(NodeProbe probe)
4448
{
@@ -63,7 +67,7 @@ public void execute(NodeProbe probe)
6367

6468
// display schema version for each node
6569
out.println("\tSchema versions:");
66-
Map<String, List<String>> schemaVersions = printPort ? probe.getSpProxy().getSchemaVersionsWithPort() : probe.getSpProxy().getSchemaVersions();
70+
Map<String, List<String>> schemaVersions = printPortMixin.printPort ? probe.getSpProxy().getSchemaVersionsWithPort() : probe.getSpProxy().getSchemaVersions();
6771
for (Map.Entry<String, List<String>> entry : schemaVersions.entrySet())
6872
{
6973
out.printf("\t\t%s: %s%n%n", entry.getKey(), entry.getValue());

‎src/java/org/apache/cassandra/tools/nodetool/DescribeRing.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,20 @@
2323
import org.apache.cassandra.tools.NodeProbe;
2424

2525
import picocli.CommandLine.Command;
26+
import picocli.CommandLine.Mixin;
2627
import picocli.CommandLine.Parameters;
2728

2829
import static org.apache.commons.lang3.StringUtils.EMPTY;
2930

3031
@Command(name = "describering", description = "Shows the token ranges info of a given keyspace")
31-
public class DescribeRing extends WithPortDisplayAbstractCommand
32+
public class DescribeRing extends AbstractCommand
3233
{
3334
@Parameters(description = "The keyspace name", arity = "1")
3435
private String keyspace = EMPTY;
3536

37+
@Mixin
38+
private PrintPortMixin printPortMixin = new PrintPortMixin();
39+
3640
@Override
3741
public void execute(NodeProbe probe)
3842
{
@@ -41,7 +45,7 @@ public void execute(NodeProbe probe)
4145
out.println("TokenRange: ");
4246
try
4347
{
44-
for (String tokenRangeString : probe.describeRing(keyspace, printPort))
48+
for (String tokenRangeString : probe.describeRing(keyspace, printPortMixin.printPort))
4549
{
4650
out.println("\t" + tokenRangeString);
4751
}

‎src/java/org/apache/cassandra/tools/nodetool/FailureDetectorInfo.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,18 @@
2525
import org.apache.cassandra.tools.NodeProbe;
2626

2727
import picocli.CommandLine.Command;
28+
import picocli.CommandLine.Mixin;
2829

2930
@Command(name = "failuredetector", description = "Shows the failure detector information for the cluster")
30-
public class FailureDetectorInfo extends WithPortDisplayAbstractCommand
31+
public class FailureDetectorInfo extends AbstractCommand
3132
{
33+
@Mixin
34+
private PrintPortMixin printPortMixin = new PrintPortMixin();
35+
3236
@Override
3337
public void execute(NodeProbe probe)
3438
{
35-
TabularData data = probe.getFailureDetectorPhilValues(printPort);
39+
TabularData data = probe.getFailureDetectorPhilValues(printPortMixin.printPort);
3640
probe.output().out.printf("%10s,%16s%n", "Endpoint", "Phi");
3741
for (Object o : data.keySet())
3842
{

‎src/java/org/apache/cassandra/tools/nodetool/GetEndpoints.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,14 @@
2525
import org.apache.cassandra.tools.nodetool.layout.CassandraUsage;
2626

2727
import picocli.CommandLine.Command;
28+
import picocli.CommandLine.Mixin;
2829
import picocli.CommandLine.Parameters;
2930

3031
import static com.google.common.base.Preconditions.checkArgument;
3132
import static org.apache.cassandra.tools.nodetool.CommandUtils.concatArgs;
3233

3334
@Command(name = "getendpoints", description = "Print the end points that owns the key")
34-
public class GetEndpoints extends WithPortDisplayAbstractCommand
35+
public class GetEndpoints extends AbstractCommand
3536
{
3637
@CassandraUsage(usage = "<keyspace> <table> <key>", description = "The keyspace, the table, and the partition key for which we need to find the endpoint")
3738
private List<String> args = new ArrayList<>();
@@ -45,6 +46,9 @@ public class GetEndpoints extends WithPortDisplayAbstractCommand
4546
@Parameters(index = "2", arity = "0..1", description = "The partition key for which we need to find the endpoint")
4647
private String key;
4748

49+
@Mixin
50+
private PrintPortMixin printPortMixin = new PrintPortMixin();
51+
4852
@Override
4953
public void execute(NodeProbe probe)
5054
{
@@ -55,7 +59,7 @@ public void execute(NodeProbe probe)
5559
String table = args.get(1);
5660
String key = args.get(2);
5761

58-
if (printPort)
62+
if (printPortMixin.printPort)
5963
{
6064
for (String endpoint : probe.getEndpointsWithPort(ks, table, key))
6165
{

‎src/java/org/apache/cassandra/tools/nodetool/GossipInfo.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,21 @@
2020
import org.apache.cassandra.tools.NodeProbe;
2121

2222
import picocli.CommandLine.Command;
23+
import picocli.CommandLine.Mixin;
2324
import picocli.CommandLine.Option;
2425

2526
@Command(name = "gossipinfo", description = "Shows the gossip information for the cluster")
26-
public class GossipInfo extends WithPortDisplayAbstractCommand
27+
public class GossipInfo extends AbstractCommand
2728
{
2829
@Option(paramLabel = "resolve_ip", names = { "-r", "--resolve-ip" }, description = "Show node domain names instead of IPs")
2930
private boolean resolveIp = false;
3031

32+
@Mixin
33+
private PrintPortMixin printPortMixin = new PrintPortMixin();
34+
3135
@Override
3236
public void execute(NodeProbe probe)
3337
{
34-
probe.output().out.println(probe.getGossipInfo(printPort, resolveIp));
38+
probe.output().out.println(probe.getGossipInfo(printPortMixin.printPort, resolveIp));
3539
}
3640
}

‎src/java/org/apache/cassandra/tools/nodetool/NetStats.java‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,16 +31,20 @@
3131
import org.apache.cassandra.tools.NodeProbe;
3232

3333
import picocli.CommandLine.Command;
34+
import picocli.CommandLine.Mixin;
3435
import picocli.CommandLine.Option;
3536

3637
@Command(name = "netstats", description = "Print network information on provided host (connecting node by default)")
37-
public class NetStats extends WithPortDisplayAbstractCommand
38+
public class NetStats extends AbstractCommand
3839
{
3940
@Option(paramLabel = "human_readable",
4041
names = { "-H", "--human-readable" },
4142
description = "Display bytes in human readable form, i.e. KiB, MiB, GiB, TiB")
4243
private boolean humanReadable = false;
4344

45+
@Mixin
46+
private PrintPortMixin printPortMixin = new PrintPortMixin();
47+
4448
@Override
4549
public void execute(NodeProbe probe)
4650
{
@@ -54,11 +58,11 @@ public void execute(NodeProbe probe)
5458
out.printf("%s %s%n", status.streamOperation.getDescription(), status.planId.toString());
5559
for (SessionInfo info : status.sessions)
5660
{
57-
out.printf(" %s", InetAddressAndPort.toString(info.peer, printPort));
61+
out.printf(" %s", InetAddressAndPort.toString(info.peer, printPortMixin.printPort));
5862
// print private IP when it is used
5963
if (!info.peer.equals(info.connecting))
6064
{
61-
out.printf(" (using %s)", InetAddressAndPort.toString(info.connecting, printPort));
65+
out.printf(" (using %s)", InetAddressAndPort.toString(info.connecting, printPortMixin.printPort));
6266
}
6367
out.printf("%n");
6468
if (!info.receivingSummaries.isEmpty())
@@ -142,7 +146,7 @@ public void printReceivingSummaries(PrintStream out, SessionInfo info, boolean p
142146

143147
for (ProgressInfo progress : info.getReceivingFiles())
144148
{
145-
out.printf(" %s%n", progress.toString(printPort));
149+
out.printf(" %s%n", progress.toString(printPortMixin.printPort));
146150
}
147151
}
148152

@@ -166,7 +170,7 @@ public void printSendingSummaries(PrintStream out, SessionInfo info, boolean pri
166170

167171
for (ProgressInfo progress : info.getSendingFiles())
168172
{
169-
out.printf(" %s%n", progress.toString(printPort));
173+
out.printf(" %s%n", progress.toString(printPortMixin.printPort));
170174
}
171175
}
172176
}

‎src/java/org/apache/cassandra/tools/nodetool/NodetoolCommand.java‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020

2121
import picocli.CommandLine.Command;
2222
import picocli.CommandLine.Model.CommandSpec;
23-
import picocli.CommandLine.Option;
2423
import picocli.CommandLine.Spec;
2524

2625
import static org.apache.cassandra.tools.nodetool.Help.printTopCommandUsage;
@@ -61,6 +60,7 @@
6160
AccordAdmin.class,
6261
AlterTopology.class,
6362
Assassinate.class,
63+
AsyncProfileCommandGroup.class,
6464
AutoRepairStatus.class,
6565
Bootstrap.class,
6666
CIDRFilteringStats.class,
@@ -71,6 +71,7 @@
7171
Compact.class,
7272
CompactionHistory.class,
7373
CompactionStats.class,
74+
CompressionDictionaryCommandGroup.class,
7475
ConsensusMigrationAdmin.class,
7576
DataPaths.class,
7677
Decommission.Abort.class,
@@ -149,7 +150,6 @@
149150
NetStats.class,
150151
PauseHandoff.class,
151152
ProfileLoad.class,
152-
AsyncProfileCommandGroup.class,
153153
ProxyHistograms.class,
154154
RangeKeySample.class,
155155
Rebuild.class,
@@ -208,7 +208,6 @@
208208
TableStats.class,
209209
TopPartitions.class,
210210
TpStats.class,
211-
CompressionDictionaryCommandGroup.class,
212211
TruncateHints.class,
213212
UpdateCIDRGroup.class,
214213
UpgradeSSTable.class,
@@ -220,12 +219,6 @@ public class NodetoolCommand implements Runnable
220219
@Spec
221220
public CommandSpec spec;
222221

223-
// TODO CASSANDRA-20790 this option is used only in several commands and should not be the global option.
224-
// It should be pushed down to specific commands to clean up the global hierarchy, while maintaining backwards compatibility.
225-
// Calls such as './nodetool --print-port subcommand', and './nodetool subcommand --print-port' should work as expected.
226-
@Option(names = { "-pp", "--print-port" }, description = "Operate in 4.0 mode with hosts disambiguated by port number")
227-
public boolean printPort = false;
228-
229222
public void run()
230223
{
231224
printTopCommandUsage(spec.commandLine(),

src/java/org/apache/cassandra/tools/nodetool/WithPortDisplayAbstractCommand.java renamed to src/java/org/apache/cassandra/tools/nodetool/PrintPortMixin.java

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,23 +18,14 @@
1818

1919
package org.apache.cassandra.tools.nodetool;
2020

21-
import picocli.CommandLine.ParentCommand;
21+
import picocli.CommandLine;
2222

23-
/**
24-
* Abstract class for commands that display endpoints that can be disambiguated by port number. (e.g. gossipinfo, describecluster etc.).
25-
*/
26-
abstract class WithPortDisplayAbstractCommand extends AbstractCommand
23+
public class PrintPortMixin
2724
{
28-
@ParentCommand
29-
private NodetoolCommand parent;
30-
31-
/** See {@link NodetoolCommand#printPort} option. */
32-
protected boolean printPort;
25+
public static final String PRINT_PORT_SHORT = "-pp";
26+
public static final String PRINT_PORT_LONG = "--print-port";
3327

34-
@Override
35-
protected boolean shouldConnect()
36-
{
37-
printPort = parent.printPort;
38-
return true;
39-
}
28+
@CommandLine.Option(names = { PRINT_PORT_SHORT, PRINT_PORT_LONG },
29+
description = "Operate in 4.0 mode with hosts disambiguated by port number")
30+
public boolean printPort = false;
4031
}

0 commit comments

Comments
 (0)