jyothsnakonisa commented on code in PR #4521:
URL: https://github.com/apache/cassandra/pull/4521#discussion_r2621119246
##########
src/java/org/apache/cassandra/db/compression/CompressionDictionaryCache.java:
##########
@@ -102,7 +102,12 @@ public void add(@Nullable CompressionDictionary
compressionDictionary)
// Only update cache if not already in the cache
DictId newDictId = compressionDictionary.dictId();
- cache.get(newDictId, id -> compressionDictionary);
+ CompressionDictionary cached = cache.get(newDictId, id ->
compressionDictionary);
+ if (cached != compressionDictionary)
+ {
+ compressionDictionary.close();
+ return;
+ }
Review Comment:
@smiklosovic The `add()` method is closing dictionaries that this class
didn't create. Callers may attempt to use dictionaries that have been silently
closed. have you considered other alternatives? may be like
1. Moving `SystemDistributedKeyspace.retrieveCompressionDictionary` call to
`add()` method and handling the close in the same method
2. `add ()` method indicating the caller that the dictionary has to be
closed and the caller takes action based on the result
3. Any other options
--
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]
