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-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
