search

scylladb / scylla-tools-java

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

cassandra-stress: switch create table to use keyspace.table format #358

Closed roydahan closed 8 months ago

roydahan commented 11 months ago

Stop using "Use keyspace" since it can fail when using multiple stress commands in parallel due to a race condition.

fixes: #356

mykaul commented 10 months ago

ping

roydahan commented 10 months ago

I never had the chance to test it, but it's also very rare case, so not urgent.

avelanarius commented 10 months ago

Does it really solve #356? I don't see why it'd solve that problem - if USE keyspace would fail, why wouldn't CREATE keyspace.table also fail?

roydahan commented 10 months ago

The problem happens when you run multiple clients at the same time. It's a race condition that would be hard to prove that is fixed. However, In any case I don't see any benefit of using "USE keyspace" instead of creating with full path.

I still need to test that it doesn't break anything, just didn't find a time for it yet.

mykaul commented 9 months ago

What's holding this?

avelanarius commented 9 months ago

What's holding this?

This PR lacks any explanation/proof why this really fixes the problem.

And personally I believe that this doesn't fix anything. If it really fixed the problem, that would mean that there is a serious bug in Java Driver around USE statements that should be fixed and not worked around.

fruch commented 9 months ago

What's holding this?

This PR lacks any explanation/proof why this really fixes the problem.

And personally I believe that this doesn't fix anything. If it really fixed the problem, that would mean that there is a serious bug in Java Driver around USE statements that should be fixed and not worked around.

see https://github.com/scylladb/scylladb/pull/16969, it's a really issue in scylla's end, that create KS doesn't always return to the driver that the schema has changed.

anyhow this change is removing one command, that lower the changes of this issue to be happening

fruch commented 9 months ago

taking this change locally, it won't work.

the rest of the calls in c-s are depended on this USE call...

one need to go all over the place to make sure it's mentioning the keyspace in any CQL command is executed

roydahan commented 8 months ago

The real fix should be this one: https://github.com/scylladb/scylladb/pull/16969. Closing this in favor of handling the real issue.