smiklosovic commented on code in PR #4514:
URL: https://github.com/apache/cassandra/pull/4514#discussion_r2592761187
##########
src/java/org/apache/cassandra/io/compress/ZstdDictionaryCompressor.java:
##########
Review Comment:
not particularly related to this patch but this just got me thinking ...
`instancePerDict` is a cache where entries are expired after
`DatabaseDescriptor.getCompressionDictionaryCacheExpireSeconds()`. We also have
invalidate method which clears this, for testing purposes.
What happens with `instancesPerLevel` map content? We are not cleaning this
up anywhere. This is populated in `getOrCreate` if dictionary is null. That is
basically when I do `create table abc with compression = { zstddict,
compression_level: ..... various levels ....}`
When I alter a table across all possible levels there there will be at most
as many entries in this map as many possible levels there are? Minimum is 1 and
max is 22 so this will have 22 entries ever at most, never GCed?
I would expect that just for sake of correctness this should get somehow
cleaned up as well. If we stop to use ZstdDictionary compressor and replace it
with something else then this will be still populated for ever.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
