Re: [VOTE] KIP-79 - ListOffsetRequest v1 and search by timestamp methods in new consumer.

[Becket Qin]

Thanks everyone for the comments and votes.

KIP-79 has passed with +4 binding (Jason, Neha, Jun, Ismael) and +1
non-binding (Bill).

On Mon, Sep 19, 2016 at 1:54 PM, Ismael Juma  wrote:

> Thanks for the KIP, +1 (binding) to the latest version.
>
> Ismael
>
> On Sat, Sep 10, 2016 at 12:38 AM, Becket Qin  wrote:
>
> > Hi all,
> >
> > I'd like to start the voting for KIP-79
> >
> > In short we propose to :
> > 1. add a ListOffsetRequest/ListOffsetResponse v1, and
> > 2. add earliestOffsts(), latestOffsets() and offsetForTime() methods in
> the
> > new consumer.
> >
> > The KIP wiki is the following:
> > https://cwiki.apache.org/confluence/pages/viewpage.
> action?pageId=65868090
> >
> > Thanks,
> >
> > Jiangjie (Becket) Qin
> >
>

Re: [VOTE] KIP-79 - ListOffsetRequest v1 and search by timestamp methods in new consumer.

[Ismael Juma]

Thanks for the KIP, +1 (binding) to the latest version.

Ismael

On Sat, Sep 10, 2016 at 12:38 AM, Becket Qin  wrote:

> Hi all,
>
> I'd like to start the voting for KIP-79
>
> In short we propose to :
> 1. add a ListOffsetRequest/ListOffsetResponse v1, and
> 2. add earliestOffsts(), latestOffsets() and offsetForTime() methods in the
> new consumer.
>
> The KIP wiki is the following:
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=65868090
>
> Thanks,
>
> Jiangjie (Becket) Qin
>

Re: [VOTE] KIP-79 - ListOffsetRequest v1 and search by timestamp methods in new consumer.

[Jun Rao]

Hi, Jiangjie,

For 1, yes, since this api is about timestamp, returning any other metadata
more than the timestamp may not make sense any way. So, we can leave the
api as it is.

Thanks,

Jun

On Mon, Sep 19, 2016 at 12:57 PM, Becket Qin  wrote:

> Thanks for the comment Jun. Please see the reply in line
>
> On Mon, Sep 19, 2016 at 10:21 AM, Jason Gustafson 
> wrote:
>
> > +1 on Jun's suggestion to use "beginning" and "end". The term "latest" is
> > misleading since the last message in the log may not have the largest
> > timestamp.
> >
> > On Mon, Sep 19, 2016 at 9:49 AM, Jun Rao  wrote:
> >
> > > Hi, Jiangjie,
> > >
> > > Thanks for the proposal. Looks good to me overall. Just a couple of
> minor
> > > comments.
> > >
> > > 1. I thought at some point you considered to only return offset in
> > > offsetsForTimes instead of offset and timestamp. One benefit of doing
> > that
> > > is that it will make the return type consistent among all three new
> apis.
> > > It also feels a bit weird that we only return timestamp, but not other
> > > metadata associated with a message.
> >
> I changed the interface for earliestOffsets() and latestOffsets() to return
> only offset instead of OffsetAndTimestamp because the timestamp may not
> always be meaningful.
> For offsetsForTimes(), the primary reason to return the timestamp with
> offset is because the found timestamp may not always be the same as target
> timestamp. Returning timestamp with offset may be useful if users do not
> always want to read the message afterwards. For example, user may not want
> to consume a message if its timestamp is too far away from the target
> timestamp. Also users may only want to perform some query based timestamp
> without consuming all the messages. e.g. total number of messages in a time
> range. I did not think much about other metadata of the message because it
> seems the interface only cares about the timestamp and the offsets.
>
> >
> > > 2. To be consistent with existing seek apis, would it be better to
> rename
> > > earliestOffsets() and latestOffsets() to beginningOffsets() and
> > > endOffsets()?
> >
> Good point, will make the change. This makes the name align with
> seekToBeginning() and seekToEnd().
>
> >
> > > Jun
> > >
> > > On Fri, Sep 9, 2016 at 4:38 PM, Becket Qin 
> wrote:
> > >
> > > > Hi all,
> > > >
> > > > I'd like to start the voting for KIP-79
> > > >
> > > > In short we propose to :
> > > > 1. add a ListOffsetRequest/ListOffsetResponse v1, and
> > > > 2. add earliestOffsts(), latestOffsets() and offsetForTime() methods
> in
> > > the
> > > > new consumer.
> > > >
> > > > The KIP wiki is the following:
> > > > https://cwiki.apache.org/confluence/pages/viewpage.
> > > action?pageId=65868090
> > > >
> > > > Thanks,
> > > >
> > > > Jiangjie (Becket) Qin
> > > >
> > >
> >
>

