scylladb / scylla-tools-java

Apache Cassandra, supplying tools for Scylla
Apache License 2.0
53 stars 85 forks source link

bin/nodetool-wrapper: override the -p option with the one set by -Dcom.scylladb.apiPort #380

Closed tchaikov closed 9 months ago

tchaikov commented 9 months ago

"scylla nodetool" connects to the port of the API service, while Cassandra nodetool connects to the port of the JMX service. and nodetool-wrapper tries to be a broker to delegate to the latter if the "scylla nodetool" cannot handle the specified command. but, if the command line has "-p 7199" instead of "-p 10000" specified, and the command line is able to be handled by "scylla nodetool", nodetool-wrapper would be instructing "scylla nodetool" to connect to JMX server instead of the API service. that's why we'd have following test failure when we tries to enable "scylla nodetool" to handle more commands in the test:

ccmlib.node.ToolError: Subprocess /jenkins/workspace/scylla-master/gating-dtest-release/scylla/.ccm/scylla-repository/17238/share/cassandra/bin/nodetool -h 127.0.18.1 -p 7199 -Dcom.sun.jndi.rmiURLParsing=legacy flush exited with non-zero status; exit status: 2;
stderr: error running operation: std::runtime_error (Invalid response)

in this change, we let the wrapper to mutate the "-p" or "--port" option in the command line options with the one specified by "-Dcom.scylladb.apiPort=", which is not recognized by Cassandra nodetool, but it is set by the ccmlib, when the test is performed against a scylla server. so, both "scylla nodetool" can connect to the API service when it is called by the tests in dtest.

tchaikov commented 9 months ago

v2:

tchaikov commented 9 months ago

v3:

denesb commented 9 months ago

@tchaikov I think we don't need this PR anymore, with v2 of https://github.com/scylladb/scylladb/pull/17168. The latter adds native support for -Dcom.scylladb.apiPort in scylla-nodetool, so we can just let the option pass-through unchanged.

tchaikov commented 9 months ago

ack. closing this change.