Hey, no problem, Sagar!
Yes, I think this is a good time for the voting thread, since all the concerns
have been addressed in the discussion, and it’s been a while.
You can find the rules on the main KIP page, and examples of vote messages in
the mailing list.
Thanks,
John
On Sat, Jul 11,
Thanks John,
Sorry I’m new to this process. does it mean I start a voting email?
Pardon my ignorance.
Sagar.
On Sat, 11 Jul 2020 at 8:06 PM, John Roesler wrote:
> Hi Sagar,
>
> Thanks for the update. As far as I’m concerned, I’m ready to vote now.
>
> Thanks,
> John
>
> On Mon, Jul 6,
Hi Sagar,
Thanks for the update. As far as I’m concerned, I’m ready to vote now.
Thanks,
John
On Mon, Jul 6, 2020, at 12:58, Sagar wrote:
> Hi John,
>
> Thanks, I have updated the KIP.
>
> Thanks!
> Sagar.
>
> On Mon, Jul 6, 2020 at 12:00 AM John Roesler wrote:
>
> > Hi Sagar,
> >
> >
Hi John,
Thanks, I have updated the KIP.
Thanks!
Sagar.
On Mon, Jul 6, 2020 at 12:00 AM John Roesler wrote:
> Hi Sagar,
>
> Sorry for the ambiguity. You could just mention it in the Public
> Interfaces section. Or, if you want to be more specific, you can show it in
> the method definition
Hi Sagar,
Sorry for the ambiguity. You could just mention it in the Public Interfaces
section. Or, if you want to be more specific, you can show it in the method
definition snippet. I don’t think it matters, as long as it’s clearly stated,
since it affects backward compatibility with existing
Hi John,
Thank you! Question on the comment, where should I add the default
implementation? I guess that needs to be added in the Proposal Section of
the kIP.
Thanks!
Sagar.
On Sat, Jul 4, 2020 at 11:46 PM John Roesler wrote:
> Thanks Sagar,
>
> That looks good to me! The only minor comment
Thanks Sagar,
That looks good to me! The only minor comment I’d make is that I think the
method declaration should have a default implementation that throws an
UnsupportedOperationException, for source compatibility with existing state
stores.
Otherwise, as far as I’m concerned, I’m ready to
Hi John,
I have updated the KIP with all the new changes we discussed in this
discussion thread.
https://cwiki.apache.org/confluence/display/KAFKA/KIP-614%3A+Add+Prefix+Scan+support+for+State+Stores
Request you to go through the same.
Thanks!
Sagar.
On Tue, Jun 30, 2020 at 8:09 AM John
Hi Sagar,
That’s a good observation; yes, it should go in the ReadOnlyKeyValueStore
interface.
Thanks again for the great work,
John
On Sun, Jun 28, 2020, at 23:54, Sagar wrote:
> Hi John,
>
> Thank you for the positive feedback! The meaningful discussions we had on
> the mailing list helped
Hi John,
Thank you for the positive feedback! The meaningful discussions we had on
the mailing list helped me understand what needed to be done.
I am definitely open to any further suggestions on this.
Before I updated the KIP, I just had one question, is it fine to have it
for KeyValueStore or
Woah, this is great, Sagar!
I think this API looks really good. I'm curious if anyone else has
any concern. For my part, I think this will work just fine. People
might face tricky bugs getting their key serde and their prefix
serde "aligned", but I think the API makes it pretty obvious what
has
Hi John,
I took some time out and as we discussed, looked to implement these
changes. Most of these changes are for demonstrative purposes but I thought
I will share.
I added the new prefixSeek method at the KeyValueStore interface level:
Hi John,
You rightly pointed out, the devil is in the detail :). I will start with
the implementation to get a sense.
Here are my thoughts on the core challenge that you pointed out. The key
value store abstractions that have been exposed via the state store DSL
APIs, make it possible for the
Hi Sagar,
Thanks for the reply. I agree that your UUID example illustrates the
problem I was pointing out.
Yes, I think that experimenting with the API in the PR is probably the
best way to make progress (as opposed to just thinking in terms of
design on the wiki) because with this kind of
Hi John,
Thanks for the response. For starters, for our use case, we tweaked our
keys etc to avoid prefix scans. So, we are good there.
Regarding the KIP, I see what you mean when you say that the same key type
for prefix won't work. For example, continuing with the UUID example that
you gave,
Hi Sagar,
Thanks for the question, and sorry for muddying the water.
I meant the Bytes/byte[] thing as advice for how you all can solve your problem
in the mean time, while we work through this KIP. I don’t think it’s relevant
for the KIP itself.
I think the big issue here is what the type of
Hi John,
Just to add to my previous email(and sorry for the spam), if we consider
using Bytes/byte[] and manually invoke the serdes, if you could provide
examples where the same Serde for keys won't work for the prefix types. As
far as my understanding goes, the prefix seek would depend upon
Hi John,
Thank you. I think it makes sense to modify the KIP to add the prefixScan()
as part of the existing interfaces and add the new mixin behaviour as
Rejected alternatives. I am not very aware of other stores apart from
keyValueStore so is it fine if I keep it there for now?
Regarding the
Hi Sagar,
Thanks for the response. Your use case makes sense to me; I figured it must be
something like that.
On a pragmatic level, in the near term, you might consider basically doing the
same thing we did in KIP-213. If you swap out the store types for Byte/byte[]
and “manually” invoke the
Hi John,
Thanks for the detailed reply. I was a bit crammed with work last week so
couldn't respond earlier so apologies for that.
First of all, thanks for the context that both you and Adam have
provided me on the issues faced previously. As I can clearly see, while I
was able to cut some
Hi, Sagar!
Thanks for this KIP. I'm sorry it took me so long to reply. I'll number my
points differently to avoid confusion.
I can provide some additional context on the difficulties we previously
faced in KIP-213 (which you and Adam have already discussed).
J1) In your KIP, you propose the
Whoops, I guess I didn't finish reading the KIP all the way to the end
earlier. Thanks
for including the link to the RocksDB PR in the KIP!
I have one additional question about the proposal: do you plan to also add
this
prefix seek API to the dual column family iterators? These are used by
Hey @Adam,
Thanks for sharing your experience with using prefix seek. I did look at
your code for RocksDBPrefixIterator, infact I have repurposed that class
itself since it wasn't being used else where. Regarding how I plan to
expose them through-out the state stores, what I have tried to do is
Not to derail this KIP discussion, but to leave a few notes on some of the
RocksDB points that have come up:
Someone actually merged some long overdue performance improvements to
the RocksJava implementation (the PR was opened back in 2017! yikes).
I haven't looked into the prefix seek API
Thanks Adam, Sagar.
I read your PR as well the rocksDB reference, and I have a few quick
questions:
1. In your code I saw you did not specifically overwrite any rocksDB
configs like `useFixedLengthPrefixExtractor`. Also, by comparing the
`RocksDBPrefixIterator` and `RocksDBRangeIterator`
Hi Guozhang
For clarity, the issues I was running into was not about the actual
*prefixSeek* function itself, but about exposing it to the same level of
access as the *range* function throughout Kafka Streams. It required a lot
of changes, and also required that most state stores stub it out
Hello Adam,
I'm wondering if you can provide a bit more context on the blockers of
using prefixSeek of RocksDB (I saw you have a RocksDBPrefixIterator class
but not used anywhere yet)? I'm currently looking at ways to allow some
secondary indices with rocksDB following some existing approaches
Hi Sagar
I implemented a very similar interface for KIP-213, the foreign-key joiner.
We pulled it out of the final implementation and instead used RocksDB range
instead. You can see the particular code where we use RocksDB.range(...) to
get the same iterator result.
Hi All,
I would like to start a discussion on the KIP that I created below to add
prefix scan support in State Stores:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-614%3A+Add+Prefix+Scan+support+for+State+Stores
Thanks!
Sagar.
29 matches
Mail list logo
