[CASSANDRA-15252] Don't consider current keyspace in prepared statement id when the query is qualified

• Type: Bug
• Status: Resolved
• Priority: Normal
• Resolution: Fixed
• Fix Version/s: 3.0.26, 3.11.12, 4.0.2, 4.1-alpha1, 4.1
• Component/s: Messaging/Client
• Bug Category: Availability - Response Crash
• Severity: Critical
• Complexity: Challenging
• Discovered By: User Report
• Platform: All
• Since Version: 3.0.0

Source Control Link

https://github.com/apache/cassandra/commit/13632e9a99e8256a565bd6919d2d11b3e476e973

Test and Documentation Plan

Tests are included in the patch

• Assignee: Alex Petrov
• Reporter: Olivier Michallat

Authors

Alex Petrov

Reviewers

Alex Petrov, Marcus Eriksson

• Created: 02/Aug/19 00:32
• Updated: 27/May/22 19:25
• Resolved: 01/Oct/21 16:18

Description

QueryProcessor.computeId takes into account the session's current keyspace in the MD5 digest.

String toHash = keyspace == null ? queryString : keyspace + queryString;

This is desirable for unqualified queries, because switching to a different keyspace produces a different statement. However, for a qualified query, the current keyspace makes no difference, the prepared id should always be the same.

This can lead to an infinite reprepare loop on the client. Consider this example (Java driver 3.x):

    Cluster cluster = null;
    try {
      cluster = Cluster.builder().addContactPoint("127.0.0.1").build();
      Session session = cluster.connect();

      session.execute(
          "CREATE KEYSPACE IF NOT EXISTS test WITH replication = {'class': 'SimpleStrategy', 'replication_factor': 1}");
      session.execute("CREATE TABLE IF NOT EXISTS test.foo(k int PRIMARY KEY)");

      PreparedStatement pst = session.prepare("SELECT * FROM test.foo WHERE k=?");

      // Drop and recreate the table to invalidate the prepared statement server-side
      session.execute("DROP TABLE test.foo");
      session.execute("CREATE TABLE test.foo(k int PRIMARY KEY)");

      session.execute("USE test");

      // This will try to reprepare on the fly
      session.execute(pst.bind(0));
    } finally {
      if (cluster != null) cluster.close();
    }

When the driver goes to execute the bound statement (last line before the finally block), it will get an UNPREPARED response because the statement was evicted from the server cache (as a result of dropping the table earlier).
In those cases, the driver recovers transparently by sending another PREPARE message and retrying the bound statement.
However, that second PREPARE cached the statement under a different id, because we switched to another keyspace. Yet the driver is still using the original id (stored in pst) when it retries, so it will get UNPREPARED again, etc.

I would consider this low priority because issuing a USE statement after having prepared statements is a bad idea to begin with. And even if we fix the generated id for qualified query strings, the issue will remain for unqualified ones.

We'll add a check in the driver to fail fast and avoid the infinite loop if the id returned by the second PREPARE doesn't match the original one. That might be enough to cover this issue.

Issue Links

• breaks: CASSANDRA-17328 Fix flaky test - dtest.repair_tests.repair_test.TestRepair.test_dc_parallel_repair
• breaks: CASSANDRA-17349 Fix flaky test - dtest-novnode.repair_tests.repair_test.TestRepair.test_simple_sequential_repair