vvcephei commented on a change in pull request #9388:
URL: https://github.com/apache/kafka/pull/9388#discussion_r501435613
##########
File path:
streams/src/main/java/org/apache/kafka/streams/state/internals/MeteredWindowStore.java
##########
@@ -122,7 +158,7 @@ public boolean setFlushListener(final
CacheFlushListener<Windowed<K>, V> listene
@Override
public void put(final K key,
final V value) {
- put(key, value, context.timestamp());
+ put(key, value, context != null ? context.timestamp() : 0L);
Review comment:
Yeah, it's not something I normally like to do, either. In this case,
though, it's necessary. The thing is that all our internal StateStoreContexts
are InternalProcessorContext implementations, and therefore, they are also
ProcessorContext implementations, so they have a `timestamp()` method.
The thing that makes this unavoidable is that it's ok for users to `init` a
state store using the MockProcessorContext we provide for them in `test-utils`.
This is a bit of a bleed-over from the _next_ pr, which I'm still finishing up,
but it's better if we keep their context "pure". I.e., I'm going to propose to
add a new context that's _just_ an `api.ProcessorContext` and a separate
implementation that _just_ a `StateStoreContext`. We should discuss on that PR
whether that's really the best way to present it, but if you ultimately agree,
then it means we have to expect a null context here.
Note that the only functionality it affects is the recording of metrics that
probably don't matter in unit tests and this stub behavior for a deprecated
method that people shouldn't be using.
If after reviewing the next PR, we do wind up converging the
implementations, I'll come back and undo these checks here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
