scylladb / scylla-tools-java

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

nodetool status, ring: fix effectiveOwnership() error reporting #398

Closed lersek closed 4 months ago

lersek commented 4 months ago
nodetool status, ring: fix effectiveOwnership() error reporting

The IllegalArgumentException catch stanza around the effectiveOwnership()
method call goes back to "scylla-tools-java" commit d002c7edc58b ("Don't
output nonsense ownership without a keyspace", 2014-09-13). It was
originally meant to handle the case when the user specified a keyspace,
but the keyspace couldn't be found. In that case, nodetool was expected to
fail at once.

Today, the IllegalArgumentException catch stanza is dead code, as the
StorageService managed bean (having been rebased to scylladb) never throws
an IllegalArgumentException; it only propagates scylladb-originated errors
as IllegalStateException. Consequently, when the user asks for a
nonexistent keyspace, we pretend the user didn't ask for a keyspace at
all, and continue with getOwnership().

If the user asks for a keyspace, make any IllegalStateException a hard
failure. Reach the getOwnership() call only if the user does not ask for a
keyspace.

In addition to "nodetool status", "nodetool ring" consumes the
effectiveOwnership() storage service. Reflect the same change to "nodetool
ring".

Cc: Amnon Heiman <amnon@scylladb.com>
Signed-off-by: Laszlo Ersek <laszlo.ersek@scylladb.com>
lersek commented 4 months ago

see also https://github.com/scylladb/scylla-jmx/pull/240

mykaul commented 4 months ago

We've moved (in 6.0) to a native (C++ based) nodetool. Does the issue happen with it as well?

lersek commented 4 months ago

@mykaul no, I've just checked for the parallel discussion with @avikivity ; the C++ built-in command works fine (it reports an explicit, proper failure when a nonexistent keyspace is requested for the ring or status subcommand)

lersek commented 4 months ago

Obsoleted by the C++ rewrite of nodetool; closing.