-
Notifications
You must be signed in to change notification settings - Fork 884
CASSJAVA-104: Fix Flaky tests #2045
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 4.x
Are you sure you want to change the base?
Changes from all commits
16b7883
6f31d18
ff0cc04
6594f3b
faeda6e
935cf34
ab7b1c1
7ac257a
5593f5b
646ffc9
02dedff
e2b5062
eb8822b
a7ff308
d7165f2
ec7a107
cf054fa
9045265
933bafb
6ec3c73
9ee5a8b
3289fda
98eb757
c0d2f74
4f22bd6
9ba8203
647d262
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -194,8 +194,8 @@ private void invalidationResultSetTest( | |
| Consumer<CqlSession> setupTestSchema, Set<String> expectedChangedTypes) { | ||
| invalidationTestInner( | ||
| setupTestSchema, | ||
| "select f from test_table_1 where e = ?", | ||
| "select h from test_table_2 where g = ?", | ||
| "select f from test_table_caching_1 where e = ?", | ||
| "select h from test_table_caching_2 where g = ?", | ||
| expectedChangedTypes); | ||
| } | ||
|
|
||
|
|
@@ -206,8 +206,8 @@ private void invalidationVariableDefsTest( | |
| String condition = isCollection ? "contains ?" : "= ?"; | ||
| invalidationTestInner( | ||
| setupTestSchema, | ||
| String.format("select e from test_table_1 where f %s allow filtering", condition), | ||
| String.format("select g from test_table_2 where h %s allow filtering", condition), | ||
| String.format("select e from test_table_caching_1 where f %s allow filtering", condition), | ||
| String.format("select g from test_table_caching_2 where h %s allow filtering", condition), | ||
| expectedChangedTypes); | ||
| } | ||
|
|
||
|
|
@@ -263,16 +263,18 @@ private void invalidationTestInner( | |
| preparedStmtCacheRemoveLatch.countDown(); | ||
| }); | ||
|
|
||
| // alter test_type_2 to trigger cache invalidation and above events | ||
| session.execute("ALTER TYPE test_type_2 add i blob"); | ||
| // alter test_type_caching_2 to trigger cache invalidation and above events | ||
| session.execute("ALTER TYPE test_type_caching_2 add i blob"); | ||
|
|
||
| session.checkSchemaAgreement(); | ||
|
|
||
| // wait for latches and fail if they don't reach zero before timeout | ||
| assertThat( | ||
| Uninterruptibles.awaitUninterruptibly( | ||
| preparedStmtCacheRemoveLatch, 10, TimeUnit.SECONDS)) | ||
| preparedStmtCacheRemoveLatch, 120, TimeUnit.SECONDS)) | ||
| .withFailMessage("preparedStmtCacheRemoveLatch did not trigger before timeout") | ||
| .isTrue(); | ||
| assertThat(Uninterruptibles.awaitUninterruptibly(typeChangeEventLatch, 10, TimeUnit.SECONDS)) | ||
| assertThat(Uninterruptibles.awaitUninterruptibly(typeChangeEventLatch, 20, TimeUnit.SECONDS)) | ||
| .withFailMessage("typeChangeEventLatch did not trigger before timeout") | ||
| .isTrue(); | ||
|
|
||
|
|
@@ -295,17 +297,20 @@ private void invalidationTestInner( | |
|
|
||
| Consumer<CqlSession> setupCacheEntryTestBasic = | ||
| (session) -> { | ||
| session.execute("CREATE TYPE test_type_1 (a text, b int)"); | ||
| session.execute("CREATE TYPE test_type_2 (c int, d text)"); | ||
| session.execute("CREATE TABLE test_table_1 (e int primary key, f frozen<test_type_1>)"); | ||
| session.execute("CREATE TABLE test_table_2 (g int primary key, h frozen<test_type_2>)"); | ||
| session.execute("CREATE TYPE test_type_caching_1 (a text, b int)"); | ||
| session.execute("CREATE TYPE test_type_caching_2 (c int, d text)"); | ||
| session.execute( | ||
| "CREATE TABLE test_table_caching_1 (e int primary key, f frozen<test_type_caching_1>)"); | ||
| session.execute( | ||
| "CREATE TABLE test_table_caching_2 (g int primary key, h frozen<test_type_caching_2>)"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good job, former me 🤦 Should the names of the tables and types created here not follow the basic/collection/tuple/nested split we use for setupCacheEntryTest* method names throughout? Seems like this should be: and so on if we really want to avoid collisions.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But don't we have to change the table name since these tests don't clean up after themselves? Once you execute the tests for, say, basic you've already created test_type_1, test_type_2 and all the rest. Won't you have a conflict then if you try to create those types against the same Cassandra backend (since they were never removed)?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test suite has a
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wait, is that how that works? I remembered that working differently. My recollection was that CcmBridge just controlled the ccm interaction and that the session rule was the entity that constrained ops to a given keyspace. Yeah, as I read this SessionRule can create a new keyspace if the user specifies it in the builder but this IT does not currently do so and this PR doesn't change that behaviour. I do agree we need a new keyspace per test (since without that the |
||
| }; | ||
|
|
||
| @Test | ||
| public void should_invalidate_cache_entry_on_basic_udt_change_result_set() { | ||
| SchemaChangeSynchronizer.withLock( | ||
| () -> { | ||
| invalidationResultSetTest(setupCacheEntryTestBasic, ImmutableSet.of("test_type_2")); | ||
| invalidationResultSetTest( | ||
| setupCacheEntryTestBasic, ImmutableSet.of("test_type_caching_2")); | ||
| }); | ||
| } | ||
|
|
||
|
|
@@ -314,25 +319,26 @@ public void should_invalidate_cache_entry_on_basic_udt_change_variable_defs() { | |
| SchemaChangeSynchronizer.withLock( | ||
| () -> { | ||
| invalidationVariableDefsTest( | ||
| setupCacheEntryTestBasic, false, ImmutableSet.of("test_type_2")); | ||
| setupCacheEntryTestBasic, false, ImmutableSet.of("test_type_caching_2")); | ||
| }); | ||
| } | ||
|
|
||
| Consumer<CqlSession> setupCacheEntryTestCollection = | ||
| (session) -> { | ||
| session.execute("CREATE TYPE test_type_1 (a text, b int)"); | ||
| session.execute("CREATE TYPE test_type_2 (c int, d text)"); | ||
| session.execute("CREATE TYPE test_type_caching_1 (a text, b int)"); | ||
| session.execute("CREATE TYPE test_type_caching_2 (c int, d text)"); | ||
| session.execute( | ||
| "CREATE TABLE test_table_1 (e int primary key, f list<frozen<test_type_1>>)"); | ||
| "CREATE TABLE test_table_caching_1 (e int primary key, f list<frozen<test_type_caching_1>>)"); | ||
| session.execute( | ||
| "CREATE TABLE test_table_2 (g int primary key, h list<frozen<test_type_2>>)"); | ||
| "CREATE TABLE test_table_caching_2 (g int primary key, h list<frozen<test_type_caching_2>>)"); | ||
| }; | ||
|
|
||
| @Test | ||
| public void should_invalidate_cache_entry_on_collection_udt_change_result_set() { | ||
| SchemaChangeSynchronizer.withLock( | ||
| () -> { | ||
| invalidationResultSetTest(setupCacheEntryTestCollection, ImmutableSet.of("test_type_2")); | ||
| invalidationResultSetTest( | ||
| setupCacheEntryTestCollection, ImmutableSet.of("test_type_caching_2")); | ||
| }); | ||
| } | ||
|
|
||
|
|
@@ -341,25 +347,26 @@ public void should_invalidate_cache_entry_on_collection_udt_change_variable_defs | |
| SchemaChangeSynchronizer.withLock( | ||
| () -> { | ||
| invalidationVariableDefsTest( | ||
| setupCacheEntryTestCollection, true, ImmutableSet.of("test_type_2")); | ||
| setupCacheEntryTestCollection, true, ImmutableSet.of("test_type_caching_2")); | ||
| }); | ||
| } | ||
|
|
||
| Consumer<CqlSession> setupCacheEntryTestTuple = | ||
| (session) -> { | ||
| session.execute("CREATE TYPE test_type_1 (a text, b int)"); | ||
| session.execute("CREATE TYPE test_type_2 (c int, d text)"); | ||
| session.execute("CREATE TYPE test_type_caching_1 (a text, b int)"); | ||
| session.execute("CREATE TYPE test_type_caching_2 (c int, d text)"); | ||
| session.execute( | ||
| "CREATE TABLE test_table_1 (e int primary key, f tuple<int, test_type_1, text>)"); | ||
| "CREATE TABLE test_table_caching_1 (e int primary key, f tuple<int, test_type_caching_1, text>)"); | ||
| session.execute( | ||
| "CREATE TABLE test_table_2 (g int primary key, h tuple<text, test_type_2, int>)"); | ||
| "CREATE TABLE test_table_caching_2 (g int primary key, h tuple<text, test_type_caching_2, int>)"); | ||
| }; | ||
|
|
||
| @Test | ||
| public void should_invalidate_cache_entry_on_tuple_udt_change_result_set() { | ||
| SchemaChangeSynchronizer.withLock( | ||
| () -> { | ||
| invalidationResultSetTest(setupCacheEntryTestTuple, ImmutableSet.of("test_type_2")); | ||
| invalidationResultSetTest( | ||
| setupCacheEntryTestTuple, ImmutableSet.of("test_type_caching_2")); | ||
| }); | ||
| } | ||
|
|
||
|
|
@@ -368,26 +375,29 @@ public void should_invalidate_cache_entry_on_tuple_udt_change_variable_defs() { | |
| SchemaChangeSynchronizer.withLock( | ||
| () -> { | ||
| invalidationVariableDefsTest( | ||
| setupCacheEntryTestTuple, false, ImmutableSet.of("test_type_2")); | ||
| setupCacheEntryTestTuple, false, ImmutableSet.of("test_type_caching_2")); | ||
| }); | ||
| } | ||
|
|
||
| Consumer<CqlSession> setupCacheEntryTestNested = | ||
| (session) -> { | ||
| session.execute("CREATE TYPE test_type_1 (a text, b int)"); | ||
| session.execute("CREATE TYPE test_type_2 (c int, d text)"); | ||
| session.execute("CREATE TYPE test_type_3 (e frozen<test_type_1>, f int)"); | ||
| session.execute("CREATE TYPE test_type_4 (g int, h frozen<test_type_2>)"); | ||
| session.execute("CREATE TABLE test_table_1 (e int primary key, f frozen<test_type_3>)"); | ||
| session.execute("CREATE TABLE test_table_2 (g int primary key, h frozen<test_type_4>)"); | ||
| session.execute("CREATE TYPE test_type_caching_1 (a text, b int)"); | ||
| session.execute("CREATE TYPE test_type_caching_2 (c int, d text)"); | ||
| session.execute("CREATE TYPE test_type_caching_3 (e frozen<test_type_caching_1>, f int)"); | ||
| session.execute("CREATE TYPE test_type_caching_4 (g int, h frozen<test_type_caching_2>)"); | ||
| session.execute( | ||
| "CREATE TABLE test_table_caching_1 (e int primary key, f frozen<test_type_caching_3>)"); | ||
| session.execute( | ||
| "CREATE TABLE test_table_caching_2 (g int primary key, h frozen<test_type_caching_4>)"); | ||
| }; | ||
|
|
||
| @Test | ||
| public void should_invalidate_cache_entry_on_nested_udt_change_result_set() { | ||
| SchemaChangeSynchronizer.withLock( | ||
| () -> { | ||
| invalidationResultSetTest( | ||
| setupCacheEntryTestNested, ImmutableSet.of("test_type_2", "test_type_4")); | ||
| setupCacheEntryTestNested, | ||
| ImmutableSet.of("test_type_caching_2", "test_type_caching_4")); | ||
| }); | ||
| } | ||
|
|
||
|
|
@@ -396,7 +406,9 @@ public void should_invalidate_cache_entry_on_nested_udt_change_variable_defs() { | |
| SchemaChangeSynchronizer.withLock( | ||
| () -> { | ||
| invalidationVariableDefsTest( | ||
| setupCacheEntryTestNested, false, ImmutableSet.of("test_type_2", "test_type_4")); | ||
| setupCacheEntryTestNested, | ||
| false, | ||
| ImmutableSet.of("test_type_caching_2", "test_type_caching_4")); | ||
| }); | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was a little confused by this. It looks like the 1-argument constructor is used by the codebase, and the 2 argument constructor is used by tests, by moving the weakValues to the 1arg constructor, this allows the tests to work around the cache values being evicted as soon as they are no longer referenced elsewhere.
Think it would be better to just add a comment in the 1-arg constructor where weakValue is declared so it will be more clear why it's used there.