Re: [VOTE] KIP-79 - ListOffsetRequest v1 and search by timestamp methods in new consumer.

[Ismael Juma]

Hi all,

A few comments below.

On Mon, Sep 19, 2016 at 5:49 PM, Jun Rao  wrote:
>
> 1. I thought at some point you considered to only return offset in
> offsetsForTimes instead of offset and timestamp. One benefit of doing that
> is that it will make the return type consistent among all three new apis.
> It also feels a bit weird that we only return timestamp, but not other
> metadata associated with a message.
>

To me, this API is different enough from the other ones that it seems OK
for the return type to be different. And since we are querying by
timestamp, it makes sense to get the actual timestamp that matched in the
result (as Becket said elsewhere).

2. To be consistent with existing seek apis, would it be better to rename
> earliestOffsets() and latestOffsets() to beginningOffsets() and
> endOffsets()?
>

I was actually wondering why we were using `earliest`/`latest` instead of
`beginning`/`end`, so I like this suggestion.

To ensure the accordance with KIP-45, we have changed the
> "earliestOffset()" and "latestOffset()" method to take
> Collection instead of Set.


I agree with this change too (predictably since I suggested it :)).

Ismael

Re: [VOTE] KIP-79 - ListOffsetRequest v1 and search by timestamp methods in new consumer.

[Becket Qin]

Thanks for the comment Jun. Please see the reply in line

On Mon, Sep 19, 2016 at 10:21 AM, Jason Gustafson 
wrote:

> +1 on Jun's suggestion to use "beginning" and "end". The term "latest" is
> misleading since the last message in the log may not have the largest
> timestamp.
>
> On Mon, Sep 19, 2016 at 9:49 AM, Jun Rao  wrote:
>
> > Hi, Jiangjie,
> >
> > Thanks for the proposal. Looks good to me overall. Just a couple of minor
> > comments.
> >
> > 1. I thought at some point you considered to only return offset in
> > offsetsForTimes instead of offset and timestamp. One benefit of doing
> that
> > is that it will make the return type consistent among all three new apis.
> > It also feels a bit weird that we only return timestamp, but not other
> > metadata associated with a message.
>
I changed the interface for earliestOffsets() and latestOffsets() to return
only offset instead of OffsetAndTimestamp because the timestamp may not
always be meaningful.
For offsetsForTimes(), the primary reason to return the timestamp with
offset is because the found timestamp may not always be the same as target
timestamp. Returning timestamp with offset may be useful if users do not
always want to read the message afterwards. For example, user may not want
to consume a message if its timestamp is too far away from the target
timestamp. Also users may only want to perform some query based timestamp
without consuming all the messages. e.g. total number of messages in a time
range. I did not think much about other metadata of the message because it
seems the interface only cares about the timestamp and the offsets.

>
> > 2. To be consistent with existing seek apis, would it be better to rename
> > earliestOffsets() and latestOffsets() to beginningOffsets() and
> > endOffsets()?
>
Good point, will make the change. This makes the name align with
seekToBeginning() and seekToEnd().

>
> > Jun
> >
> > On Fri, Sep 9, 2016 at 4:38 PM, Becket Qin  wrote:
> >
> > > Hi all,
> > >
> > > I'd like to start the voting for KIP-79
> > >
> > > In short we propose to :
> > > 1. add a ListOffsetRequest/ListOffsetResponse v1, and
> > > 2. add earliestOffsts(), latestOffsets() and offsetForTime() methods in
> > the
> > > new consumer.
> > >
> > > The KIP wiki is the following:
> > > https://cwiki.apache.org/confluence/pages/viewpage.
> > action?pageId=65868090
> > >
> > > Thanks,
> > >
> > > Jiangjie (Becket) Qin
> > >
> >
>

