John, It's exciting to see this KIP head in this direction! In the last year or so I've tried to sketch out some usability improvements for custom state stores, and I also ended up splitting out the StateStoreContext from the ProcessorContext in an attempt to facilitate what I was doing. I sort of abandoned it when I realized how large the ideal change might have to be, but it's great to see that there is other interest in moving in this direction (from the folks that matter :) ).
Having taken a stab at it myself, I have a comment/question on this bullet about StateStoreContext: It does *not* include anything processor- or record- specific, like > `forward()` or any information about the "current" record, which is only a > well-defined in the context of the Processor. Processors process one record > at a time, but state stores may be used to store and fetch many records, so > there is no "current record". > I totally agree that record-specific or processor-specific context in a state store is often not well-defined and it would be good to separate that out, but sometimes it (at least record-specific context) is actually useful, for example, passing the record's timestamp through to the underlying storage (or changelog topic): https://github.com/apache/kafka/blob/trunk/streams/src/main/java/org/apache/kafka/streams/state/internals/ChangeLoggingKeyValueBytesStore.java#L121 You could have the writer client of the state store pass this through, but it would be nice to be able to write state stores where the client did not have this responsibility. I'm not sure if the solution is to add some things back to StateStoreContext, or make yet another context that represents record-specific context while inside a state store. Best, Paul On Wed, Sep 9, 2020 at 5:43 PM John Roesler <[email protected]> wrote: > Hello all, > > I've been slowly pushing KIP-478 forward over the last year, > and I'm happy to say that we're making good progress now. > However, several issues with the original design have come > to light. > > The major changes: > > We discovered that the original plan of just adding generic > parameters to ProcessorContext was too disruptive, so we are > now adding a new api.ProcessorContext. > > That choice forces us to add a new StateStore.init method > for the new context, but ProcessorContext really isn't ideal > for state stores to begin with, so I'm proposing a new > StateStoreContext for this purpose. In a nutshell, there are > quite a few methods in ProcessorContext that actually should > never be called from inside a StateStore. > > Also, since there is a new ProcessorContext interface, we > need a new MockProcessorContext implementation in the test- > utils module. > > > > The changeset for the KIP document is here: > > https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=118172121&selectedPageVersions=14&selectedPageVersions=10 > > And the KIP itself is here: > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-478+-+Strongly+typed+Processor+API > > > If you have any concerns, please let me know! > > Thanks, > -John > >
