+1 On Wed, May 10, 2017 at 1:45 PM, Xavier Léauté <xav...@confluent.io> wrote:
> Thank you for the feedback Michal. > > While I agree the return may be a little bit more confusing to reason > about, the reason for doing so was to keep the range query interfaces > consistent with their single-key counterparts. > > In the case of the window store, the "key" of the single-key iterator is > the actual timestamp of the underlying entry, not just range of the window, > so if we were to wrap the result key a window we wouldn't be getting back > the equivalent of the single key iterator. > > In both cases peekNextKey is just returning the timestamp of the next entry > in the window store that matches the query. > > In the case of the session store, we already return Windowed<K> for the > single-key method, so it made sense there to also return Windowed<K> for > the range method. > > Hope this make sense? Let me know if you still have concerns about this. > > Thank you, > Xavier > > On Wed, May 10, 2017 at 12:25 PM Michal Borowiecki < > michal.borowie...@openbet.com> wrote: > > > Apologies, I missed the discussion (or lack thereof) about the return > > type of: > > > > WindowStoreIterator<KeyValue<K, V>> fetch(K from, K to, long timeFrom, > > long timeTo) > > > > > > WindowStoreIterator<V> (as the KIP mentions) is a subclass of > > KeyValueIterator<Long, V> > > > > KeyValueIterator<K,V> has the following method: > > > > /** * Peek at the next key without advancing the iterator * @return the > > key of the next value that would be returned from the next call to next > > */ K peekNextKey(); > > > > Given the type in this case will be Long, I assume what it would return > > is the window timestamp of the next found record? > > > > > > In the case of WindowStoreIterator<V> fetch(K key, long timeFrom, long > > timeTo); > > all records found by fetch have the same key, so it's harmless to return > > the timestamp of the next found window but here we have varying keys and > > varying windows, so won't it be too confusing? > > > > KeyValueIterator<Windowed<K>, V> (as in the proposed > > ReadOnlySessionStore.fetch) just feels much more intuitive. > > > > Apologies again for jumping onto this only once the voting has already > > begun. > > Thanks, > > Michał > > > > On 10/05/17 20:08, Sriram Subramanian wrote: > > > +1 > > > > > > On Wed, May 10, 2017 at 11:42 AM, Bill Bejeck <bbej...@gmail.com> > wrote: > > > > > >> +1 > > >> > > >> Thanks, > > >> Bill > > >> > > >> On Wed, May 10, 2017 at 2:38 PM, Guozhang Wang <wangg...@gmail.com> > > wrote: > > >> > > >>> +1. Thank you! > > >>> > > >>> On Wed, May 10, 2017 at 11:30 AM, Xavier Léauté <xav...@confluent.io > > > > >>> wrote: > > >>> > > >>>> Hi everyone, > > >>>> > > >>>> Since there aren't any objections to this addition, I would like to > > >> start > > >>>> the voting on KIP-155 so we can hopefully get this into 0.11. > > >>>> > > >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+ > > >>>> 155+-+Add+range+scan+for+windowed+state+stores > > >>>> > > >>>> Voting will stay active for at least 72 hours. > > >>>> > > >>>> Thank you, > > >>>> Xavier > > >>>> > > >>> > > >>> > > >>> -- > > >>> -- Guozhang > > >>> > > > > >
