maedhroz commented on a change in pull request #1221:
URL: https://github.com/apache/cassandra/pull/1221#discussion_r727740163
##########
File path: test/unit/org/apache/cassandra/schema/SchemaKeyspaceTest.java
##########
@@ -183,8 +269,32 @@ public void testSchemaNoColumn()
KeyspaceParams.simple(1),
SchemaLoader.standardCFMD(testKS,
testTable));
// Delete all colmns in the schema
- String query = String.format("DELETE FROM %s.%s WHERE keyspace_name=?
and table_name=?", SchemaConstants.SCHEMA_KEYSPACE_NAME,
SchemaKeyspace.COLUMNS);
+ String query = String.format("DELETE FROM %s.%s WHERE keyspace_name=?
and table_name=?", SchemaConstants.SCHEMA_KEYSPACE_NAME,
SystemKeyspaceConstants.COLUMNS);
executeOnceInternal(query, testKS, testTable);
SchemaKeyspace.fetchNonSystemKeyspaces();
}
+
+ private void testMethodSynchronicity(Runnable method) throws Exception
+ {
+ ExecutorService pool = Executors.newFixedThreadPool(2);
+ CountDownLatch pendingStarts = new CountDownLatch(2);
+
+ threadBlock = new CountDownLatch(2);
+ pool.execute(() -> {
+ pendingStarts.countDown();
+ method.run();
+ });
+ pool.execute(() -> {
+ pendingStarts.countDown();
+ method.run();
+ });
+
+ // We have started 2 threads. 1 thread should be held by the latch
threadBlock, the other should be held as
+ // proof of thread safety.
+ pendingStarts.await(10, TimeUnit.SECONDS);
+ Util.spinAssertEquals(1L, () -> threadBlock.getCount(), 10);
+
+ // Do not execute the tasks. Just kill it.
+ pool.shutdownNow();
+ }
Review comment:
Thanks for the other minor fixes. I think we're almost there.
I know we've gone back and forth quite a bit, but the tests are still more
or less checking that we've synchronized the appropriate methods (although the
latest one does at least act as a tripwire for the static vs instance method
synchronization issue).
I put some code to the descriptions I've given in a couple other places:
```
@Test
@BMRule(name = "delay partition updates to schema tables",
targetClass = "CassandraTableWriteHandler",
targetMethod = "write",
action = "Thread.sleep(100);",
targetLocation = "AT EXIT")
public void testNoVisiblePartialSchemaUpdates() throws Exception
{
String keyspace = "sandbox";
ExecutorService pool = Executors.newFixedThreadPool(2);
Schema.truncateSystemKeyspace(); // Make sure there's nothing but the
create we're about to do
CyclicBarrier barrier = new CyclicBarrier(2);
Future<Void> creation = pool.submit(() -> {
barrier.await();
createTable(keyspace, "CREATE TABLE test (a text primary key, b int,
c int)");
return null;
});
Future<Collection<Mutation>> mutationsFromThread = pool.submit(() -> {
barrier.await();
return Schema.schemaKeyspaceAsMutations();
});
creation.get(); // make sure the creation is finished
Collection<Mutation> mutationsFromConcurrentAccess =
mutationsFromThread.get();
Collection<Mutation> settledMutations =
Schema.schemaKeyspaceAsMutations();
// If the worker thread picked up the creation at all, it should have
the same modifications.
// In other words, we should see all modifications or none.
if (mutationsFromConcurrentAccess.size() == settledMutations.size())
{
assertEquals(1, settledMutations.size());
Mutation mutationFromConcurrentAccess =
mutationsFromConcurrentAccess.iterator().next();
Mutation settledMutation = settledMutations.iterator().next();
assertEquals("Read partial schema change!",
settledMutation.getTableIds(),
mutationFromConcurrentAccess.getTableIds());
}
pool.shutdownNow();
}
```
This tests the public API of Schema for the particular use case that spawned
this Jira, reliably identifies a partial schema update on creation, and passes
reliably with proper synchronization. I would be perfectly happy w/ our
coverage (like +1 now, given it looks like we've cleaned up everything else) if
the patch added just this test, although it could probably be pretty easily
generalized to handle truncation as well.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
