tolbertam commented on code in PR #3917:
URL: https://github.com/apache/cassandra/pull/3917#discussion_r2042133412
##########
test/unit/org/apache/cassandra/cql3/PstmtPersistenceTest.java:
##########
@@ -142,19 +162,179 @@ public void testPstmtInvalidation() throws Throwable
createTable("CREATE TABLE %s (key int primary key, val int)");
+ long initialEvicted = numberOfEvictedStatements();
+
for (int cnt = 1; cnt < 10000; cnt++)
{
prepareStatement("INSERT INTO %s (key, val) VALUES (?, ?) USING
TIMESTAMP " + cnt, clientState);
- if (numberOfEvictedStatements() > 0)
+ if (numberOfEvictedStatements() - initialEvicted > 0)
{
+ assertEquals("Number of statements in table and in cache don't
match", numberOfStatementsInMemory(), numberOfStatementsOnDisk());
+
+ // prepare a more statements to trigger more evictions
+ for (int cnt2 = cnt + 1; cnt2 < cnt + 10; cnt2++)
+ prepareStatement("INSERT INTO %s (key, val) VALUES (?, ?)
USING TIMESTAMP " + cnt2, clientState);
+
+ // each new prepared statement should have caused an eviction
+ assertEquals("eviction count didn't increase by the expected
number", 10, numberOfEvictedStatements() - initialEvicted);
+ assertEquals("Number of statements in memory (expected) and
table (actual) don't match", numberOfStatementsInMemory(),
numberOfStatementsOnDisk());
+
return;
}
}
fail("Prepared statement eviction does not work");
}
+ /**
+ * Invoked by byteman whenever QueryProcessor.evictPreparedStatement is
called in testAsyncPstmtInvalidation
+ */
+ @SuppressWarnings("unused")
+ private static void recordEvictionTimestamp(MD5Digest key)
+ {
+ preparedStatementRemoveTimestamps.put(key, ClientState.getTimestamp());
+ }
+
+
+ @Test
+ @BMRules(rules= {
+ @BMRule(name = "CaptureWriteTimestamps",
+ targetClass = "SystemKeyspace",
+ targetMethod = "writePreparedStatement(String, MD5Digest, String, long)",
+ targetLocation = "AT INVOKE executeInternal",
+ action =
"org.apache.cassandra.cql3.PstmtPersistenceTest.preparedStatementLoadTimestamps.put($key,
$timestamp);"),
+ @BMRule(name = "CaptureEvictTimestamps",
+ targetClass = "QueryProcessor",
+ targetMethod = "evictPreparedStatement(MD5Digest, RemovalCause)",
+ action =
"org.apache.cassandra.cql3.PstmtPersistenceTest.recordEvictionTimestamp($key);")
+ }
+ )
+ public void testAsyncPstmtInvalidation() throws Throwable
+ {
+ ClientState clientState = ClientState.forInternalCalls();
+ createTable("CREATE TABLE %s (key int primary key, val int)");
+
+ // prepare statements concurrently in a thread pool to exercise bug
encountered in CASSANDRA-19703 where
+ // delete from table occurs before the insert due to early eviction.
+ final ExecutorService executor = Executors.newFixedThreadPool(10);
+
+ long initialEvicted = numberOfEvictedStatements();
+ try
+ {
+ int statementsToPrepare = 10000;
+ List<CompletableFuture<MD5Digest>> prepareFutures = new
ArrayList<>(statementsToPrepare);
+ for (int cnt = 1; cnt <= statementsToPrepare; cnt++)
+ {
+ final int localCnt = cnt;
+ prepareFutures.add(CompletableFuture.supplyAsync(() ->
prepareStatement("INSERT INTO %s (key, val) VALUES (?, ?) USING TIMESTAMP " +
localCnt, clientState), executor));
+ }
+
+ // Await completion
+
CompletableFuture.allOf(prepareFutures.toArray(futureArray)).get(10,
TimeUnit.SECONDS);
+
+ long evictedStatements = numberOfEvictedStatements() -
initialEvicted;
+ assertNotEquals("Should have evicted some prepared statements", 0,
evictedStatements);
+
+ // Recorded prepared statement removals should match metrics
+ assertEquals("Actual evicted statements does not match metrics",
evictedStatements, preparedStatementRemoveTimestamps.size());
+
+ // For each prepared statement evicted, assert the time it was
deleted is greater than the timestamp
+ // used for when it was loaded.
+ for (Map.Entry<MD5Digest, Long> evictedStatementEntry :
preparedStatementRemoveTimestamps.entrySet())
+ {
+ MD5Digest key = evictedStatementEntry.getKey();
+ long deletionTimestamp = evictedStatementEntry.getValue();
+ long insertionTimestamp =
preparedStatementLoadTimestamps.get(key);
+
+ assertTrue(String.format("Expected deletion timestamp for
prepared statement (%d) to be greater than insertion timestamp (%d)",
+ deletionTimestamp,
insertionTimestamp),
+ deletionTimestamp > insertionTimestamp);
+ }
+
+ // ensure the number of statements on disk match the number in
memory, if number of statements on disk eclipses in memory, there was a leak.
+ assertEquals("Number of statements in memory (expected) and table
(actual) don't match", numberOfStatementsInMemory(),
numberOfStatementsOnDisk());
+ }
+ finally
+ {
+ executor.shutdown();
+ }
+ }
+
+ // page size passed to preloadPreparedStatements
+ private static final int PRELOAD_PAGE_SIZE = 100;
+ // recorded page invocations in preloadPreparedStatements
+ private static int PAGE_INVOCATIONS = 0;
Review Comment:
sounds good, updated and also changed to AtomicInteger since we're
incrementing an existing value.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
