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

Add method to clear cache #15

Merged
merged 5 commits into from
Jan 30, 2023
Merged

Conversation

Msurrow
Copy link

@Msurrow Msurrow commented Jan 23, 2023

We have the need to clear the cache in our project, and have written code to access the sqlite db directly and clear it. I know IDistributedCache does not have a Clear method, but it would be nice to have as part of the "core" functionality.

Copy link
Member

@mqudsi mqudsi left a comment

Choose a reason for hiding this comment

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

Hey thanks for contributing. I agree that it would be useful to have this functionality and I'm not against adding it via the concrete type given that, unlike with redis-backed caches or whatever, you can't just connect to the instance and run flush all.

I'm going to assume that clearing the cache is not a common operation, so I think it probably makes sense for me to push a commit to remove the query from the query cache instances and use it directly at the call site, unless you have any other suggestions.

mqudsi and others added 5 commits January 30, 2023 15:24
There are now two `Use()` overloads, one for the command + connection (existing)
and one for the connection alone (new). The new one can be used when we need to
borrow a connection from the shared pool but don't want to use it with a command
from the command pool.
Implement `SqliteCache.Clear()` without going through the command pool, using
the new `DbCommand.Use()` override that borrows a connection w/out a command,
then creating the command and using it locally.

Assuming that calls to `SqliteCache.Clear()` are not typically going to be in
the hot path, this will reduce the memory usage by not leaving cached instances
of the `DbCommand` around after the call.
Fix the clear cache test. The persistence test made no sense.
@mqudsi mqudsi force-pushed the add-clear-cache branch 2 times, most recently from 1ebfec0 to db8aae3 Compare January 30, 2023 21:58
@mqudsi mqudsi merged commit bae9650 into neosmart:master Jan 30, 2023
@Msurrow
Copy link
Author

Msurrow commented Jan 31, 2023

Hi, sorry for the long response time. I saw your change to move the cmd out of the command pool. I think that's a design decision that's entirely yours :) My motivation for making "basic" command like this others were that I thought of it as a basic action. But if you don't think it'll be the "hot path", as you wrote, when that's all good. I'm just happy to have the clear cache functionality 👍

@mqudsi
Copy link
Member

mqudsi commented Feb 2, 2023

No worries. I keep feeling like the method should be named ClearCache() and not Clear() but I can't come up with a good reason why.

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.

2 participants