Skip to content

Commit

Permalink
ccmlib/scylla_node.py: use native scylla nodetool when it's available
Browse files Browse the repository at this point in the history
in b18f85d, we use the native nodetool if JMX is not available, but
when running dtest from the local build, the jmx repo is still around,
so the java-based nodetool is picked. but the java-based nodetool does
not support tablets, and it would be great to always test the native
implementation built locally when testing using dtest without building
a relocatable package.

so, in this change, we go a step further by checking if the command line
of `scylla nodetool help` works, if it succeeds, we go ahead and use
`scylla nodetool`, otherwise, we fall back to the java-based nodetool.
we could instead use `scylla nodetool --help` instead of calling a
certain command of nodetool, but seastar returns 1 if `--help` is
used. a pull request was created to address it . see
scylladb/seastar#2213.

also, the native nodetool expect `-h` and `--port` as parameters of
the subcommand, not the parameters of `scylla nodetool`, it complains
like:

```
E               ccmlib.node.ToolError: Subprocess /jenkins/workspace/scylla-master/gating-dtest-release/scylla/.dtest/dtest-7r2n368l/test/node1/bin/scylla nodetool -h 127.0.17.1 -p 10000 snapshot exited with non-zero status; exit status: 100;
E               stderr: error: unrecognized operation argument: expected one of ({cleanup, compact, disablebackup, disablebinary, disablegossip, enablebackup, enablebinary, enablegossip, flush, gettraceprobability, help, settraceprobability, statusbackup, statusbinary, statusgossip, version}), got --host
```

but the java-based tool looks for these options before the subcommand,
so put them conditionally.

Signed-off-by: Kefu Chai <[email protected]>
  • Loading branch information
tchaikov committed May 1, 2024
1 parent 2aa2ad8 commit c7c5fa1
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 4 deletions.
13 changes: 11 additions & 2 deletions ccmlib/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -858,8 +858,17 @@ def nodetool(self, cmd, capture_output=True, wait=True, timeout=None, verbose=Tr
if not isinstance(nodetool, list):
nodetool = [nodetool]
# see https://www.oracle.com/java/technologies/javase/8u331-relnotes.html#JDK-8278972
nodetool.extend(['-h', host, '-p', str(self.jmx_port), '-Dcom.sun.jndi.rmiURLParsing=legacy'])
nodetool.extend(cmd.split())
args = ['-h', host, '-p', str(self.jmx_port), '-Dcom.sun.jndi.rmiURLParsing=legacy']
if len(nodetool) > 1:
nodetool.extend(cmd.split())
# put args at the end of command line options, as "scylla nodetool"
# expects them as options of the subcommand
nodetool.extend(args)
else:
# while java-based tool considers them as options of the nodetool
# itself
nodetool.extend(args)
nodetool.extend(cmd.split())

return self._do_run_nodetool(nodetool, capture_output, wait, timeout, verbose)

Expand Down
11 changes: 9 additions & 2 deletions ccmlib/scylla_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -776,16 +776,23 @@ def _update_jmx_pid(self, wait=True):
self.jmx_pid = None

def nodetool(self, cmd, capture_output=True, wait=True, timeout=None, verbose=True):
if not self.has_jmx:
try:
# nodetool subcommand was added in 5.5, so check if it is available
# before using it.
nodetool = [os.path.join(self.get_bin_dir(), "scylla"), "nodetool"]
subprocess.check_call(nodetool + ["help"], stdout=subprocess.DEVNULL, stderr=subprocess.STDOUT)
if self.is_docker():
host = 'localhost'
else:
host = self.address()
nodetool = [os.path.join(self.get_bin_dir(), "scylla"), "nodetool"]
nodetool.extend(['-h', host, '-p', '10000'])
nodetool.extend(cmd.split())
return self._do_run_nodetool(nodetool, capture_output, wait, timeout, verbose)
except subprocess.CalledProcessError:
pass

# the java nodetool depends on JMX
assert self.has_jmx
# Fall-back to the java nodetool for pre 5.5.0~dev versions, which don't yet have the native nodetool
# Kill scylla-jmx in case of timeout, to supply enough debugging information

Expand Down

0 comments on commit c7c5fa1

Please sign in to comment.