Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/java/org/apache/cassandra/cql3/QueryProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -649,8 +649,11 @@ public ResultMessage.Prepared prepare(String queryString, ClientState clientStat
return createResultMessage(hashWithKeyspace, cachedWithKeyspace);
}
}
else
else if (cachedWithoutKeyspace != null || cachedWithKeyspace != null)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need something like else if ((cachedWithoutKeyspace != null) != (cachedWithKeyspace != null)), to make sure at least one of them is in cache.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me elaborate.

boolean safeToReturnCached = cachedWithoutKeyspace != null && cachedWithKeyspace != null;

We are in the else block because safeToReturnCached is false. If safeToReturnCached is false, that means either cachedWithoutKeyspace is null or cachedWithKeyspace is null or both of them.

So, at this place, if (cachedWithoutKeyspace != null || cachedWithKeyspace != null) is sufficient.

Could you please double-check?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jaydeepkumar1984 thanks for elaborating, I think you're right.
Just as a nitpick, and if you get a chance, could you add a short comment saying "only evict if we know one of the statements is cached".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorporated

{
// only evict if we know one of the statements is cached
// This can happen during upgrade when switching prepared statement behaviour
// So only if one of the cache is missing then it means we need to re-prepare
// Make sure the missing one is going to be eventually re-prepared
evictPrepared(hashWithKeyspace);
evictPrepared(hashWithoutKeyspace);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.cassandra.cql3.validation.miscellaneous;

import org.junit.Assert;
import org.junit.Test;

import org.apache.cassandra.cql3.CQLTester;
import org.apache.cassandra.cql3.QueryProcessor;
import org.apache.cassandra.service.ClientState;
import org.apache.cassandra.transport.messages.ResultMessage;

import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicBoolean;

public class PreparedStatementTest extends CQLTester
{
private static final int NUM_THREADS = 50;
private final CountDownLatch startLatch = new CountDownLatch(1);
private final CountDownLatch finishLatch = new CountDownLatch(NUM_THREADS);

@Test
public void testPreparedStatementStaysInCache() throws Throwable
{
execute("CREATE TABLE " + KEYSPACE + ".test_fullyqualified(a int primary key, b int)");

ClientState state = ClientState.forInternalCalls();
Assert.assertEquals(0, QueryProcessor.instance.getPreparedStatements().size());
final ResultMessage.Prepared[] preparedSelect = new ResultMessage.Prepared[NUM_THREADS];
AtomicBoolean preparedStatementPresentInCache = new AtomicBoolean(true);
for (int i = 0; i < NUM_THREADS; i++)
{
int threadId = i;
Thread thread = new Thread(() -> {
try
{
// Wait until the start signal is given
startLatch.await();

// Code to be executed in each thread
preparedSelect[threadId] = QueryProcessor.instance.prepare(
String.format("SELECT b FROM %s.test_fullyqualified where a = 10", KEYSPACE), state);
Assert.assertNotNull(preparedSelect[threadId].statementId);
if(!QueryProcessor.instance.getPreparedStatements().containsKey(preparedSelect[threadId].statementId))
{
preparedStatementPresentInCache.set(false);
}
}
catch (InterruptedException e)
{
Thread.currentThread().interrupt();
}
finally
{
// Signal that this thread has finished
finishLatch.countDown();
}
Assert.fail();
});
thread.start();
}

// Signal all threads to start
startLatch.countDown();

// Wait for all threads to finish
finishLatch.await();
Assert.assertTrue(preparedStatementPresentInCache.get());
}
}