scylladb / scylla-jmx

Scylla JMX proxy
GNU Affero General Public License v3.0
28 stars 51 forks source link

ColumnFamilyStore: only quote table names if necessary #230

Closed nyh closed 7 months ago

nyh commented 7 months ago

In a recent patch, we added quoting of table names to ColumnFamilyStore, to solve a bug where the existance of a table with a colon in its name broke various JMX requests.

I was under the impression that these quoted names were only used internally, for checking the validity of table names, and the code unquoted them when necessary. I thought based on my not-extensive-enough testing that nodetool now works correctly both when table names with colon names exist, and when they don't.

Unfortunately, further runs of all nodetool dtests revealed that the quoting broke the "nodetool getsstables" command (and only it). For some reason I don't fully understand yet, this specific command ends up using ColumnFamilyStore and getting confused by the quoted table names.

So in this patch, ColumnFamilyStore makes an effort to only quote a table name if it's necessary (i.e., contains a colon), and only unquote it when necessary.

After this patch, the tests for "nodetool tablestats" and "nodetool info" with a colon in a table's name continue to pass (these are the cases solved by the previous patches, and this patch doesn't break them), and the "nodetool getsstables" test starts working again.

nyh commented 7 months ago

Hi @denesb, this is a followup to the fix in https://github.com/scylladb/scylla-jmx/pull/227 (it's a followup, meaning I assume you didn't revert that older patch). I'm sorry about the mess.

I confirmed that after this patch, the two dtests that failed the previous CI run now pass. I also confirmed that the two uncommitted dtests fixed by 227 didn't break by this patch. I didn't run all dtests again (life is too short), but I'm hopefull.

denesb commented 7 months ago

Submodule update PR: https://github.com/scylladb/scylladb/pull/16146.