[GitHub] [kafka] guozhangwang commented on a diff in pull request #13369: KAFKA-14172: Should clear cache when active recycled from standby

["guozhangwang (via GitHub)" <gi...@apache.org>]

guozhangwang commented on code in PR #13369:
URL: https://github.com/apache/kafka/pull/13369#discussion_r1158888604


##########
streams/src/main/java/org/apache/kafka/streams/state/internals/NamedCache.java:
##########
@@ -361,6 +361,13 @@ synchronized void close() {
         streamsMetrics.removeAllCacheLevelSensors(Thread.currentThread().getName(), taskName, storeName);
     }
 
+    synchronized void clear() {

Review Comment:
   Yes, since the call is only triggered in `stateMgr#recycle` but this is just another thing that I do not like the most: relying on the caller to do the right thing. I did not try to call another flush in recycle since, as I made it in the TODO comment, both `flushCache` and `clearCache` would likely be removed, while the "clean fix" to me would be that:
   
   0. we decouple flushing from suppression, where the caching layer would be purely a thread/task concept. And then:
   1. we do not need to flush caches upon committing at all, hence `flushCache` can be removed.
   2. upon recycling or closing the state manager, we just trigger a flush/close on the `ThreadCache -> NamedCache` on the thread level instead, which would both flushing the cache to write to the underlying store (without doing any emitting), as well as clearing the cache.
   
   In this way, The `CachedStore` interface can also be removed and we would not need either interface funcs.
   
   Of course, with that all being said, if you feel it's safer to still do it, I can add a flushing call in `clear as well` and modify the javadoc comments :P



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org