scylladb / python-driver

ScyllaDB Python Driver, originally DataStax Python Driver for Apache Cassandra
https://python-driver.docs.scylladb.com
Apache License 2.0
70 stars 42 forks source link

CREATE ROLE statement wrongly treated as idempotent and retried #296

Closed nuivall closed 5 months ago

nuivall commented 6 months ago

When doing cql.execute(SimpleStatement(f"CREATE ROLE {role}", is_idempotent=False)) I noticed that sometimes query fails with

E   cassandra.InvalidRequest: Error from server: code=2200 [Invalid query] message="Role rotest_1706809090703 already exists."

this is becacuse driver is sending it to multiple nodes (I guess speculatively) while in scylla we do distinguish between CREATE ROLE and CREATE ROLE IF NOT EXISTS.

Screenshot from 2024-02-01 18-52-48

wireshark: Screenshot from 2024-02-01 18-52-30 Screenshot from 2024-02-01 18-53-42

sylwiaszunejko commented 6 months ago

@nuivall Could you provide some more information? Maybe test that is failing or some other reproducer. How often does it fail? Maybe there are some requirements that have to be met in order to reproduce?

nuivall commented 6 months ago

With test.py something similar to:

    servers = await manager.servers_add(3)

    cql = manager.get_cql()
    await wait_for_cql_and_get_hosts(cql, servers, time.time() + 60)
    await manager.servers_see_each_other(servers)

    roles = ["ro" + unique_name() for _ in range(10)]
    for role in roles:
        cql.execute(f"CREATE ROLE {role}")

You should be able to see that the last line sometimes fails even despite the fact that role name is always unique.

sylwiaszunejko commented 6 months ago

@nuivall Do I need some special settings to be able to execute CREATE ROLE? Other commands for example SELECT work, but when I try to create a role I have:

E   cassandra.Unauthorized: Error from server: code=2100 [Unauthorized] message="You have to be logged in and not anonymous to perform this request"
nuivall commented 6 months ago

Yes you need to be authed as admin. Default user should be cassandra, password cassandra. (and authentication need to be enabled in scylla.yaml)

sylwiaszunejko commented 6 months ago

@nuivall My test looks like that:

    provider = PlainTextAuthProvider(username="cassandra", password="cassandra")
    manager.auth_provider = provider
    servers = await manager.servers_add(3)

    cql = manager.get_cql()
    await wait_for_cql_and_get_hosts(cql, servers, time.time() + 60)
    await manager.servers_see_each_other(servers)

    roles = ["ro" + unique_name() for _ in range(10)]
    for role in roles:
        cql.execute(SimpleStatement(f"CREATE ROLE {role}", is_idempotent=False))

I have run it hundreds of times and it always passes. Do you have any suggestions what can I change to reproduce the issue?

nuivall commented 6 months ago

It may pass as currently create role is eventually consistent, but have you verified that request is indeed sent to only one coordinator?

sylwiaszunejko commented 6 months ago

It may pass as currently create role is eventually consistent, but have you verified that request is indeed sent to only one coordinator?

No, I did not verify that.

nuivall commented 6 months ago

Although currently I also can't reproduce with your test, I'll try to fiddle with it.

sylwiaszunejko commented 5 months ago

@nuivall any updates? do you manage to reproduce the issue?

sylwiaszunejko commented 5 months ago

@nuivall ping

nuivall commented 5 months ago

Looks like I can no longer reproduce, closing for now.

nyh commented 4 months ago

I'm not thrilled that this issue was closed because it "no longer reproduces". Maybe it's just a matter of luck and timing?

The original report above suggested that "CREATE ROLE" is wrongly treated as idempotent, so the driver's "speculative execution policy" (https://docs.datastax.com/en/developer/python-driver/3.29/api/cassandra/policies/#cassandra.policies.SpeculativeExecutionPolicy) retries it. We should be able to inspect the driver code and/or Scylla and check if this is the case - how does the driver decide if "CREATE ROLE" is idempotent or not?

If we can confirm that the driver does, in fact, decide that CREATE ROLE (without "if not exist") is not idempotent, then definitely this issue should be closed. But I'm not sure that just not being able to reproduce this sort of race is a good-enough reason to close the issue.

nuivall commented 4 months ago

@sylwiaszunejko do driver tests cover that? I guess it's easier to craft such test on the driver side

sylwiaszunejko commented 4 months ago

Unfortunately there are no tests covering CREATE ROLE without "if not exist".

sylwiaszunejko commented 4 months ago

@nyh I tested it once again, and this issue did not occur. Also, I inspected the driver code, and there is nothing suspicious in checking if the query is idempotent. In this issue, there was a problem in the case when we explicitly specify that it should be idempotent, so the driver doesn't make any decisions; we tell it cql.execute(SimpleStatement(f"CREATE ROLE {role}", is_idempotent=False)). I am not sure why this problem happened in the first place. Perhaps it was due to human error or some other issues not related to the driver?