Re: [VOTE] KIP-79 - ListOffsetRequest v1 and search by timestamp methods in new consumer.

[Jun Rao]

+1 assuming the minor comments are addressed.

Thanks,

Jun

On Mon, Sep 19, 2016 at 9:49 AM, Jun Rao  wrote:

> Hi, Jiangjie,
>
> Thanks for the proposal. Looks good to me overall. Just a couple of minor
> comments.
>
> 1. I thought at some point you considered to only return offset in
> offsetsForTimes instead of offset and timestamp. One benefit of doing that
> is that it will make the return type consistent among all three new apis.
> It also feels a bit weird that we only return timestamp, but not other
> metadata associated with a message.
>
> 2. To be consistent with existing seek apis, would it be better to rename
> earliestOffsets() and latestOffsets() to beginningOffsets() and
> endOffsets()?
>
> Jun
>
> On Fri, Sep 9, 2016 at 4:38 PM, Becket Qin  wrote:
>
>> Hi all,
>>
>> I'd like to start the voting for KIP-79
>>
>> In short we propose to :
>> 1. add a ListOffsetRequest/ListOffsetResponse v1, and
>> 2. add earliestOffsts(), latestOffsets() and offsetForTime() methods in
>> the
>> new consumer.
>>
>> The KIP wiki is the following:
>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=65868090
>>
>> Thanks,
>>
>> Jiangjie (Becket) Qin
>>
>
>

Re: [VOTE] KIP-79 - ListOffsetRequest v1 and search by timestamp methods in new consumer.

[Jason Gustafson]

+1 on Jun's suggestion to use "beginning" and "end". The term "latest" is
misleading since the last message in the log may not have the largest
timestamp.

On Mon, Sep 19, 2016 at 9:49 AM, Jun Rao  wrote:

> Hi, Jiangjie,
>
> Thanks for the proposal. Looks good to me overall. Just a couple of minor
> comments.
>
> 1. I thought at some point you considered to only return offset in
> offsetsForTimes instead of offset and timestamp. One benefit of doing that
> is that it will make the return type consistent among all three new apis.
> It also feels a bit weird that we only return timestamp, but not other
> metadata associated with a message.
>
> 2. To be consistent with existing seek apis, would it be better to rename
> earliestOffsets() and latestOffsets() to beginningOffsets() and
> endOffsets()?
>
> Jun
>
> On Fri, Sep 9, 2016 at 4:38 PM, Becket Qin  wrote:
>
> > Hi all,
> >
> > I'd like to start the voting for KIP-79
> >
> > In short we propose to :
> > 1. add a ListOffsetRequest/ListOffsetResponse v1, and
> > 2. add earliestOffsts(), latestOffsets() and offsetForTime() methods in
> the
> > new consumer.
> >
> > The KIP wiki is the following:
> > https://cwiki.apache.org/confluence/pages/viewpage.
> action?pageId=65868090
> >
> > Thanks,
> >
> > Jiangjie (Becket) Qin
> >
>

Re: [VOTE] KIP-79 - ListOffsetRequest v1 and search by timestamp methods in new consumer.

[Jun Rao]

Hi, Jiangjie,

Thanks for the proposal. Looks good to me overall. Just a couple of minor
comments.

1. I thought at some point you considered to only return offset in
offsetsForTimes instead of offset and timestamp. One benefit of doing that
is that it will make the return type consistent among all three new apis.
It also feels a bit weird that we only return timestamp, but not other
metadata associated with a message.

2. To be consistent with existing seek apis, would it be better to rename
earliestOffsets() and latestOffsets() to beginningOffsets() and
endOffsets()?

Jun

On Fri, Sep 9, 2016 at 4:38 PM, Becket Qin  wrote:

> Hi all,
>
> I'd like to start the voting for KIP-79
>
> In short we propose to :
> 1. add a ListOffsetRequest/ListOffsetResponse v1, and
> 2. add earliestOffsts(), latestOffsets() and offsetForTime() methods in the
> new consumer.
>
> The KIP wiki is the following:
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=65868090
>
> Thanks,
>
> Jiangjie (Becket) Qin
>

Re: [VOTE] KIP-79 - ListOffsetRequest v1 and search by timestamp methods in new consumer.

