Skip to content

CASSANDRA-20876: Offline nodetool commands should not print network options in help #4871

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.

Sign up for GitHub

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

Open
arvindKandpal-ksolves wants to merge 1 commit into
base: trunk
Choose a base branch
from
+31 −21
Open

CASSANDRA-20876: Offline nodetool commands should not print network options in help #4871

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 17 additions & 0 deletions src/java/org/apache/cassandra/tools/nodetool/AbstractCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,23 @@ protected boolean shouldConnect() throws ExecutionException
return true;
}

/**
* Check if the command is designed to run locally without a JMX connection.
* @return {@code true} if it is an offline command, {@code false} otherwise.
*/
public boolean isOfflineCommand()
{
try
{
return !shouldConnect();

@Mmuzaf Mmuzaf Jun 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@arvindKandpal-ksolves The problem with the current approach is exactly that isOfflineCommand() calls shouldConnect(), which is an execution-flow method, intentionally runtim-overridable and side-effecting (that's why it declares throws ExecutionException). See the Sjk (where the 'locality' is runtime dependent) command and check the test NodetoolHelpCommandsOutputTest.

As for the history command we know in advance that it is local, and this knowledge should be a part of command metdata.

So the right fix is to make this knowledge to be a part of command hierary: so either create a LocalCommand marker interface, or AbstractLocalCommand extends AbstractCommand. Then in the Layout class, check whether the command is local by examining the class it extends.

}
catch (Exception e)
{
// Safe fallback: default to online if evaluation fails
return false;
}
}

/**
* Execute the command using the supplied {@link NodeProbe} instance, which is already connected
* to the node and ready to use. If the {@link NodeProbe} is not {@code null}, it is guaranteed that it
Expand Down
14 changes: 13 additions & 1 deletion src/java/org/apache/cassandra/tools/nodetool/layout/CassandraCliHelpLayout.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

import org.apache.commons.lang3.StringUtils;

import org.apache.cassandra.tools.nodetool.AbstractCommand;
import org.apache.cassandra.tools.nodetool.CommandUtils;
import org.apache.cassandra.tools.nodetool.JmxConnect;
import org.apache.cassandra.tools.nodetool.NodetoolCommand;
Expand Down Expand Up @@ -257,6 +258,12 @@ private static List<CommandLine.Model.OptionSpec> parentCommandOptionsWithJmxOpt
if (commandSpec.helpCommand())
return Collections.emptyList();

boolean shouldShowJmx = true;
if (commandSpec.userObject() instanceof AbstractCommand)
{
shouldShowJmx = !((AbstractCommand) commandSpec.userObject()).isOfflineCommand();
}

List<CommandLine.Model.CommandSpec> hierarhy = new LinkedList<>();
CommandLine.Model.CommandSpec curr;
CommandLine.Model.CommandSpec command = commandSpec;
Expand All @@ -271,11 +278,16 @@ private static List<CommandLine.Model.OptionSpec> parentCommandOptionsWithJmxOpt
{
for (CommandLine.Model.OptionSpec option : spec.options())
{
// JmxConnect and NodetoolCommand options are always shown in the help output for backwards compatibility.
// JmxConnect options are shown for online commands only; NodetoolCommand options are always shown for backwards compatibility.
if (option.userObject() instanceof Field &&
(((Field) option.userObject()).getDeclaringClass().equals(JmxConnect.class) ||
((Field) option.userObject()).getDeclaringClass().equals(NodetoolCommand.class)))
{
if (((Field) option.userObject()).getDeclaringClass().equals(JmxConnect.class) && !shouldShowJmx)
continue;

options.add(option);
}
else
{
if (option.hidden() || option.scopeType() == CommandLine.ScopeType.LOCAL)
Expand Down
21 changes: 1 addition & 20 deletions test/resources/nodetool/help/history
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,8 @@ NAME
nodetool history - Print previously executed nodetool commands

SYNOPSIS
nodetool [(-h <host> | --host <host>)] [(-p <port> | --port <port>)]
[(-pw <password> | --password <password>)]
[(-pwf <passwordFilePath> | --password-file <passwordFilePath>)]
[(-u <username> | --username <username>)] history
[(-n <commands> | --number-of-commands <commands>)]
nodetool history [(-n <commands> | --number-of-commands <commands>)]

OPTIONS
-h <host>, --host <host>
Node hostname or ip address

-n <commands>, --num <commands>, --number-of-commands <commands>
Number of commands to print, defaults to 1000.

-p <port>, --port <port>
Remote jmx agent port number

-pw <password>, --password <password>
Remote jmx agent password

-pwf <passwordFilePath>, --password-file <passwordFilePath>
Path to the JMX password file

-u <username>, --username <username>
Remote jmx agent username