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

Auto cleanup option #14

Closed
wants to merge 4 commits into from
Closed

Auto cleanup option #14

wants to merge 4 commits into from

Conversation

Qqwy
Copy link

@Qqwy Qqwy commented Dec 20, 2022

Adds an auto_cleanup option to the ActiveSupport::Cache::DatabaseStore class initialization options.

When true, cleanup will be run whenever a write or delete is executed. This keeps the amount of expired entries in the database small.

  • implementation
  • documentation
  • tests

@Qqwy Qqwy mentioned this pull request Dec 20, 2022
def initialize(options = nil)
@model = (options || {}).delete(:model) || Model
@auto_cleanup = (options || {}).delete(:auto_cleanup) || false
Copy link
Member

Choose a reason for hiding this comment

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

|| false will never yield a result, I would suggest (options || {}).delete(:auto_cleanup) == true instead

@@ -20,8 +20,10 @@ def self.supports_cache_versioning?

# param [Hash] options options
# option options [Class] :model model class. Default: ActiveSupport::Cache::DatabaseStore::Model
# option options [Boolean] :auto_cleanup When true, runs {#cleanup} after every {#write_entry} and {#delete_entry}. Default: false
Copy link
Member

Choose a reason for hiding this comment

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

Is there an example of any other store implementing this feature? I am not sure I agree with making this a boolean option and cleaning up on every write. I assume most users would want this to be a time-based option

@dim
Copy link
Member

dim commented Jun 15, 2023

So I revisited this PR and I am not sure it's necessary to clean up on every write. Instead it may be more beneficial to have a cleanup_every option, something that triggers a cleanup every N seconds/minutes/hours. That being said, I think #13 (comment) triggered by a background worker is probably the simplest option

@dim dim closed this Jun 15, 2023
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