-
Notifications
You must be signed in to change notification settings - Fork 68
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
ccmlib/scylla_node.py: use native scylla nodetool when it's available #573
Conversation
That reminds me of the functionality of the wrapper script we've had. Why not use it? |
cause it's not there anymore ? |
cf7315f
to
7e44d85
Compare
@mykaul hi Yaniv, as @fruch put, it was dropped. see https://github.com/scylladb/scylla-tools-java/pull/386/files |
This comment was marked as resolved.
This comment was marked as resolved.
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]>
7e44d85
to
c7c5fa1
Compare
it's a resurrection of #572. which was queued but then dequeued due to a regression. but the regression was addressed by scylladb/scylladb#18470. |
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.
LGTM
@tchaikov please retest all next_gating tests and resubmit |
i am actually testing it at https://jenkins.scylladb.com/job/scylla-staging/job/tchaikov/job/gating-dtest-release/16/. but some tests are failing even without this patch. see https://jenkins.scylladb.com/job/scylla-staging/job/tchaikov/job/gating-dtest-release/17/ . still looking. |
Apparently it's due to the https://github.com/scylladb/scylla-dtest/pull/4186 fiasco. I think that the reason test passes now in next-gating is: https://github.com/scylladb/scylla-dtest/pull/4179/files#diff-ae5a46b3aa3bae6d415117bd5efb9b347ef844d4bdf4c9ef899445c8f77edc6aR13 |
@tchaikov please also test rolling_upgrade_test.py |
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 usescylla nodetool
, otherwise, we fall back to the java-based nodetool. we could instead usescylla 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.