Support limited cache size and automatic cleanup

[Qqwy]

Hi there! This is a very well-written implementation of a SQL-based ActiveSupport::Cache::Store backend. 💚

By reading through the source code, I came across the cleanup method.

It seems like currently you'd have to call this manually. (Or e.g. put it in a whenever-based periodic task to make sure it happens ever so often).
Very much related, it might happen that the cache becomes very large (by being filled with many objects with no expires_in or one that is relatively far in the future).

It would be relatively straightforward to add the following two options:

• A boolean auto_cleanup: option which, when true, will always (in an ensure block for good measure) call cleanup at the end of write_entry.
• A positive-integermax_size: option which, when not-nil, will when calling write_entry while going over the limit, first run cleanup and if that is not enough, remove the oldest non-expired record (based on created_at).

What do you think?

I'd love to contribute a PR for these changes 👍 !

[Qqwy]

PR for the first option has been made: #14

[Qqwy]

And the second part can be found in #15

[dim]

@Qqwy I have reviewed both PRs. As mentioned inline, I do see a use case for both, but I am a little hesitant to add them without further discussion as - although optional - seem to add quite a bit of overhead the average user may not want/be aware of. I have not seen any other cache backends implementing them, have you?

The #cleanup method is a bit of a beast as bulk deletes can be expensive. That's why - as you suggested - it is meant to be called via jobs, in the background. The cache is supposed to be quick, doing a cleanup on every write is going to have a heavily detrimental impact on performance. What do you think?

[dim]

And thank you very much for the contribution, btw. It's really appreciated!

[skatkov]

We're running into a similar issue -- our cache grows by huge margin and it seems to be happening because of expires_at is missing for a lot of records.

It wouldn't be suitable for us to clean cache on write_entry as @dim already mentioned -- it will affect performance.

Instead, I was thinking of modifying cleanup method to introduce ability to purge records that are older than 1 month (this could be configurable). This could be determined based on created_at value.

[dim]

@skatkov I can work with that, would you be able to submit a PR?

[dim]

Alternatively, you could just have a background worker/task/job doing:

m = ActiveSupport::Cache::DatabaseStore::Model
m.where(m.arel_table[:updated_at].lt(1.month.ago)).delete_all

[skatkov]

@dim will submit a PR with a fix.

[skatkov]

Should we close this issue?

Here is PR that solved an issue at hand:
#24