Hi all, Thank you for the feedback. So we agreed on no default ordering for keys and TSs. So I must provide both withAscendingXx() and with DescendingXx() for the class. Apart from that, I think we can either remove the existing constructor for the `VersionedRecord` class or follow the `Optional` thing.
Since many hidden aspects of the KIP are quite clear now and we have come to a consensus about them, I think it 's time to vote ;-) I look forward to your votes. Thanks a lot. Cheers, Alieh On Fri, Nov 17, 2023 at 2:27 AM Matthias J. Sax <mj...@apache.org> wrote: > Thanks, Alieh. > > Overall SGTM. About `validTo` -- wondering if we should make it an > `Optional` and set to `empty()` by default? > > I am totally ok with going with the 3-way option about ordering using > default "undefined". For this KIP (as it's all net new) nothing really > changes. -- However, we should amend `RangeQuery`/KIP-985 to align it. > > Btw: so far we focused on key-ordering, but I believe the same "ordering > undefined by default" would apply to time-ordering, too? This might > affect KIP-997, too. > > > -Matthias > > On 11/16/23 12:51 AM, Bruno Cadonna wrote: > > Hi, > > > > 80) > > We do not keep backwards compatibility with IQv1, right? I would even > > say that currently we do not need to keep backwards compatibility among > > IQv2 versions since we marked the API "Evolving" (do we only mean code > > compatibility here or also behavioral compatibility?). I propose to try > > to not limit ourselves for backwards compatibility that we explicitly > > marked as evolving. > > I re-read the discussion on KIP-985. In that discussion, we were quite > > focused on what the state store provides. I see that for range queries, > > we have methods on the state store interface that specify the order, but > > that should be kind of orthogonal to the IQv2 query type. Let's assume > > somebody in the future adds a state store implementation that is not > > order based. To account for use cases where the order does not matter, > > this person might also add a method to the state store interface that > > does not guarantee any order. However, our range query type is specified > > to guarantee order by default. So we need to add something like > > withNoOrder() to the query type to allow the use cases that does not > > need order and has the better performance in IQ. That does not look very > > nice to me. Having the no-order-guaranteed option does not cost us > > anything and it keeps the IQv2 interface flexible. I assume we want to > > drop the Evolving annotation at some point. > > Sorry for not having brought this up in the discussion about KIP-985. > > > > Best, > > Bruno > > > > > > > > > > > > On 11/15/23 6:56 AM, Matthias J. Sax wrote: > >> Just catching up on this one. > >> > >> > >> 50) I am also in favor of setting `validTo` in VersionedRecord for > >> single-key single-ts lookup; it seems better to return the proper > >> timestamp. The timestamp is already in the store and it's cheap to > >> extract it and add to the result, and it might be valuable information > >> for the user. Not sure though if we should deprecate the existing > >> constructor though, because for "latest" it's convenient to have? > >> > >> > >> 60) Yes, I meant `VersionedRecord`. Sorry for the mixup. > >> > >> > >> 80) We did discuss this question on KIP-985 (maybe you missed it > >> Bruno). It's kinda tricky. > >> > >> Historically, it seems that IQv1, ie, the `ReadOnlyXxx` interfaces > >> provide a clear contract that `range()` is ascending and > >> `reverseRange()` is descending. > >> > >> For `RangeQuery`, the question is, if we did implicitly inherit this > >> contract? Our conclusion on KIP-985 discussion was, that we did > >> inherit it. If this holds true, changing the contract would be a > >> breaking change (what might still be acceptable, given that the > >> interface is annotated as unstable, and that IQv2 is not widely > >> adopted yet). I am happy to go with the 3-option contract, but just > >> want to ensure we all agree it's the right way to go, and we are > >> potentially willing to pay the price of backward incompatibility. > >> > >> > >> > >>> Do we need a snapshot semantic or can we specify a weaker but still > >>> useful semantic? > >> > >> I don't think we necessarily need it, but as pointed out by Lucas, all > >> existing queries provide it. Overall, my main point is really about > >> not implementing something "random", but defining a proper binding > >> contract that allows users to reason about it. > >> > >> I general, I agree that weaker semantics might be sufficient, but I am > >> not sure if we can implement anything weaker in a reasonable way? > >> Happy to be convinced otherwise. (I have some example, that I will > >> omit for now, as I hope we can actually go with snapshot semantics.) > >> > >> The RocksDB Snaptshot idea from Lucas sounds very promising. I was not > >> aware that we could do this with RocksDB (otherwise I might have > >> suggested it on the PR right away). I guess the only open question > >> remaining would be, if we can provide the same guarantees for a future > >> in-memory implementation for VersionedStores? It sounds possible to > >> do, but we should have some level of confidence about it? > >> > >> > >> -Matthias > >> > >> On 11/14/23 6:33 AM, Lucas Brutschy wrote: > >>> Hi Alieh, > >>> > >>> I agree with Bruno that a weaker guarantee could be useful already, > >>> and it's anyway possible to strengthen the guarantees later on. On the > >>> other hand, it would be nice to provide a consistent view across all > >>> segments if it doesn't come with major downsides, because until now IQ > >>> does provide a consistent view (via iterators), and this would be the > >>> first feature that diverges from that guarantee. > >>> > >>> I think a consistent could be achieved relatively easily by creating a > >>> snapshot (https://github.com/facebook/rocksdb/wiki/Snapshot) that will > >>> ensure a consistent view on a single DB and using > >>> `ReadOptions.setSnapshot` for the gets. Since versioned state stores > >>> segments seem to be backed by a single RocksDB instance (that was > >>> unclear to me during our earlier discussion), a single snapshot should > >>> be enough to guarantee a consistent view - we just need to make sure > >>> to clean up the snapshots after use, similar to iterators. If we > >>> instead need a consistent view across multiple RocksDB instances, we > >>> may have to acquire a write lock on all of those (could use the > >>> current object monitors of our `RocksDB` interface) for the duration > >>> of creating snapshots across all instances - which I think would also > >>> be permissible performance-wise. Snapshots are just a sequence number > >>> and should be pretty lightweight to create (they have, however, > >>> downside when it comes to compaction just like iterators). > >>> > >>> With that in mind, I would be in favor of at least exploring the > >>> option of using snapshots for a consistent view here, before dropping > >>> this useful guarantee. > >>> > >>> Cheers, > >>> Lucas > >>> > >>> On Tue, Nov 14, 2023 at 2:20 PM Bruno Cadonna <cado...@apache.org> > >>> wrote: > >>>> > >>>> Hi Alieh, > >>>> > >>>> Regarding the semantics/guarantees of the query type: > >>>> > >>>> Do we need a snapshot semantic or can we specify a weaker but still > >>>> useful semantic? > >>>> > >>>> An option could be to guarantee that: > >>>> 1. the query returns the latest version when the query arrived at the > >>>> state store > >>>> 2. the query returns a valid history, i.e., versions with adjacent and > >>>> non-overlapping validity intervals. > >>>> > >>>> I think guarantee 1 is quite obvious. Guarantee 2 maybe needs some > >>>> explanation. If we do not avoid writes to the state store during the > >>>> processing of interactive queries, it might for example happen that > the > >>>> latest version in the state store moves to data structures that are > >>>> responsible for older versions. In our RocksDB implementation that are > >>>> the segments. Thus, it could be that during query processing Kafka > >>>> Streams reads the latest value x and encounters again x in a segment > >>>> because a processor put a newer version of x in the versioned state > >>>> store. A similar scenario might also happen to earlier versions. If > >>>> Streams does not account for such cases it could return invalid > >>>> histories. > >>>> > >>>> Maybe such weaker guarantees are enough for most use cases. > >>>> > >>>> You could consider implementing weaker guarantees as I described and > if > >>>> there is demand propose stricter guarantees in a follow-up KIP. > >>>> > >>>> Maybe there are also other simpler guarantees that make sense. > >>>> > >>>> Best, > >>>> Bruno > >>>> > >>>> > >>>> On 11/9/23 12:30 PM, Bruno Cadonna wrote: > >>>>> Hi, > >>>>> > >>>>> Thanks for the updates! > >>>>> > >>>>> First my take on previous comments: > >>>>> > >>>>> > >>>>> 50) > >>>>> I am in favor of deprecating the constructor that does not take the > >>>>> validTo parameter. That implies that I am in favor of modifying > >>>>> get(key, > >>>>> asOf) to set the correct validTo. > >>>>> > >>>>> > >>>>> 60) > >>>>> I am in favor of renaming ValueIterator to VersionedRecordIterator > and > >>>>> define it as: > >>>>> > >>>>> public interface VersionedRecordIterator<V> extends > >>>>> Iterator<VersionedRecord<V>> > >>>>> > >>>>> (Matthias, you mixed up ValueAndTimestamp with VersionedRecord in > your > >>>>> last e-mail, didn't you? Just double-checking if I understood what > you > >>>>> are proposing.) > >>>>> > >>>>> > >>>>> 70) > >>>>> I agree with Matthias that adding a new method on the > >>>>> VersionedKeyValueStore interface defeats the purpose of one of the > >>>>> goals > >>>>> of IQv2, i.e., not to need to extend the state store interface for > >>>>> IQ. I > >>>>> would say if we do not need the method in normal processing, we > should > >>>>> not extend the public state store interface. BTW, nobody forces you > to > >>>>> StoreQueryUtils. I think that internal utils class was introduced for > >>>>> convenience to leverage existing methods on the state store > interface. > >>>>> > >>>>> > >>>>> 80) > >>>>> Why do we limit ourselves by specifying a default order on the > result? > >>>>> Different state store implementation might have different > >>>>> strategies to > >>>>> store the records which affects the order in which the records are > >>>>> returned if they are not sorted before they are returned to the user. > >>>>> Some users might not be interested in an order of the result and so > >>>>> there is no reason those users pay the cost for sorting. I propose to > >>>>> not specify a default order but sort the results (if needed) when > >>>>> withDescendingX() and withAscendingX() is specified on the query > >>>>> object. > >>>>> > >>>>> > >>>>> Regarding the snapshot guarantees for the iterators, I need to think > a > >>>>> bit more about it. I will come back to this thread in the next days. > >>>>> > >>>>> > >>>>> Best, > >>>>> Bruno > >>>>> > >>>>> > >>>>> On 11/8/23 5:30 PM, Alieh Saeedi wrote: > >>>>>> Thank you, Bruno and Matthias, for keeping the discussion going > >>>>>> and for > >>>>>> reviewing the PR. > >>>>>> > >>>>>> Here are the KIP updates: > >>>>>> > >>>>>> - I removed the `peek()` from the `ValueIterator` interface > >>>>>> since > >>>>>> we do > >>>>>> not need it. > >>>>>> - Yes, Bruno, the `validTo` field in the `VersionedRecod` > >>>>>> class is > >>>>>> exclusive. I updated the javadocs for that. > >>>>>> > >>>>>> > >>>>>> Very important critical open questions. I list them here based on > >>>>>> priority > >>>>>> (descendingly). > >>>>>> > >>>>>> - I implemented the `get(key, fromtime, totime)` method here > >>>>>> > >>>>>> < > https://github.com/aliehsaeedii/kafka/blob/9578b7cb7cdade22cc63f671693f5aeb993937ca/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBVersionedStore.java#L262 > >: > >>>>>> the problem is that this implementation does not guarantee > >>>>>> consistency > >>>>>> because processing might continue interleaved (no snapshot > >>>>>> semantics is > >>>>>> implemented). More over, it materializes all results in memory. > >>>>>> - Solution 1: Use a lock and release it after retrieving all > >>>>>> desired > >>>>>> records from all segments. > >>>>>> - positive point: snapshot semantics is implemented > >>>>>> - negative points: 1) It is expensive since iterating > >>>>>> over all > >>>>>> segments may take a long time. 2) It still requires > >>>>>> materializing results > >>>>>> on memory > >>>>>> - Solution 2: use `RocksDbIterator`. > >>>>>> - positive points: 1) It guarantees snapshot segments. > >>>>>> 2) It > >>>>>> does > >>>>>> not require materializing results in memory. > >>>>>> - negative points: it is expensive because, anyway, we > >>>>>> need to > >>>>>> iterate over all (many) segments. > >>>>>> > >>>>>> Do you have any thoughts on this issue? (ref: > Matthias's > >>>>>> comment > >>>>>> < > https://github.com/apache/kafka/pull/14626#pullrequestreview-1709280589>) > >>>>>> > >>>>>> - I added the field `validTo` in `VersionedRecord`. Its default > >>>>>> value is > >>>>>> MAX. But as Matthias mentioned, for the single-key single-ts > >>>>>> (`VersionedKeyQuery` in KIP-960), it may not always be true. > >>>>>> If the > >>>>>> returned record belongs to an old segment, maybe it is not > valid > >>>>>> any more. > >>>>>> So MAX is not the correct value for `ValidTo`. Two solutions > >>>>>> come > >>>>>> to mind: > >>>>>> - Solution 1: make the `ValidTo` as an `Optional` and set > >>>>>> it to > >>>>>> `empty` for the retuned result of `get(key, asOfTimestamp)`. > >>>>>> - Solution 2: change the implementation of `get(key, > >>>>>> asOfTimestamp)` > >>>>>> so that it finds the correct `validTo` for the returned > >>>>>> versionedRecord. > >>>>>> > >>>>>> - In this KIP and the next one, even though the default > >>>>>> ordering is > >>>>>> with ascending ts, I added the method > >>>>>> `withAscendingTimestamps()` > >>>>>> to have > >>>>>> more user readibility (as Bruno suggested), while Hanyu did > >>>>>> only add > >>>>>> `withDescending...` methods (he did not need ascending because > >>>>>> that's the > >>>>>> default anyway). Matthias believes that we should not have > >>>>>> inconsistencies (he actually hates them:D). Shall I change my > >>>>>> KIP > >>>>>> or Hanyu? > >>>>>> Thoughts? > >>>>>> > >>>>>> > >>>>>> That would be maybe helpful to look into the PR > >>>>>> <https://github.com/apache/kafka/pull/14626> for more clarity and > >>>>>> even > >>>>>> review that ;-) > >>>>>> > >>>>>> Cheers, > >>>>>> Alieh > >>>>>> > >>>>>> On Thu, Nov 2, 2023 at 7:13 PM Bruno Cadonna <cado...@apache.org> > >>>>>> wrote: > >>>>>> > >>>>>>> Hi Alieh, > >>>>>>> > >>>>>>> First of all, I like the examples. > >>>>>>> > >>>>>>> Is validTo in VersionedRecord exclusive or inclusive? > >>>>>>> In the javadocs you write: > >>>>>>> > >>>>>>> "@param validTo the latest timestamp that value is valid" > >>>>>>> > >>>>>>> I think that is not true because the validity is defined by the > >>>>>>> start > >>>>>>> time of the new version. The new and the old version cannot both be > >>>>>>> valid at that same time. > >>>>>>> > >>>>>>> Theoretically, you could set validTo to the start time of the new > >>>>>>> version - 1. However, what is the unit of the 1? Is it nanoseconds? > >>>>>>> Milliseconds? Seconds? Sure we could agree on one, but I think it > is > >>>>>>> more elegant to just make the validTo exclusive. Actually, you > >>>>>>> used it > >>>>>>> as exclusive in your examples. > >>>>>>> > >>>>>>> > >>>>>>> Thanks for the KIP! > >>>>>>> > >>>>>>> Best, > >>>>>>> Bruno > >>>>>>> > >>>>>>> On 11/1/23 9:01 PM, Alieh Saeedi wrote: > >>>>>>>> Hi all, > >>>>>>>> @Matthias: I think Victoria was right. I must add the method > >>>>>>>> `get(key, > >>>>>>>> fromTime, toTime)` to the interface `VersionedKeyValueStore`. > Right > >>>>>>>> now, > >>>>>>>> having the method only in `RocksDBVersionedStore`, made me to > >>>>>>>> have an > >>>>>>>> instance of `RocksDBVersionedStore` (instead of > >>>>>>>> `VersionedKeyValueStore`) > >>>>>>>> in `StoreQueryUtils.runMultiVersionedKeyQuery()` method. In > >>>>>>>> future, we > >>>>>>> are > >>>>>>>> going to use the same method for In-memory/SPDB/blaBla versioned > >>>>>>>> stores. > >>>>>>>> Then either this method won't work any more, or we have to add > >>>>>>>> code (if > >>>>>>>> clauses) for each type of versioned stores. What do you think > about > >>>>>>>> that? > >>>>>>>> > >>>>>>>> Bests, > >>>>>>>> Alieh > >>>>>>>> > >>>>>>>> On Tue, Oct 24, 2023 at 10:01 PM Alieh Saeedi > >>>>>>>> <asae...@confluent.io> > >>>>>>> wrote: > >>>>>>>> > >>>>>>>>> Thank you, Matthias, Bruno, and Guozhang for keeping the > >>>>>>>>> discussion > >>>>>>> going. > >>>>>>>>> > >>>>>>>>> Here is the list of changes I made: > >>>>>>>>> > >>>>>>>>> 1. I enriched the "Example" section as Bruno suggested. > >>>>>>>>> Do you > >>>>>>> please > >>>>>>>>> have a look at that section? I think I devised an > >>>>>>>>> interesting one > >>>>>>> ;-) > >>>>>>>>> 2. As Matthias and Guozhang suggested, I renamed > >>>>>>>>> variables and > >>>>>>> methods > >>>>>>>>> as follows: > >>>>>>>>> - "fromTimestamp" -> "fromTime" > >>>>>>>>> - "asOfTimestamp" -> "toTime" > >>>>>>>>> - "from(Instant fromTime)" -> "fromTime(Instant > >>>>>>>>> fromTime)" > >>>>>>>>> - "asOf(Instant toTime)" -> "toTime(Instant toTime)" > >>>>>>>>> 3. > >>>>>>>>> > >>>>>>>>> About throwing an NPE when time bounds are `null`: Ok, > >>>>>>>>> Matthias, I > >>>>>>> am > >>>>>>>>> convinced by your idea. I consider a null timestamp as "no > >>>>>>>>> bound". > >>>>>>> But I > >>>>>>>>> had to change KIP-960 and the corresponding PR to be > >>>>>>>>> consistent as > >>>>>>> well. > >>>>>>>>> > >>>>>>>>> Answering questions and some more discussion points: > >>>>>>>>> > >>>>>>>>> 1. > >>>>>>>>> > >>>>>>>>> For now, I keep the class names as they are. > >>>>>>>>> 2. > >>>>>>>>> > >>>>>>>>> About the new field "validTo" in VersionedRecord. Yes > >>>>>>>>> Matthias I > >>>>>>> keep > >>>>>>>>> the old constructor, which does not have `validTo` as an > >>>>>>>>> input > >>>>>>> parameter. > >>>>>>>>> But in the body of the old constructor, I consider the > >>>>>>>>> default > >>>>>>> value MAX > >>>>>>>>> for the validTo field. I think MAX must be the default > >>>>>>>>> value for > >>>>>>> `validTo` > >>>>>>>>> since before inserting a tombstone or a new value for the > >>>>>>>>> same > >>>>>>>>> key, > >>>>>>> the > >>>>>>>>> value must be valid till iternity. > >>>>>>>>> 3. > >>>>>>>>> > >>>>>>>>> Regarding changing the ValueIterator interface from `public > >>>>>>> interface > >>>>>>>>> ValueIterator<V> extends Iterator<V>` to `public interface > >>>>>>>>> ValueIterator<V> extends Iterator<VersionedRecord<V>>`: > >>>>>>>>> Matthias, I > >>>>>>> do not > >>>>>>>>> know how it improves type safety because the > >>>>>>>>> MultiVersionedKeyQuery > >>>>>>> class > >>>>>>>>> returns a ValueIterator of VersionedRecord any way. But > >>>>>>>>> if we > >>>>>>>>> want > >>>>>>> to be > >>>>>>>>> consistent with KeyValueIterator, we must apply the > >>>>>>>>> changes you > >>>>>>> suggested. > >>>>>>>>> 4. > >>>>>>>>> > >>>>>>>>> Regarding adding the new `get(key, fromTime, toTime)` > method > >>>>>>>>> to the > >>>>>>>>> public interface `VersionedKeyValueStore` or only adding > >>>>>>>>> it to > >>>>>>>>> the > >>>>>>>>> class `RocksDBVersionedStore`: In the KIP, I changed the > >>>>>>>>> interface > >>>>>>> as > >>>>>>>>> Victoria suggested. But still, I am not convinced why we > >>>>>>>>> do that. > >>>>>>> @Victoria: > >>>>>>>>> Do you please clarify why we have to define it in the > >>>>>>>>> interface? > >>>>>>> More > >>>>>>>>> specifically, why do we need to use generic > >>>>>>>>> VersionedKeyValueStore > >>>>>>>>> instead of simply using the implementing classes? > >>>>>>>>> > >>>>>>>>> Cheers, > >>>>>>>>> Alieh > >>>>>>>>> > >>>>>>>>> On Sat, Oct 14, 2023 at 3:35 AM Guozhang Wang < > >>>>>>> guozhang.wang...@gmail.com> > >>>>>>>>> wrote: > >>>>>>>>> > >>>>>>>>>> Thanks Alieh for the KIP, as well as a nice summary of all the > >>>>>>>>>> discussions! Just my 2c regarding your open questions: > >>>>>>>>>> > >>>>>>>>>> 1. I just checked KIP-889 > >>>>>>>>>> ( > >>>>>>>>>> > >>>>>>> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-889%3A+Versioned+State+Stores > >>>>>>>>>> ) > >>>>>>>>>> and we used "VersionedRecord<V> get(K key, long > asOfTimestamp);", > >>>>>>>>>> so I > >>>>>>>>>> feel to be consistent with this API is better compared with > being > >>>>>>>>>> consistent with "WindowKeyQuery"? > >>>>>>>>>> > >>>>>>>>>> 3. I agree with Matthias that naming is always tricky, and I > also > >>>>>>>>>> tend > >>>>>>>>>> to be consistent with what we already have (even if in retro > >>>>>>>>>> it may > >>>>>>>>>> not be the best idea :P and if that was really becoming a > >>>>>>>>>> complaint, > >>>>>>>>>> we would change it across the board in one shot as well later). > >>>>>>>>>> > >>>>>>>>>> Guozhang > >>>>>>>>>> > >>>>>>>>>> On Wed, Oct 11, 2023 at 9:12 PM Matthias J. Sax > >>>>>>>>>> <mj...@apache.org> > >>>>>>> wrote: > >>>>>>>>>>> > >>>>>>>>>>> Thanks for the update! > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> Some thoughts about changes you made and open questions you > >>>>>>>>>>> raised: > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> 10) About asOf vs until: I was just looking into > >>>>>>>>>>> `WindowKeyQuery`, > >>>>>>>>>>> `WindowRangeQuery` and also `ReadOnlyWindowStore` interfaces. > >>>>>>>>>>> For > >>>>>>> those, > >>>>>>>>>>> we use "timeFrom" and "timeTo", so it seems best to actually > use > >>>>>>>>>>> `to(Instant toTime)` to keep the naming consistent across the > >>>>>>>>>>> board? > >>>>>>>>>>> > >>>>>>>>>>> If yes, we should also do `from (Instant fromTime)` and use > >>>>>>>>>>> getters > >>>>>>>>>>> `fromTime()` and `toTime()` -- given that it's range bounds > >>>>>>>>>>> it seems > >>>>>>>>>>> acceptable to me, to diverge a little bit from KIP-960 > >>>>>>> `asOfTimestamp()` > >>>>>>>>>>> -- but we could also rename it to `asOfTime()`? -- Given that > we > >>>>>>>>>>> strongly type with `Instant` I am not worried about semantic > >>>>>>> ambiguity. > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> 20) About throwing a NPE when time bounds are `null` -- why? > >>>>>>>>>>> (For > >>>>>>>>>>> the > >>>>>>>>>>> key it makes sense as is mandatory to have a key.) Could we not > >>>>>>>>>>> interpret `null` as "no bound". We did KIP-941 to add `null` > for > >>>>>>>>>>> open-ended `RangeQueries`, so I am wondering if we should > >>>>>>>>>>> just stick > >>>>>>> to > >>>>>>>>>>> the same semantics? > >>>>>>>>>>> > >>>>>>>>>>> Cf > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-941%3A+Range+queries+to+accept+null+lower+and+upper+bounds > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> 30) About the class naming. That's always tricky, and I am not > >>>>>>>>>>> married > >>>>>>>>>>> to my proposal. I agree with Bruno that the other suggested > >>>>>>>>>>> names > >>>>>>>>>>> are > >>>>>>>>>>> not really better. -- The underlying idea was, to get some > >>>>>>> "consistent" > >>>>>>>>>>> naming across the board. > >>>>>>>>>>> > >>>>>>>>>>> Existing `KeyQuery` > >>>>>>>>>>> New `VersionedKeyQuery` (KIP-960; we add a prefix) > >>>>>>>>>>> New `MultiVersionKeyQuery` (this KIP; extend the prefix with a > >>>>>>>>>> pre-prefix) > >>>>>>>>>>> > >>>>>>>>>>> Existing `RangeQuery` > >>>>>>>>>>> New `MultiVersionRangeQuery` (KIP-969; add same prefix as > above) > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> 40) I am fine with not adding `range(from, to)` -- it was > >>>>>>>>>>> just an > >>>>>>> idea. > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> Some more follow up question: > >>>>>>>>>>> > >>>>>>>>>>> 50) You propose to add a new constructor and getter to > >>>>>>> `VersionedRecord` > >>>>>>>>>>> -- I am wondering if this implies that `validTo` is optional > >>>>>>>>>>> because > >>>>>>> the > >>>>>>>>>>> existing constructor is not deprecated? -- Also, what happens > if > >>>>>>>>>>> `validTo` is not set and `valueTo()` is called? Or do we > >>>>>>>>>>> intent to > >>>>>>> make > >>>>>>>>>>> `validTo` mandatory? > >>>>>>>>>>> > >>>>>>>>>>> Maybe this question can only be answered when working on the > >>>>>>>>>>> code, > >>>>>>> but I > >>>>>>>>>>> am wondering if we should make `validTo` mandatory or not... > And > >>>>>>>>>>> what > >>>>>>>>>>> the "blast radius" of changing `VersionedRecord` will be in > >>>>>>>>>>> general. > >>>>>>> Do > >>>>>>>>>>> you have already some POC PR that we could look at to get some > >>>>>>>>>>> signals > >>>>>>>>>>> about this? > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> 60) The new query class is defined to return > >>>>>>>>>>> `ValueIterator<VersionedRecord<V>>` -- while I like the idea > >>>>>>>>>>> to add > >>>>>>>>>>> `ValueIterator<V>` in a generic way on the one hand, I am > >>>>>>>>>>> wondering if > >>>>>>>>>>> it might be better to change it, and enforce its usage (ie, > >>>>>>>>>>> return > >>>>>>> type) > >>>>>>>>>>> of `VersionedRecord` to improve type safety (type erasure is > >>>>>>>>>>> often a > >>>>>>>>>>> pain, and we could mitigate it this way). > >>>>>>>>>>> > >>>>>>>>>>> Btw: We actually do a similar thing for `KeyValueIterator`. > >>>>>>>>>>> > >>>>>>>>>>> Ie, > >>>>>>>>>>> > >>>>>>>>>>> public interface ValueIterator<V> extends > >>>>>>> Iterator<ValueAndTimestamp<V>> > >>>>>>>>>>> > >>>>>>>>>>> and > >>>>>>>>>>> > >>>>>>>>>>> ValueAndTimestamp<V> peek(); > >>>>>>>>>>> > >>>>>>>>>>> This would imply that the return type of the new query is > >>>>>>>>>>> `ValueIterator<V>` on the interface what seems simpler and more > >>>>>>> elegant? > >>>>>>>>>>> > >>>>>>>>>>> If we go with the change, I am also wondering if we need to > >>>>>>>>>>> find a > >>>>>>>>>>> better name for the new iterator class? Maybe > >>>>>>>>>>> `VersionIterator` or > >>>>>>>>>>> something like this? > >>>>>>>>>>> > >>>>>>>>>>> Of course it might limit the use of `ValueIterator` for other > >>>>>>>>>>> value > >>>>>>>>>>> types -- not sure if this a limitation that is prohibitive? > >>>>>>>>>>> My gut > >>>>>>>>>>> feeling is, that is should not be too limiting. > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> 70) Do we really need the change in `VersionedKeyValueStore` > and > >>>>>>>>>>> add a > >>>>>>>>>>> new method? In the end, the idea of IQv2 is to avoid exactly > >>>>>>>>>>> this... > >>>>>>> It > >>>>>>>>>>> was the main issue for IQv1, that the base interface of the > >>>>>>>>>>> store > >>>>>>> needed > >>>>>>>>>>> an update and thus all classed implementing the base interface, > >>>>>>>>>>> making > >>>>>>>>>>> it very cumbersome to add new query types. -- Of course, we > need > >>>>>>>>>>> this > >>>>>>>>>>> new method on the actually implementation (as private method) > >>>>>>>>>>> that can > >>>>>>>>>>> be called from `query()` method, but adding it to the interface > >>>>>>>>>>> seems > >>>>>>> to > >>>>>>>>>>> defeat the purpose of IQv2. > >>>>>>>>>>> > >>>>>>>>>>> Note, for existing IQv2 queries types that go against others > >>>>>>>>>>> stores, > >>>>>>> the > >>>>>>>>>>> public methods already existed when IQv2 was introduces, and > >>>>>>>>>>> thus > >>>>>>>>>>> the > >>>>>>>>>>> implementation of these query types just pragmatically re-used > >>>>>>> existing > >>>>>>>>>>> methods -- but it does not imply that new public method > >>>>>>>>>>> should be > >>>>>>> added. > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> -Matthias > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> On 10/11/23 5:11 AM, Bruno Cadonna wrote: > >>>>>>>>>>>> Thanks for the updates, Alieh! > >>>>>>>>>>>> > >>>>>>>>>>>> The example in the KIP uses the allVersions() method which we > >>>>>>>>>>>> agreed > >>>>>>>>>> to > >>>>>>>>>>>> remove. > >>>>>>>>>>>> > >>>>>>>>>>>> Regarding your questions: > >>>>>>>>>>>> 1. asOf vs. until: I am fine with both but slightly prefer > >>>>>>>>>>>> until. > >>>>>>>>>>>> 2. What about KeyMultiVersionsQuery, KeyVersionsQuery (KIP-960 > >>>>>>>>>>>> would > >>>>>>>>>>>> then be KeyVersionQuery). However, I am also fine with > >>>>>>>>>>>> MultiVersionedKeyQuery since none of the names sounds better > or > >>>>>>>>>>>> worse > >>>>>>>>>> to > >>>>>>>>>>>> me. > >>>>>>>>>>>> 3. I agree with you not to introduce the method with the two > >>>>>>>>>>>> bounds > >>>>>>> to > >>>>>>>>>>>> keep things simple. > >>>>>>>>>>>> 4. Forget about fromTime() an asOfTime(), from() and asOf() is > >>>>>>>>>>>> fine. > >>>>>>>>>>>> 5. The main purpose is to show how to use the API. Maybe > >>>>>>>>>>>> make an > >>>>>>>>>> example > >>>>>>>>>>>> with just the key to distinguish this query from the single > >>>>>>>>>>>> value > >>>>>>>>>> query > >>>>>>>>>>>> of KIP-960 and then one with a key and a time range. When you > >>>>>>>>>>>> iterate > >>>>>>>>>>>> over the results you could also call validTo(). Maybe add some > >>>>>>>>>>>> actual > >>>>>>>>>>>> records in the comments to show what the result might look > >>>>>>>>>>>> like. > >>>>>>>>>>>> > >>>>>>>>>>>> Regarding the test plan, I hope you also plan to add unit > >>>>>>>>>>>> tests in > >>>>>>> all > >>>>>>>>>>>> of your KIPs. Maybe you could also explain why system tests > >>>>>>>>>>>> are not > >>>>>>>>>>>> needed here. > >>>>>>>>>>>> > >>>>>>>>>>>> Best, > >>>>>>>>>>>> Bruno > >>>>>>>>>>>> > >>>>>>>>>>>> On 10/10/23 5:36 PM, Alieh Saeedi wrote: > >>>>>>>>>>>>> Thank you all for the very exact and constructive comments. I > >>>>>>>>>>>>> really > >>>>>>>>>>>>> enjoyed reading your ideas and all the important points you > >>>>>>>>>>>>> made me > >>>>>>>>>> aware > >>>>>>>>>>>>> of. I updated KIP-968 as follows: > >>>>>>>>>>>>> > >>>>>>>>>>>>> 1. If the key or time bounds are null, the method > >>>>>>>>>>>>> returns > >>>>>>>>>>>>> NPE. > >>>>>>>>>>>>> 2. The "valid" word: I removed the sentence "all the > >>>>>>>>>>>>> records > >>>>>>>>>> that are > >>>>>>>>>>>>> valid..." and replaced it with an exact explanation. > >>>>>>>>>>>>> More > >>>>>>> over, I > >>>>>>>>>>>>> explained > >>>>>>>>>>>>> it with an example in the KIP but not in the > >>>>>>>>>>>>> javadocs. Do I > >>>>>>> need > >>>>>>>>>>>>> to add the > >>>>>>>>>>>>> example to the javadocs as well? > >>>>>>>>>>>>> 3. Since I followed Bruno's suggestion and removed the > >>>>>>>>>> allVersions() > >>>>>>>>>>>>> method, the problem of meaningless combinations is > >>>>>>>>>>>>> solved, and > >>>>>>> I > >>>>>>>>>>>>> do not > >>>>>>>>>>>>> need any IllegalArgumentException or something like > >>>>>>>>>>>>> that. > >>>>>>>>>>>>> Therefore, the > >>>>>>>>>>>>> change is that if no time bound is specified, the > query > >>>>>>>>>>>>> returns > >>>>>>>>>>>>> the records > >>>>>>>>>>>>> with the specified key for all timestamps (all > >>>>>>>>>>>>> versions). > >>>>>>>>>>>>> 4. As Victoria suggested, adding a method to the > >>>>>>>>>>>>> *VersionedKeyValueStore > >>>>>>>>>>>>> *interface is essential. So I did that. I had this > >>>>>>>>>>>>> method > >>>>>>>>>>>>> only > >>>>>>>>>> in the > >>>>>>>>>>>>> RocksDBVersionedStore class, which was not enough. > >>>>>>>>>>>>> 5. I added the *validTo* field to the VersionedRecord > >>>>>>>>>>>>> class to > >>>>>>> be > >>>>>>>>>>>>> able > >>>>>>>>>>>>> to represent the tombstones. As you suggested, we > >>>>>>>>>>>>> postpone > >>>>>>>>>> solving > >>>>>>>>>>>>> the > >>>>>>>>>>>>> problem of retrieving consecutive tombstones for > later. > >>>>>>>>>>>>> 6. I added the "Test Plan" section to all KIPs. I hope > >>>>>>>>>>>>> what I > >>>>>>>>>>>>> wrote is > >>>>>>>>>>>>> convincing. > >>>>>>>>>>>>> 7. I added the *withAscendingTimestamp()* method to > >>>>>>>>>>>>> provide > >>>>>>> more > >>>>>>>>>>>>> code readability > >>>>>>>>>>>>> for the user. > >>>>>>>>>>>>> 8. I removed the evil word "get" from all getter > >>>>>>>>>>>>> methods. > >>>>>>>>>>>>> > >>>>>>>>>>>>> There have also been some more suggestions which I am still > >>>>>>>>>>>>> not > >>>>>>>>>> convinced > >>>>>>>>>>>>> or clear about them: > >>>>>>>>>>>>> > >>>>>>>>>>>>> 1. Regarding asOf vs until: reading all comments, my > >>>>>>>>>>>>> conclusion > >>>>>>>>>>>>> was that > >>>>>>>>>>>>> I keep it as "asOf" (following Walker's idea as the > >>>>>>>>>>>>> native > >>>>>>>>>> speaker > >>>>>>>>>>>>> as well > >>>>>>>>>>>>> as Bruno's suggestion to be consistent with > >>>>>>> single-key_single_ts > >>>>>>>>>>>>> queries). > >>>>>>>>>>>>> But I do not have a personal preference. If you > >>>>>>>>>>>>> insist on > >>>>>>>>>> "until", > >>>>>>>>>>>>> I change > >>>>>>>>>>>>> it. > >>>>>>>>>>>>> 2. Bruno suggested renaming the class > >>>>>>>>>>>>> "MultiVersionedKeyQuery" > >>>>>>>>>> to sth > >>>>>>>>>>>>> else. We already had a long discussion about the > >>>>>>>>>>>>> name with > >>>>>>>>>>>>> Matthias. I am > >>>>>>>>>>>>> open to renaming it to something else, but do you > >>>>>>>>>>>>> have any > >>>>>>> ideas? > >>>>>>>>>>>>> 3. Matthias suggested having a method with two input > >>>>>>>>>>>>> parameters > >>>>>>>>>> that > >>>>>>>>>>>>> enables the user to specify both time bounds in the > >>>>>>>>>>>>> same > >>>>>>> method. > >>>>>>>>>>>>> Isn't it > >>>>>>>>>>>>> introducing redundancy? It is somehow disrespectful to > >>>>>>>>>>>>> the idea > >>>>>>>>>> of > >>>>>>>>>>>>> having > >>>>>>>>>>>>> composable methods. > >>>>>>>>>>>>> 4. Bruno suggested renaming the methods "asOf" and > >>>>>>>>>>>>> "from" to > >>>>>>>>>>>>> "asOfTime" > >>>>>>>>>>>>> and "fromTime". If I do that, then it is not > >>>>>>>>>>>>> consistent with > >>>>>>>>>> KIP-960. > >>>>>>>>>>>>> Moreover, the input parameter is clearly a > >>>>>>>>>>>>> timestamp, which > >>>>>>>>>> explains > >>>>>>>>>>>>> enough. What do you think about that? > >>>>>>>>>>>>> 5. I was asked to add more examples to the example > >>>>>>>>>>>>> section. My > >>>>>>>>>>>>> question > >>>>>>>>>>>>> is, what is the main purpose of that? If I know it > >>>>>>>>>>>>> clearly, > >>>>>>> then > >>>>>>>>>> I > >>>>>>>>>>>>> can add > >>>>>>>>>>>>> what you mean. > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> Cheers, > >>>>>>>>>>>>> Alieh > >>>>>>>>>>>>> > >>>>>>>>>>>>> On Tue, Oct 10, 2023 at 1:13 AM Matthias J. Sax > >>>>>>>>>>>>> <mj...@apache.org> > >>>>>>>>>> wrote: > >>>>>>>>>>>>> > >>>>>>>>>>>>>> Bruno and I had some background conversation about the `get` > >>>>>>>>>>>>>> prefix > >>>>>>>>>>>>>> question including a few other committers. > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> The official policy was never changed, and we should not > >>>>>>>>>>>>>> add the > >>>>>>>>>>>>>> `get`-prefix. It's a slip on our side in previous KIPs to > >>>>>>>>>>>>>> add the > >>>>>>>>>>>>>> `get`-prefix and we should actually clean it up doing a > >>>>>>>>>>>>>> follow up > >>>>>>>>>> KIP. > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> -Matthias > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> On 10/5/23 5:26 AM, Bruno Cadonna wrote: > >>>>>>>>>>>>>>> Hi Matthias, > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Given all the IQv2 KIPs that use getX and given recent PRs > >>>>>>>>>> (internal > >>>>>>>>>>>>>>> interfaces mainly) that got merged, I was under the > >>>>>>>>>>>>>>> impression > >>>>>>>>>> that we > >>>>>>>>>>>>>>> moved away from the strict no-getX policy. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> I do not think it was an accident using getX in the IQv2 > >>>>>>>>>>>>>>> KIPs > >>>>>>> since > >>>>>>>>>>>>>>> somebody would have brought it up, otherwise. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> I am fine with both types of getters. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> If we think, we need to discuss this in a broader > >>>>>>>>>>>>>>> context, let's > >>>>>>>>>>>>>>> start a > >>>>>>>>>>>>>>> separate thread. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Best, > >>>>>>>>>>>>>>> Bruno > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> On 10/5/23 7:44 AM, Matthias J. Sax wrote: > >>>>>>>>>>>>>>>> I agree to (almost) everything what Bruno said. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> In general, we tend to move away from using getters > >>>>>>>>>>>>>>>>> without > >>>>>>>>>> "get", > >>>>>>>>>>>>>>>>> recently. So I would keep the "get". > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> This is new to me? Can you elaborate on this point? Why > >>>>>>>>>>>>>>>> do you > >>>>>>>>>> think > >>>>>>>>>>>>>>>> that's the case? > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> I actually did realize (after Walker mentioned it) that > >>>>>>>>>>>>>>>> existing > >>>>>>>>>> query > >>>>>>>>>>>>>>>> types use `get` prefix, but to me it seems that it was by > >>>>>>>>>> accident and > >>>>>>>>>>>>>>>> we should consider correcting it? Thus, I would actually > >>>>>>>>>>>>>>>> prefer > >>>>>>>>>> to not > >>>>>>>>>>>>>>>> add the `get` prefix for new methods query types. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> IMHO, we should do a follow up KIP to deprecate all > methods > >>>>>>>>>>>>>>>> with > >>>>>>>>>> `get` > >>>>>>>>>>>>>>>> prefix and replace them with new ones without `get` -- > >>>>>>>>>>>>>>>> it's of > >>>>>>>>>> course > >>>>>>>>>>>>>>>> always kinda "unnecessary" noise, but if we don't do it, > we > >>>>>>>>>>>>>>>> might > >>>>>>>>>> get > >>>>>>>>>>>>>>>> into more and more inconsistent naming what would result > >>>>>>>>>>>>>>>> in a > >>>>>>>>>> "bad" > >>>>>>>>>>>>>>>> API. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> If we indeed want to change the convention and use the > >>>>>>>>>>>>>>>> `get` > >>>>>>>>>> prefix, I > >>>>>>>>>>>>>>>> would strongly advocate to bit the bullet and do KIP to > >>>>>>>>>> pro-actively > >>>>>>>>>>>>>>>> add the `get` "everywhere" it's missing... But overall, it > >>>>>>>>>>>>>>>> seems > >>>>>>>>>> to be > >>>>>>>>>>>>>>>> a much broader decision, and we should get buy in from > many > >>>>>>>>>> committers > >>>>>>>>>>>>>>>> about it -- as long as there is no broad consensus to > >>>>>>>>>>>>>>>> add `get` > >>>>>>>>>>>>>>>> everywhere, I would strongly prefer not to diverge from > the > >>>>>>>>>> current > >>>>>>>>>>>>>>>> agreement to omit `get`. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> -Matthias > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> On 10/4/23 2:36 AM, Bruno Cadonna wrote: > >>>>>>>>>>>>>>>>> Hi, > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Regarding tombstones: > >>>>>>>>>>>>>>>>> As far as I understand, we need to add either a validTo > >>>>>>>>>>>>>>>>> field to > >>>>>>>>>>>>>>>>> VersionedRecord or we need to return tombstones, > >>>>>>>>>>>>>>>>> otherwise the > >>>>>>>>>> result > >>>>>>>>>>>>>>>>> is not complete, because users could never know a > >>>>>>>>>>>>>>>>> record was > >>>>>>>>>> deleted > >>>>>>>>>>>>>>>>> at some point before the second non-null value was put. > >>>>>>>>>>>>>>>>> I like more adding the validTo field since it makes the > >>>>>>>>>>>>>>>>> result > >>>>>>>>>> more > >>>>>>>>>>>>>>>>> concise and easier interpretable. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Extending on Victoria's example, with the following puts > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> put(k, v1, time=0) > >>>>>>>>>>>>>>>>> put(k, null, time=5) > >>>>>>>>>>>>>>>>> put(k, null, time=10) > >>>>>>>>>>>>>>>>> put(k, null, time=15) > >>>>>>>>>>>>>>>>> put(k, v2, time=20) > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> the result with tombstones would be > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> value, timestamp > >>>>>>>>>>>>>>>>> (v1, 0) > >>>>>>>>>>>>>>>>> (null, 5) > >>>>>>>>>>>>>>>>> (null, 10) > >>>>>>>>>>>>>>>>> (null, 15) > >>>>>>>>>>>>>>>>> (v2, 20) > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> instead of > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> value, timestamp, validTo > >>>>>>>>>>>>>>>>> (v1, 0, 5) > >>>>>>>>>>>>>>>>> (v2, 20, null) > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> The benefit of conciseness would already apply to one > >>>>>>>>>>>>>>>>> single > >>>>>>>>>>>>>>>>> tombstone. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> On the other hand, why would somebody write consecutive > >>>>>>>>>> tombstones > >>>>>>>>>>>>>>>>> into a versioned state store? I guess if somebody does > >>>>>>>>>>>>>>>>> that on > >>>>>>>>>>>>>>>>> purpose, then there should be a way to retrieve each of > >>>>>>>>>>>>>>>>> those > >>>>>>>>>>>>>>>>> tombstones, right? > >>>>>>>>>>>>>>>>> So maybe we need both -- validTo field and the option to > >>>>>>>>>>>>>>>>> return > >>>>>>>>>>>>>>>>> tombstones. The latter might be moved to a future KIP in > >>>>>>>>>>>>>>>>> case we > >>>>>>>>>> see > >>>>>>>>>>>>>>>>> the need. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Regarding .within(fromTs, toTs): > >>>>>>>>>>>>>>>>> I would keep it simple with .from() and > >>>>>>>>>>>>>>>>> .asOfTimestamp() (or > >>>>>>>>>>>>>>>>> .until()). If we go with .within(), I would opt for > >>>>>>>>>>>>>>>>> .withinTimeRange(fromTs, toTs), because the query > >>>>>>>>>>>>>>>>> becomes more > >>>>>>>>>>>>>> readable: > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> MultiVersionedKeyQuery > >>>>>>>>>>>>>>>>> .withKey(1) > >>>>>>>>>>>>>>>>> .withinTimeRange(Instant.parse(2023-08-03T10:37:30.00Z), > >>>>>>>>>>>>>>>>> Instant.parse(2023-08-04T10:37:30.00Z)) > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> If we stay with .from() and .until(), we should consider > >>>>>>>>>> .fromTime() > >>>>>>>>>>>>>>>>> and .untilTime() (or .toTime()): > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> MultiVersionedKeyQuery > >>>>>>>>>>>>>>>>> .withKey(1) > >>>>>>>>>>>>>>>>> .fromTime(Instant.parse(2023-08-03T10:37:30.00Z)) > >>>>>>>>>>>>>>>>> .untilTime(Instant.parse(2023-08-04T10:37:30.00Z)) > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Regarding asOf vs. until: > >>>>>>>>>>>>>>>>> I think asOf() is more used in point in time queries as > >>>>>>>>>>>>>>>>> Walker > >>>>>>>>>>>>>>>>> mentioned where this KIP specifies a time range. IMO > >>>>>>>>>>>>>>>>> asOf() > >>>>>>>>>>>>>>>>> fits > >>>>>>>>>> very > >>>>>>>>>>>>>>>>> well with KIP-960 where one version is queried, but here > I > >>>>>>>>>>>>>>>>> think > >>>>>>>>>>>>>>>>> .until() fits better. That might just be a matter of > >>>>>>>>>>>>>>>>> taste and > >>>>>>>>>> in the > >>>>>>>>>>>>>>>>> end I am fine with both as long as it is well documented. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Regarding getters without "get": > >>>>>>>>>>>>>>>>> In the other IQv2 classes we used getters with "get". In > >>>>>>>>>> general, we > >>>>>>>>>>>>>>>>> tend to move away from using getters without "get", > >>>>>>>>>>>>>>>>> recently. So > >>>>>>>>>> I > >>>>>>>>>>>>>>>>> would keep the "get". > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Best, > >>>>>>>>>>>>>>>>> Bruno > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> On 10/3/23 7:49 PM, Walker Carlson wrote: > >>>>>>>>>>>>>>>>>> Hey Alieh thanks for the KIP, > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> Weighing in on the AsOf vs Until debate I think either > is > >>>>>>>>>>>>>>>>>> fine > >>>>>>>>>>>>>>>>>> from a > >>>>>>>>>>>>>>>>>> natural language perspective. Personally AsOf makes more > >>>>>>>>>>>>>>>>>> sense > >>>>>>>>>> to me > >>>>>>>>>>>>>>>>>> where > >>>>>>>>>>>>>>>>>> until gives me the idea that the query is making a > >>>>>>>>>>>>>>>>>> change. > >>>>>>>>>>>>>>>>>> It's > >>>>>>>>>>>>>>>>>> totally a > >>>>>>>>>>>>>>>>>> connotative difference and not that important. I think > as > >>>>>>>>>>>>>>>>>> of is > >>>>>>>>>>>>>>>>>> pretty > >>>>>>>>>>>>>>>>>> frequently used in point of time queries. > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> Also for these methods it makes sense to drop the > >>>>>>>>>>>>>>>>>> "get" We > >>>>>>> don't > >>>>>>>>>>>>>>>>>> normally use that in getters > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> * The key that was specified for this query. > >>>>>>>>>>>>>>>>>> */ > >>>>>>>>>>>>>>>>>> public K getKey(); > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> /** > >>>>>>>>>>>>>>>>>> * The starting time point of the query, if > >>>>>>>>>>>>>>>>>> specified > >>>>>>>>>>>>>>>>>> */ > >>>>>>>>>>>>>>>>>> public Optional<Instant> getFromTimestamp(); > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> /** > >>>>>>>>>>>>>>>>>> * The ending time point of the query, if > >>>>>>>>>>>>>>>>>> specified > >>>>>>>>>>>>>>>>>> */ > >>>>>>>>>>>>>>>>>> public Optional<Instant> getAsOfTimestamp(); > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> Other than that I didn't have too much to add. Overall > >>>>>>>>>>>>>>>>>> I like > >>>>>>>>>> the > >>>>>>>>>>>>>>>>>> direction > >>>>>>>>>>>>>>>>>> of the KIP and think the funcatinlyt is all there! > >>>>>>>>>>>>>>>>>> best, > >>>>>>>>>>>>>>>>>> Walker > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> On Mon, Oct 2, 2023 at 10:46 PM Matthias J. Sax < > >>>>>>>>>> mj...@apache.org> > >>>>>>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> Thanks for the updated KIP. Overall I like it. > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> Victoria raises a very good point, and I personally > >>>>>>>>>>>>>>>>>>> tend to > >>>>>>>>>>>>>>>>>>> prefer (I > >>>>>>>>>>>>>>>>>>> believe so does Victoria, but it's not totally clear > >>>>>>>>>>>>>>>>>>> from > >>>>>>>>>>>>>>>>>>> her > >>>>>>>>>>>>>>>>>>> email) if > >>>>>>>>>>>>>>>>>>> a range query would not return any tombstones, ie, > >>>>>>>>>>>>>>>>>>> only two > >>>>>>>>>> records > >>>>>>>>>>>>>> in > >>>>>>>>>>>>>>>>>>> Victoria's example. Thus, it seems best to include a > >>>>>>>>>>>>>>>>>>> `validTo` > >>>>>>>>>>>>>>>>>>> ts-field > >>>>>>>>>>>>>>>>>>> to `VersionedRecord` -- otherwise, the retrieved result > >>>>>>>>>>>>>>>>>>> cannot > >>>>>>>>>> be > >>>>>>>>>>>>>>>>>>> interpreted correctly. > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> Not sure what others think about it. > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> I would also be open to actually add a > >>>>>>>>>>>>>>>>>>> `includeDeletes()` > >>>>>>>>>>>>>>>>>>> (or > >>>>>>>>>>>>>>>>>>> `includeTombstones()`) method/flag (disabled by > >>>>>>>>>>>>>>>>>>> default) to > >>>>>>>>>> allow > >>>>>>>>>>>>>>>>>>> users > >>>>>>>>>>>>>>>>>>> to get all tombstone: this would only be helpful if > >>>>>>>>>>>>>>>>>>> there > >>>>>>>>>>>>>>>>>>> are > >>>>>>>>>> two > >>>>>>>>>>>>>>>>>>> consecutive tombstone though (if I got it right), so > not > >>>>>>>>>>>>>>>>>>> sure > >>>>>>>>>> if we > >>>>>>>>>>>>>>>>>>> want > >>>>>>>>>>>>>>>>>>> to add it or not -- it seems also possible to add it > >>>>>>>>>>>>>>>>>>> later if > >>>>>>>>>> there > >>>>>>>>>>>>>> is > >>>>>>>>>>>>>>>>>>> user demand for it, so it might be a premature > >>>>>>>>>>>>>>>>>>> addition as > >>>>>>> this > >>>>>>>>>>>>>> point? > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> Nit: > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> the public interface ValueIterator is used > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> "is used" -> "is added" (otherwise it sounds like as if > >>>>>>>>>>>>>>>>>>> `ValueIterator` > >>>>>>>>>>>>>>>>>>> exist already) > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> Should we also add a `.within(fromTs, toTs)` (or > >>>>>>>>>>>>>>>>>>> maybe some > >>>>>>>>>> better > >>>>>>>>>>>>>>>>>>> name?) to allow specifying both bounds at once? The > >>>>>>>>>>>>>>>>>>> existing > >>>>>>>>>>>>>>>>>>> `RangeQuery` does the same for specifying the > >>>>>>>>>>>>>>>>>>> key-range, so > >>>>>>>>>>>>>>>>>>> might be > >>>>>>>>>>>>>>>>>>> good to add for time-range too? > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> -Matthias > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> On 9/6/23 5:01 AM, Bruno Cadonna wrote: > >>>>>>>>>>>>>>>>>>>> In my last e-mail I missed to finish a sentence. > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> "I think from a KIP" > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> should be > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> "I think the KIP looks good!" > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> On 9/6/23 1:59 PM, Bruno Cadonna wrote: > >>>>>>>>>>>>>>>>>>>>> Hi Alieh, > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> Thanks for the KIP! > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> I think from a KIP > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> 1. > >>>>>>>>>>>>>>>>>>>>> I propose to throw an IllegalArgumentException or an > >>>>>>>>>>>>>>>>>>>>> IllegalStateException for meaningless combinations. > >>>>>>>>>>>>>>>>>>>>> In any > >>>>>>>>>> case, > >>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>>>>> KIP should specify what exception is thrown. > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> 2. > >>>>>>>>>>>>>>>>>>>>> Why does not specifying a range return the latest > >>>>>>>>>>>>>>>>>>>>> version? I > >>>>>>>>>>>>>>>>>>>>> would > >>>>>>>>>>>>>>>>>>>>> expect that it returns all versions since an empty > >>>>>>>>>>>>>>>>>>>>> lower or > >>>>>>>>>> upper > >>>>>>>>>>>>>>>>>>>>> limit is interpreted as no limit. > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> 3. > >>>>>>>>>>>>>>>>>>>>> I second Matthias comment about replacing "asOf" with > >>>>>>>>>> "until" or > >>>>>>>>>>>>>>>>>>>>> "to". > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> 4. > >>>>>>>>>>>>>>>>>>>>> Do we need "allVersions()"? As I said above I would > >>>>>>>>>>>>>>>>>>>>> return > >>>>>>>>>> all > >>>>>>>>>>>>>>>>>>>>> versions if no limits are specified. I think if we > >>>>>>>>>>>>>>>>>>>>> get rid > >>>>>>> of > >>>>>>>>>>>>>>>>>>>>> allVersions() there might not be any meaningless > >>>>>>> combinations > >>>>>>>>>>>>>>>>>>>>> anymore. > >>>>>>>>>>>>>>>>>>>>> If a user applies twice the same limit like for > >>>>>>>>>>>>>>>>>>>>> example > >>>>>>>>>>>>>>>>>>>>> MultiVersionedKeyQuery.with(key).from(t1).from(t2) > the > >>>>>>>>>>>>>>>>>>>>> last > >>>>>>>>>> one > >>>>>>>>>>>>>>>>>>>>> wins. > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> 5. > >>>>>>>>>>>>>>>>>>>>> Could you add some more examples with time ranges > >>>>>>>>>>>>>>>>>>>>> to the > >>>>>>>>>> example > >>>>>>>>>>>>>>>>>>> section? > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> 6. > >>>>>>>>>>>>>>>>>>>>> The KIP misses the test plan section. > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> 7. > >>>>>>>>>>>>>>>>>>>>> I propose to rename the class to > >>>>>>>>>>>>>>>>>>>>> "MultiVersionKeyQuery" > >>>>>>>>>> since we > >>>>>>>>>>>>>> are > >>>>>>>>>>>>>>>>>>>>> querying multiple versions of the same key. > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> 8. > >>>>>>>>>>>>>>>>>>>>> Could you also add withAscendingTimestamps()? IMO > >>>>>>>>>>>>>>>>>>>>> it gives > >>>>>>>>>> users > >>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>>>>> possibility to make their code more readable > >>>>>>>>>>>>>>>>>>>>> instead of > >>>>>>>>>>>>>>>>>>>>> only > >>>>>>>>>>>>>> relying > >>>>>>>>>>>>>>>>>>>>> on the default. > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> Best, > >>>>>>>>>>>>>>>>>>>>> Bruno > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> On 8/17/23 4:13 AM, Matthias J. Sax wrote: > >>>>>>>>>>>>>>>>>>>>>> Thanks for splitting this part into a separate KIP! > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> For `withKey()` we should be explicit that `null` > >>>>>>>>>>>>>>>>>>>>>> is not > >>>>>>>>>>>>>>>>>>>>>> allowed. > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> (Looking into existing `KeyQuery` it seems the > >>>>>>>>>>>>>>>>>>>>>> JavaDocs > >>>>>>>>>> don't > >>>>>>>>>>>>>> cover > >>>>>>>>>>>>>>>>>>>>>> this either -- would you like to do a tiny cleanup > >>>>>>>>>>>>>>>>>>>>>> PR for > >>>>>>>>>>>>>>>>>>>>>> this, or > >>>>>>>>>>>>>>>>>>>>>> fix on-the-side in one of your PRs?) > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> The key query returns all the records that are > >>>>>>>>>>>>>>>>>>>>>>> valid in > >>>>>>> the > >>>>>>>>>>>>>>>>>>>>>>> time > >>>>>>>>>>>>>>>>>>>>>>> range starting from the timestamp {@code > >>>>>>>>>>>>>>>>>>>>>>> fromTimestamp}. > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> In the JavaDocs you use the phrase `are valid` -- > >>>>>>>>>>>>>>>>>>>>>> I think > >>>>>>> we > >>>>>>>>>>>>>>>>>>>>>> need to > >>>>>>>>>>>>>>>>>>>>>> explain what "valid" means? It might even be worth > >>>>>>>>>>>>>>>>>>>>>> to add > >>>>>>>>>> some > >>>>>>>>>>>>>>>>>>>>>> examples. It's annoying, but being precise if kinda > >>>>>>>>>> important. > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> With regard to KIP-962, should we allow `null` for > >>>>>>>>>>>>>>>>>>>>>> time > >>>>>>>>>> bounds ? > >>>>>>>>>>>>>>>>>>>>>> The > >>>>>>>>>>>>>>>>>>>>>> JavaDocs should also be explicit if `null` is > >>>>>>>>>>>>>>>>>>>>>> allowed or > >>>>>>>>>> not and > >>>>>>>>>>>>>>>>>>>>>> what > >>>>>>>>>>>>>>>>>>>>>> the semantics are if allowed. > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> You are using `asOf()` however, because we are doing > >>>>>>>>>> time-range > >>>>>>>>>>>>>>>>>>>>>> queries, to me using `until()` to describe the upper > >>>>>>>>>>>>>>>>>>>>>> bound > >>>>>>>>>> would > >>>>>>>>>>>>>>>>>>>>>> sound better (I am not a native speaker though, so > >>>>>>>>>>>>>>>>>>>>>> maybe I > >>>>>>>>>> am > >>>>>>>>>>>>>> off?) > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> The key query returns all the records that have > >>>>>>>>>>>>>>>>>>>>>>> timestamp > >>>>>>>>>> <= > >>>>>>>>>>>>>>>>>>>>>>> {@code > >>>>>>>>>>>>>>>>>>>>>>> asOfTimestamp}. > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> This is only correct if not lower-bound is set, > >>>>>>>>>>>>>>>>>>>>>> right? > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> In your reply to KIP-960 you mentioned: > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> the meaningless combinations are prevented by > >>>>>>>>>>>>>>>>>>>>>>> throwing > >>>>>>>>>>>>>> exceptions. > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> We should add corresponding JavaDocs like: > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> @throws IllegalArgumentException if {@code > >>>>>>>>>>>>>>>>>>>>>> fromTimestamp} is > >>>>>>>>>>>>>>>>>>>>>> equal or > >>>>>>>>>>>>>>>>>>>>>> larger > than > >>>>>>>>>>>>>>>>>>>>>> {@code > >>>>>>>>>>>>>>>>>>>>>> untilTimestamp} > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> Or something similar. > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> With regard to KIP-960: if we need to introduce a > >>>>>>>>>>>>>>>>>>>>>> `VersionedKeyQuery` > >>>>>>>>>>>>>>>>>>>>>> class for single-key-single-ts lookup, would we > >>>>>>>>>>>>>>>>>>>>>> need to > >>>>>>> find > >>>>>>>>>>>>>>>>>>>>>> a new > >>>>>>>>>>>>>>>>>>>>>> name for the query class of this KIP, given that the > >>>>>>>>>>>>>>>>>>>>>> return > >>>>>>>>>> type > >>>>>>>>>>>>>> is > >>>>>>>>>>>>>>>>>>>>>> different? > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> -Matthias > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> On 8/16/23 10:57 AM, Alieh Saeedi wrote: > >>>>>>>>>>>>>>>>>>>>>>> Hi all, > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> I splitted KIP-960 > >>>>>>>>>>>>>>>>>>>>>>> < > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>> > >>>>>>> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-960%3A+Support+single-key_single-timestamp+interactive+queries+%28IQv2%29+for+versioned+state+stores > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> into three separate KIPs. Therefore, please > continue > >>>>>>>>>>>>>>>>>>>>>>> discussions > >>>>>>>>>>>>>>>>>>>>>>> about single-key, multi-timestamp interactive > >>>>>>>>>>>>>>>>>>>>>>> queries > >>>>>>>>>> here. You > >>>>>>>>>>>>>>>>>>>>>>> can > >>>>>>>>>>>>>>>>>>>>>>> see all > >>>>>>>>>>>>>>>>>>>>>>> the addressed reviews on the following page. > >>>>>>>>>>>>>>>>>>>>>>> Thanks in > >>>>>>>>>> advance. > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> KIP-968: Support single-key_multi-timestamp > >>>>>>>>>>>>>>>>>>>>>>> interactive > >>>>>>>>>> queries > >>>>>>>>>>>>>>>>>>>>>>> (IQv2) for > >>>>>>>>>>>>>>>>>>>>>>> versioned state stores > >>>>>>>>>>>>>>>>>>>>>>> < > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>> > >>>>>>> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-968%3A+Support+single-key_multi-timestamp+interactive+queries+%28IQv2%29+for+versioned+state+stores > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> I look forward to your feedback! > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> Cheers, > >>>>>>>>>>>>>>>>>>>>>>> Alieh > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>> > >>>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>> > >>> >
