Hi Bruno,
Thanks for reviewing the KIP, and for voting!
> I would make the constructor public and remove the static method make().
You are right. The static factory method is not providing much benefit for
the VersionedRecord class so I will remove it in order to simplify the
class.
Best,
Victo
Hi Victoria,
I am +1 on the KIP. I just have one minor comment:
Why do we need method
public static VersionedRecord make(final V value, final long
timestamp)
in the VersionedRecord?
The public constructor would do exactly the same, wouldn't it?
I would make the constructor public and remov
Thanks again for the great discussion, Sagar, Bruno, and Matthias. I've
just sent a message to start the vote on this KIP. Please have a look when
you get the chance.
Thanks,
Victoria
On Wed, Dec 14, 2022 at 12:28 PM Matthias J. Sax wrote:
> Thanks for clarifying about the null-question. SGTM.
Thanks for clarifying about the null-question. SGTM.
On 12/13/22 3:06 PM, Victoria Xia wrote:
Hi Matthias,
Thanks for chiming in! Barring objections from anyone on this thread, I
will start the vote for this KIP on Thursday. That should be enough time to
incorporate any lingering minor changes.
Hi Matthias,
Thanks for chiming in! Barring objections from anyone on this thread, I
will start the vote for this KIP on Thursday. That should be enough time to
incorporate any lingering minor changes.
> I slightly prefer to add `VersionedRecord` interface (also
like the name). I agree that it's
Thanks Victoria.
I did not re-read the KIP in full on the wiki but only your email.
Points (1)-(8) SGTM.
About (9): I slightly prefer to add `VersionedRecord` interface (also
like the name). I agree that it's low overhead and providing a clean
path forward for future changes seems worth it to
Thanks for the discussion, Bruno, Sagar, and Matthias!
It seems we've reached consensus on almost all of the discussion points.
I've updated the KIP with the following:
1) renamed "timestampTo" in `get(key, timestampTo)` to "asOfTimestamp" to
clarify that this timestamp bound is inclusive, per the
Thanks Victoria!
(1) About `ReadOnlyVersionedKeyValueStore` -- I am not sure about IQv1
vs IQv2. But you might be right that adding the interface later might
not be an issue -- so it does not matter. Just wanted to double check.
(2) About `delete(key, ts)` -- as already discussed, I agree t
Thanks Victoria,
I guess an advantage of exposing a method like delete(key, timestamp) could
be that from a user's standpoint, it is a single operation and not 2. The
equivalent of this method i.e put followed by get is not atomic so exposing
it certainly sounds like a good idea.
Thanks!
Sagar.
Hi Victoria,
Regarding
>I notice that
> there is code for validating topic configs and collecting validation
errors
> (
>
https://github.com/apache/kafka/blob/be032735b39360df1a6de1a7feea8b4336e5bcc0/streams/src/main/java/org/apache/kafka/streams/processor/internals/InternalTopicManager.java#
Thanks, Sagar and Bruno, for your insights and comments!
> Sagar: Can we name according to the semantics that you want to
support like `getAsOf` or something like that? I am not sure if we do that
in our codebase though. Maybe the experts can chime in.
Because it is a new method that will be adde
Hi all,
Thanks for the KIP, Victoria!
I have a couple of comments.
1. delete(key)
I think delete(key) should not remove all versions of a key. We should
use it to close the validity interval of the last version.
Assuming we have records of different versions for key A:
(A, e, 0, 2),
(A, f, 2,
Hi Vicky,
Thanks for your response!
I would just use numbers to refer to your comments.
1) Thanks for your response. Even I am not totally sure whether these
should be supported via IQv2 or via store interface. That said, I wouldn't
definitely qualify this as blocking the KIP for sure so we can
Thanks, Matthias and Sagar, for your comments! I've responded here for now,
and will update the KIP afterwards with the outcome of our discussions as
they resolve.
--- Matthias's comments ---
> (1) Why does the new store not extend KeyValueStore, but StateStore?
In the end, it's a
Hi Victoria,
Thanks for the KIP. Seems like a very interesting idea!
I have a couple of questions:
1) Did you consider adding a method similar to :
List> get(K key, long from, long to)?
I think this could be useful considering that this
versioning scheme unlocks time travel at a key basis. WDYT
Thanks for the KIP Victoria. Very well written!
Couple of questions (many might just require to add some more details to
the KIP):
(1) Why does the new store not extend KeyValueStore, but StateStore?
In the end, it's a KeyValueStore?
(2) Should we have a ReadOnlyVersionedKeyValueStore? E
Hi everyone,
I have a proposal for introducing versioned state stores in Kafka Streams.
Versioned state stores are similar to key-value stores except they can
store multiple record versions for a single key. This KIP focuses on
interfaces only in order to limit the scope of the KIP.
https://cwiki
17 matches
Mail list logo
