Skip to content
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

CQLSessionCache - evictionListener changed to removalListener #730

Merged
merged 6 commits into from
Dec 20, 2023

Conversation

kathirsvn
Copy link
Contributor

What this PR does:

In CQLSessionCache, Caffeine cache's evictionListener is changed to removalListener.

  1. The configured removal listener is invoked after the entry has been removed from the cache, so any session that gets evicted will first be removed from the cache so it will no longer be found for any key.
  2. The configured removal listener closes the CqlSession that is passed which is a graceful shut down (i.e. any in-flight queries will be completed before shutting down the channels)

Which issue(s) this PR fixes:
Fixes #645

Checklist

  • Changes manually tested
  • Automated Tests added/updated
  • Documentation added/updated
  • CLA Signed: DataStax CLA

@kathirsvn kathirsvn requested a review from a team as a code owner December 13, 2023 12:47
@kathirsvn kathirsvn changed the title CQLSessionCache Caffeine cache's evictionListener changed to removalL… CQLSessionCache - evictionListener changed to removalListener Dec 13, 2023
@kathirsvn
Copy link
Contributor Author

@stargate/core The IT for the native image fails in this PR.
https://github.com/stargate/jsonapi/actions/runs/7195421867/job/19598287312?pr=730.

I reported the same here. Can we disable native image test for now until this is fixed on the Quarkus side?

@jeffreyscarpenter
Copy link
Contributor

Can we disable native image test for now until this is fixed on the Quarkus side?

Yes, I agree this makes sense @kathirsvn

@kathirsvn
Copy link
Contributor Author

Excluded the Native Image CI tests and the tests look good now.

Copy link
Contributor

@Yuqi-Du Yuqi-Du left a comment

Choose a reason for hiding this comment

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

LGTM

@kathirsvn kathirsvn merged commit b992077 into main Dec 20, 2023
2 checks passed
@kathirsvn kathirsvn deleted the session_eviction_fix branch December 20, 2023 03:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Closing session with possible in-flight queries
3 participants