4.x: Stabilize will_cache_invalid_cql test method.
#664
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The main issue is that it uses `cache.size()`. While it may not be a bad idea to use that, the documentation mentions that it returns an approximate size of the cache. `CqlPrepareAsyncProcessor` actually invalidates the prepare requests that finish with errors (see `process` method). Such is the case with this test. The `PreparedStatement` gets cached for a really short period and then invalidated because of the exception. Depending on the exact timings it would be correct for the cache to report either size 0 or size 1. Additionally the moment when the size gets updated may differ from the moments modifications to the cache contents are made. This change is a quick workaround that should achieve the same end result as the setup in PreparedStatementCachingIT. Here we use reflection to forcibly change the cache used by the request processor. Same type, but has removal listener added to it. Instead of relying on the uncertain cache size the test will listen for the removal from the cache. The alternative would be reimplementing similar setup as in PreparedStatementCachingIT with some changes. That means having a test version of DefaultDriverContext, CqlPrepareAsyncProcessor, SessionBuilder and necessary methods, so that we could inject the listener without the reflection. The other option is moving this test method to the PreparedStatementCachingIT, but then the cache's remove callback which is used there throughout the class needs to be adjusted specifically for this method.
Bouncheck
changed the title
will_cache_invalid_cql test method.will_cache_invalid_cql test method.
Author
|
So far around 100 runs completed without an issue. |
dkropachev
approved these changes
Sep 24, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The main issue is that it uses
cache.size().While it may not be a bad idea to use that, the documentation mentions that it returns an approximate size of the cache.
CqlPrepareAsyncProcessoractually invalidates the prepare requests that finish with errors (seeprocessmethod). Such is the case with this test. ThePreparedStatementgets cached for a really short period and then invalidated because of the exception.Depending on the exact timings it would be correct for the cache to report either size 0 or size 1. Additionally the moment when the size gets updated may differ from the moments modifications to the cache contents are made.
This change is a quick workaround that should
achieve the same end result as the setup in PreparedStatementCachingIT. Here we use reflection to forcibly change the cache used by the request processor. Same type, but has removal listener added to it. Instead of relying on the uncertain cache size the test will listen for the removal from the cache.
The alternative would be reimplementing similar setup as in PreparedStatementCachingIT with some changes. That means having a test version of DefaultDriverContext, CqlPrepareAsyncProcessor, SessionBuilder and necessary methods, so that we could inject the listener without the reflection.
The other option is moving this test method to the PreparedStatementCachingIT, but then the cache's remove callback which is used there throughout the class needs to be adjusted specifically for this method.
Fixes #514