mjsax commented on code in PR #22267:
URL: https://github.com/apache/kafka/pull/22267#discussion_r3230438249
##########
streams/src/main/java/org/apache/kafka/streams/state/internals/MeteredKeyValueStore.java:
##########
@@ -182,7 +182,21 @@ private void registerMetrics() {
);
if (!persistent()) {
StateStoreMetrics.addNumKeysGauge(taskId.toString(), metricsScope,
name(), streamsMetrics,
- (config, now) -> wrapped().approximateNumEntries());
+ (config, now) -> {
+ final InMemoryKeyValueStore inMemoryStore =
findInMemoryKeyValueStore(wrapped());
+ return inMemoryStore != null ?
inMemoryStore.approximateNumEntries() : -1L;
+ }
+ );
+ }
+ }
+
+ private static InMemoryKeyValueStore findInMemoryKeyValueStore(final
StateStore store) {
+ if (store instanceof InMemoryKeyValueStore) {
+ return (InMemoryKeyValueStore) store;
+ } else if (store instanceof WrappedStateStore) {
+ return findInMemoryKeyValueStore(((WrappedStateStore<?, ?, ?>)
store).wrapped());
+ } else {
+ return null;
Review Comment:
Ah. Custom stores. Good point.
> I guess the question is do we want to register this metric when a custom
state store is used?
I don't think we can -- there is no public API for it -- we use
`numEntries()` which is only added on our specific built-in classes (it's not
declared on any public interface).
For KV-store case, it would be different, because we don't have a new
`numEntries()` but re-use the existing `approximateNumEntries()`, but adding it
only for custom KV-stores, would be a little bit odd, too. In double, we would
need to change `approximateNumEntries()` and maybe move if to `StateStore`
interface, such that all stores would implement it. But this would require a
new KIP.
I think it's fine as-is. Custom stores a rare, and if it doesn't work on
custom stores, and there is demand, we can address in the future.
--
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]
