KAFKA-7277: default implementation for new window store overloads

[vvcephei]

Removes the necessity for every store implementation to provide identical, trivial overrides.

Also, helps ensure that stores won't forget to validate the Instant inputs.

Committer Checklist (excluded from commit message)

• Verify design and implementation
• Verify test coverage and CI build status
• Verify documentation (including upgrade notes)

[vvcephei]

@bbejeck @mjsax @guozhangwang @nizhikov ,

This was a minor point I raised late during the PR for #5682. WDYT?

[mjsax]

Three comments:

• this was not part of the KIP and I think it requires KIP coverage (\cc @guozhangwang WDYT?)
• not sure, if we can piggy-back on KIP-372 or need a new one
• this only affects users that implement custom stores which should be rare (in not piggy-backed, is it worth to do a new KIP?)

Thus, I am not opposed but don't see big value either.

[bbejeck]

I think this would require a separate KIP. Maybe this could be piggybacked on KIP-372, but it would need updating, and these changes don't seem related to the original one.

Overall I like the consolidation of the code, but on the other hand, I don't see a big advantage in adding this either. With that in mind maybe we should hold off until later?

[vvcephei]

I think that the concerns here are fundamentally about the fact that this code was not proposed in the KIP, which is a very appropriate concern.

@nizhikov , This would be a modification to your KIP.

I think it's valuable to provide the default implementations, since it preserves compatibility for both store implementers and users of those implementations. FWIW, I'm sorry I didn't think of it earlier.

Would you be in favor of this change? And if so, could you update the KIP to mention the default methods and send an update to the vote thread?

[guozhangwang]

I think we should guarantee source compatibility in a minor release (2.1), and obviously we overlooked this issue in the KIP discussion. I'm in favor of updating the KIP with this PR accordingly. @nizhikov wdyt?

[nizhikov]

Hello @vvcephei
Personally, I like the idea to provide default implementation of new methods in interface.
Because, we reduce amount of boilerplate code.

I'm +1 for this change.

Should I update KIP-358 accordingly? Or you can do it by yourself?

[mjsax]

@nizhikov Yes, please update the KIP.

I hope this is the last change to it. Can you also send an follow up email to the VOTE thread, and summarize the latest changes that happened after the vote? Should be two things: change close(Duration) semantics and add default implementation to WindowStore methods. It's an FYI email to make sure people can object (even if we don't expect any objection).

Thanks a lot.

[mjsax]

@vvcephei Can you rename the PR? It's not minor, but should use the KIP's Jira number. Maybe also add a comment with the PR link on the ticket -- updating the title does not auto-link. Thanks.

[nizhikov]

@mjsax Done.

[vvcephei]

@mjsax Done.

[bbejeck]

LGTM. I was going to ask for tests, but I guess running the original tests is sufficient.

[mjsax]

Merged to trunk and cherry-picked to 2.1 branch.