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

Pluggable compression for cache values. #42

Merged
merged 15 commits into from
Sep 26, 2023

Conversation

skatkov
Copy link
Contributor

@skatkov skatkov commented Sep 19, 2023

We deployed database-cache-database gem to one of our endpoints and for ~3000 records we're averaging ~1.3 GB memory that cache table takes. It seems a bit excessive.

Usually, most cache stores in Rails deploy compression to minimize storage requirements (with on difference of local storage). With standard gzip we're able to cut down 85% of data stored with very little overhead on write/read operation.

Some other compression algorithms can do it slightly better and slightly faster (like brotli), but adding another dependency to project might not be everyone's preference. This PR offers pluggable compression solution (only gzip support at this point) with 'plain' compression as default. This gives backward-compatibility for existing installations.

This PR depends on #41

@skatkov skatkov changed the title Variable compression for cache values. Pluggable compression for cache values. Sep 19, 2023
@skatkov skatkov marked this pull request as draft September 20, 2023 14:16
julik and others added 5 commits September 22, 2023 16:33
For storing cache entries compression can be very beneficial, as most renders from Rails are actually strings with whitespace and all. This allows for pluggable compression methods, with 'plain' and 'gzip' being available by default. If desired, additional compression methods (such as Brotli) can be added later on. The entries will be decompressed with the appropriate decompression algorithm since the compression method gets recorded in the cache entries table, so a switch to compressed entries is both reversible and deployable gradually - without having to delete existing cache data.

The compression method can be specified by adding the `compression:` kwarg when instantiating the Store:

    Store.new(compression: 'gzip')
@skatkov skatkov force-pushed the variable-compression branch from 626416d to e514bfd Compare September 22, 2023 15:17
@skatkov skatkov marked this pull request as ready for review September 22, 2023 16:10
@skatkov skatkov requested a review from dim September 22, 2023 16:10
@@ -131,7 +138,7 @@ def write_multi_entries(hash, **_options)
def entry_attributes(entry)
expires_at = Time.zone.at(entry.expires_at) if entry.expires_at

{ value: Marshal.dump(entry.value), version: entry.version.presence, expires_at: expires_at }
compression_attributes(entry.value).merge(version: entry.version.presence, expires_at: expires_at)
Copy link
Member

Choose a reason for hiding this comment

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

pls use update, not merge here too. this will allocate a new object and there's no need

def decompress(record)
return Marshal.load(record.value) if record.compression.nil?

COMPRESSION_HANDLERS.fetch(record.compression).decompress(record.value)
Copy link
Member

Choose a reason for hiding this comment

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

you still need to Marshal.load the result

Copy link
Member

Choose a reason for hiding this comment

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

sorry, one last bit, could you please add a test to demonstrate that this is working/was not working before? something where you are e.g. storing a (large enough) Hash in the cache.

@skatkov skatkov requested a review from dim September 25, 2023 14:56
@dim
Copy link
Member

dim commented Sep 26, 2023

Thanks! Looks like there are some CI failures though.

@skatkov skatkov force-pushed the variable-compression branch from ff27ab3 to 69c9c93 Compare September 26, 2023 09:17
@skatkov skatkov force-pushed the variable-compression branch from 69c9c93 to 6c37835 Compare September 26, 2023 09:18
@skatkov
Copy link
Contributor Author

skatkov commented Sep 26, 2023

@dim resolved, sorry about that -)

@dim dim merged commit 5d51f8a into bsm:main Sep 26, 2023
@@ -21,6 +24,10 @@ def self.supports_cache_versioning?
# option options [Class] :model model class. Default: ActiveSupport::Cache::DatabaseStore::Model
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, one thing I forgot to check. What I meant by "documentation" is adding a line here too, to document the new option. I would appreciate if could you do a quick fix PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

3 participants