[Becket Qin]

Thanks everyone for the votes, I want to mention a minor change made to the
interface.

To ensure the accordance with KIP-45, we have changed the
"earliestOffset()" and "latestOffset()" method to take
Collection instead of Set.

Thanks,

Jiangjie (Becket) Qin


On Sun, Sep 18, 2016 at 5:53 PM, Neha Narkhede  wrote:

> +1 (binding)
> On Wed, Sep 14, 2016 at 9:57 AM Bill Bejeck  wrote:
>
> > +1
> >
> > On Tue, Sep 13, 2016 at 11:08 PM, Jason Gustafson 
> > wrote:
> >
> > > +1 and thanks for the great proposal!
> > >
> > > On Fri, Sep 9, 2016 at 4:38 PM, Becket Qin 
> wrote:
> > >
> > > > Hi all,
> > > >
> > > > I'd like to start the voting for KIP-79
> > > >
> > > > In short we propose to :
> > > > 1. add a ListOffsetRequest/ListOffsetResponse v1, and
> > > > 2. add earliestOffsts(), latestOffsets() and offsetForTime() methods
> in
> > > the
> > > > new consumer.
> > > >
> > > > The KIP wiki is the following:
> > > > https://cwiki.apache.org/confluence/pages/viewpage.
> > > action?pageId=65868090
> > > >
> > > > Thanks,
> > > >
> > > > Jiangjie (Becket) Qin
> > > >
> > >
> >
> --
> Thanks,
> Neha
>

Re: [VOTE] KIP-79 - ListOffsetRequest v1 and search by timestamp methods in new consumer.

[Neha Narkhede]

+1 (binding)
On Wed, Sep 14, 2016 at 9:57 AM Bill Bejeck  wrote:

> +1
>
> On Tue, Sep 13, 2016 at 11:08 PM, Jason Gustafson 
> wrote:
>
> > +1 and thanks for the great proposal!
> >
> > On Fri, Sep 9, 2016 at 4:38 PM, Becket Qin  wrote:
> >
> > > Hi all,
> > >
> > > I'd like to start the voting for KIP-79
> > >
> > > In short we propose to :
> > > 1. add a ListOffsetRequest/ListOffsetResponse v1, and
> > > 2. add earliestOffsts(), latestOffsets() and offsetForTime() methods in
> > the
> > > new consumer.
> > >
> > > The KIP wiki is the following:
> > > https://cwiki.apache.org/confluence/pages/viewpage.
> > action?pageId=65868090
> > >
> > > Thanks,
> > >
> > > Jiangjie (Becket) Qin
> > >
> >
>
-- 
Thanks,
Neha

Re: [VOTE] KIP-79 - ListOffsetRequest v1 and search by timestamp methods in new consumer.

[Bill Bejeck]

+1

On Tue, Sep 13, 2016 at 11:08 PM, Jason Gustafson 
wrote:

> +1 and thanks for the great proposal!
>
> On Fri, Sep 9, 2016 at 4:38 PM, Becket Qin  wrote:
>
> > Hi all,
> >
> > I'd like to start the voting for KIP-79
> >
> > In short we propose to :
> > 1. add a ListOffsetRequest/ListOffsetResponse v1, and
> > 2. add earliestOffsts(), latestOffsets() and offsetForTime() methods in
> the
> > new consumer.
> >
> > The KIP wiki is the following:
> > https://cwiki.apache.org/confluence/pages/viewpage.
> action?pageId=65868090
> >
> > Thanks,
> >
> > Jiangjie (Becket) Qin
> >
>

Re: [VOTE] KIP-79 - ListOffsetRequest v1 and search by timestamp methods in new consumer.

[Jason Gustafson]

+1 and thanks for the great proposal!

On Fri, Sep 9, 2016 at 4:38 PM, Becket Qin  wrote:

> Hi all,
>
> I'd like to start the voting for KIP-79
>
> In short we propose to :
> 1. add a ListOffsetRequest/ListOffsetResponse v1, and
> 2. add earliestOffsts(), latestOffsets() and offsetForTime() methods in the
> new consumer.
>
> The KIP wiki is the following:
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=65868090
>
> Thanks,
>
> Jiangjie (Becket) Qin
>