Re: [DISCUSS] KIP 88: OffsetFetch Protocol Update

2017-01-05 Thread Vahid S Hashemian
Hi Jason,

Sure, I'll do that shortly. Thanks.

--Vahid



From:   Jason Gustafson <ja...@confluent.io>
To: dev@kafka.apache.org
Date:   01/05/2017 09:59 AM
Subject:        Re: [DISCUSS] KIP 88: OffsetFetch Protocol Update



Hey Vahid,

Since there haven't been any additional comments, perhaps start a new 
vote?

-Jason

On Tue, Jan 3, 2017 at 2:14 PM, Ismael Juma <ism...@juma.me.uk> wrote:

> I also prefer option 1.
>
> Ismael
>
> On Fri, Dec 16, 2016 at 7:11 PM, Jason Gustafson <ja...@confluent.io>
> wrote:
>
> > Thanks Vahid. To clarify the impact of this issue, since we have no 
way
> to
> > send an error code in the OffsetFetchResponse when requesting all
> offsets,
> > we cannot detect when the coordinator has moved to another broker or 
when
> > it is still in the process of loading the offsets. This means we 
cannot
> > tell if there were was an error or if there were just no offsets 
stored
> for
> > the group. We've considered a few options:
> >
> > 1. Include an error code at the top level of the response. This seems
> like
> > the cleanest approach. The downside is that clients need to look for
> errors
> > in two locations for response errors. One small benefit is that many
> > OffsetFetch errors are group-level, so in that case, we can save the 
need
> > to return responses for all the requested partitions.
> > 2. Sort of hacky, but we could insert a "dummy" partition into the
> response
> > so that we have somewhere to return an error code.
> > 3. Include no error code, but use a null array in the response to
> indicate
> > that there was some error. If there was no error, and the group simply
> had
> > no partitions, then we return an empty array. I guess in this case, if
> the
> > client receives a null array in the response, it should assume the 
worst
> > and rediscover the coordinator and try again.
> >
> > My preference is the first one. Not sure if there are any other ideas?
> >
> > -Jason
> >
> > On Thu, Dec 15, 2016 at 3:02 PM, Vahid S Hashemian <
> > vahidhashem...@us.ibm.com> wrote:
> >
> > > Hi all,
> > >
> > > Even though KIP-88 was recently approved, due to a limitation that
> comes
> > > with the proposed protocol change in KIP-88 I'll have to re-open it 
to
> > > address the problem.
> > > I'd like to thank Jason Gustafson for catching this issue.
> > >
> > > I'll explain this in the KIP as well, but to summarize, KIP-88 
suggests
> > > adding the option of passing a "null" array in FetchOffset request 
to
> > > query all existing offsets for a consumer group. It does not suggest
> any
> > > modification to FetchOffset response.
> > >
> > > In the existing protocol, group or coordinator related errors are
> > reported
> > > along with each partition in the OffsetFetch response.
> > >
> > > If there are partitions in the request, they are guaranteed to 
appear
> in
> > > the response (there could be an error code associated with each). So 
if
> > > there is an error, it is reported back by being attached to some
> > partition
> > > in the request.
> > > If an empty array is passed, no error is reported (no matter what 
the
> > > group or coordinator status is). The response comes back with an 
empty
> > > list.
> > >
> > > With the proposed change in KIP-88 we could have a scenario in which 
a
> > > null array is sent in FetchOffset request, and due to some errors 
(for
> > > example if coordinator just started and hasn't caught up yet with 
the
> > > offset topic), an empty list is returned in the FetchOffset response
> (the
> > > group may or may not actually be empty). The issue is in situations
> like
> > > this no error can be returned in the response because there is no
> > > partition to attach the error to.
> > >
> > > I'll update the KIP with more details and propose to add to 
OffsetFetch
> > > response schema an "error_code" at the top level that can be used to
> > > report group related errors (instead of reporting those errors with
> each
> > > individual partition).
> > >
> > > I apologize if this causes any inconvenience.
> > >
> > > Feedback and comments are always welcome.
> > >
> > > Thanks.
> > > --Vahid
> > >
> > >
> >
>






Re: [DISCUSS] KIP 88: OffsetFetch Protocol Update

2017-01-05 Thread Jason Gustafson
Hey Vahid,

Since there haven't been any additional comments, perhaps start a new vote?

-Jason

On Tue, Jan 3, 2017 at 2:14 PM, Ismael Juma  wrote:

> I also prefer option 1.
>
> Ismael
>
> On Fri, Dec 16, 2016 at 7:11 PM, Jason Gustafson 
> wrote:
>
> > Thanks Vahid. To clarify the impact of this issue, since we have no way
> to
> > send an error code in the OffsetFetchResponse when requesting all
> offsets,
> > we cannot detect when the coordinator has moved to another broker or when
> > it is still in the process of loading the offsets. This means we cannot
> > tell if there were was an error or if there were just no offsets stored
> for
> > the group. We've considered a few options:
> >
> > 1. Include an error code at the top level of the response. This seems
> like
> > the cleanest approach. The downside is that clients need to look for
> errors
> > in two locations for response errors. One small benefit is that many
> > OffsetFetch errors are group-level, so in that case, we can save the need
> > to return responses for all the requested partitions.
> > 2. Sort of hacky, but we could insert a "dummy" partition into the
> response
> > so that we have somewhere to return an error code.
> > 3. Include no error code, but use a null array in the response to
> indicate
> > that there was some error. If there was no error, and the group simply
> had
> > no partitions, then we return an empty array. I guess in this case, if
> the
> > client receives a null array in the response, it should assume the worst
> > and rediscover the coordinator and try again.
> >
> > My preference is the first one. Not sure if there are any other ideas?
> >
> > -Jason
> >
> > On Thu, Dec 15, 2016 at 3:02 PM, Vahid S Hashemian <
> > vahidhashem...@us.ibm.com> wrote:
> >
> > > Hi all,
> > >
> > > Even though KIP-88 was recently approved, due to a limitation that
> comes
> > > with the proposed protocol change in KIP-88 I'll have to re-open it to
> > > address the problem.
> > > I'd like to thank Jason Gustafson for catching this issue.
> > >
> > > I'll explain this in the KIP as well, but to summarize, KIP-88 suggests
> > > adding the option of passing a "null" array in FetchOffset request to
> > > query all existing offsets for a consumer group. It does not suggest
> any
> > > modification to FetchOffset response.
> > >
> > > In the existing protocol, group or coordinator related errors are
> > reported
> > > along with each partition in the OffsetFetch response.
> > >
> > > If there are partitions in the request, they are guaranteed to appear
> in
> > > the response (there could be an error code associated with each). So if
> > > there is an error, it is reported back by being attached to some
> > partition
> > > in the request.
> > > If an empty array is passed, no error is reported (no matter what the
> > > group or coordinator status is). The response comes back with an empty
> > > list.
> > >
> > > With the proposed change in KIP-88 we could have a scenario in which a
> > > null array is sent in FetchOffset request, and due to some errors (for
> > > example if coordinator just started and hasn't caught up yet with the
> > > offset topic), an empty list is returned in the FetchOffset response
> (the
> > > group may or may not actually be empty). The issue is in situations
> like
> > > this no error can be returned in the response because there is no
> > > partition to attach the error to.
> > >
> > > I'll update the KIP with more details and propose to add to OffsetFetch
> > > response schema an "error_code" at the top level that can be used to
> > > report group related errors (instead of reporting those errors with
> each
> > > individual partition).
> > >
> > > I apologize if this causes any inconvenience.
> > >
> > > Feedback and comments are always welcome.
> > >
> > > Thanks.
> > > --Vahid
> > >
> > >
> >
>


Re: [DISCUSS] KIP 88: OffsetFetch Protocol Update

2017-01-03 Thread Ismael Juma
I also prefer option 1.

Ismael

On Fri, Dec 16, 2016 at 7:11 PM, Jason Gustafson  wrote:

> Thanks Vahid. To clarify the impact of this issue, since we have no way to
> send an error code in the OffsetFetchResponse when requesting all offsets,
> we cannot detect when the coordinator has moved to another broker or when
> it is still in the process of loading the offsets. This means we cannot
> tell if there were was an error or if there were just no offsets stored for
> the group. We've considered a few options:
>
> 1. Include an error code at the top level of the response. This seems like
> the cleanest approach. The downside is that clients need to look for errors
> in two locations for response errors. One small benefit is that many
> OffsetFetch errors are group-level, so in that case, we can save the need
> to return responses for all the requested partitions.
> 2. Sort of hacky, but we could insert a "dummy" partition into the response
> so that we have somewhere to return an error code.
> 3. Include no error code, but use a null array in the response to indicate
> that there was some error. If there was no error, and the group simply had
> no partitions, then we return an empty array. I guess in this case, if the
> client receives a null array in the response, it should assume the worst
> and rediscover the coordinator and try again.
>
> My preference is the first one. Not sure if there are any other ideas?
>
> -Jason
>
> On Thu, Dec 15, 2016 at 3:02 PM, Vahid S Hashemian <
> vahidhashem...@us.ibm.com> wrote:
>
> > Hi all,
> >
> > Even though KIP-88 was recently approved, due to a limitation that comes
> > with the proposed protocol change in KIP-88 I'll have to re-open it to
> > address the problem.
> > I'd like to thank Jason Gustafson for catching this issue.
> >
> > I'll explain this in the KIP as well, but to summarize, KIP-88 suggests
> > adding the option of passing a "null" array in FetchOffset request to
> > query all existing offsets for a consumer group. It does not suggest any
> > modification to FetchOffset response.
> >
> > In the existing protocol, group or coordinator related errors are
> reported
> > along with each partition in the OffsetFetch response.
> >
> > If there are partitions in the request, they are guaranteed to appear in
> > the response (there could be an error code associated with each). So if
> > there is an error, it is reported back by being attached to some
> partition
> > in the request.
> > If an empty array is passed, no error is reported (no matter what the
> > group or coordinator status is). The response comes back with an empty
> > list.
> >
> > With the proposed change in KIP-88 we could have a scenario in which a
> > null array is sent in FetchOffset request, and due to some errors (for
> > example if coordinator just started and hasn't caught up yet with the
> > offset topic), an empty list is returned in the FetchOffset response (the
> > group may or may not actually be empty). The issue is in situations like
> > this no error can be returned in the response because there is no
> > partition to attach the error to.
> >
> > I'll update the KIP with more details and propose to add to OffsetFetch
> > response schema an "error_code" at the top level that can be used to
> > report group related errors (instead of reporting those errors with each
> > individual partition).
> >
> > I apologize if this causes any inconvenience.
> >
> > Feedback and comments are always welcome.
> >
> > Thanks.
> > --Vahid
> >
> >
>


Re: [DISCUSS] KIP 88: OffsetFetch Protocol Update

2017-01-03 Thread Ewen Cheslack-Postava
Vahid,

This looks reasonable to me and fits well with the changes made for
metadata requests.

-Ewen

On Tue, Jan 3, 2017 at 12:12 PM, Vahid S Hashemian <
vahidhashem...@us.ibm.com> wrote:

> One more try to ask for feedback on this KIP (that had to go through some
> more changes after approval):
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 88%3A+OffsetFetch+Protocol+Update
>
> If there is no concern I could start the vote, hoping it could make it to
> the 0.10.2.0 release.
>
> Thanks.
>
> Regards,
> --Vahid
>
>
>
>
> From:   Vahid S Hashemian/Silicon Valley/IBM@IBMUS
> To: dev@kafka.apache.org
> Date:   12/19/2016 01:25 PM
> Subject:Re: [DISCUSS] KIP 88: OffsetFetch Protocol Update
>
>
>
> Happy Monday,
>
> Jason, thanks for further explaining the issue.
>
> I have updated the KIP and reflected the recent discussions in there:
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 88%3A+OffsetFetch+Protocol+Update
>
> You can also see the modifications to the KIP compared to the approved
> version here:
> https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?
> pageId=66849788=26=25
>
>
> Feedback and comments are welcome.
>
> Thanks.
> --Vahid
>
>
>
> From:   Jason Gustafson <ja...@confluent.io>
> To: dev@kafka.apache.org
> Date:   12/16/2016 11:11 AM
> Subject:Re: [DISCUSS] KIP 88: OffsetFetch Protocol Update
>
>
>
> Thanks Vahid. To clarify the impact of this issue, since we have no way to
> send an error code in the OffsetFetchResponse when requesting all offsets,
> we cannot detect when the coordinator has moved to another broker or when
> it is still in the process of loading the offsets. This means we cannot
> tell if there were was an error or if there were just no offsets stored
> for
> the group. We've considered a few options:
>
> 1. Include an error code at the top level of the response. This seems like
> the cleanest approach. The downside is that clients need to look for
> errors
> in two locations for response errors. One small benefit is that many
> OffsetFetch errors are group-level, so in that case, we can save the need
> to return responses for all the requested partitions.
> 2. Sort of hacky, but we could insert a "dummy" partition into the
> response
> so that we have somewhere to return an error code.
> 3. Include no error code, but use a null array in the response to indicate
> that there was some error. If there was no error, and the group simply had
> no partitions, then we return an empty array. I guess in this case, if the
> client receives a null array in the response, it should assume the worst
> and rediscover the coordinator and try again.
>
> My preference is the first one. Not sure if there are any other ideas?
>
> -Jason
>
> On Thu, Dec 15, 2016 at 3:02 PM, Vahid S Hashemian <
> vahidhashem...@us.ibm.com> wrote:
>
> > Hi all,
> >
> > Even though KIP-88 was recently approved, due to a limitation that comes
> > with the proposed protocol change in KIP-88 I'll have to re-open it to
> > address the problem.
> > I'd like to thank Jason Gustafson for catching this issue.
> >
> > I'll explain this in the KIP as well, but to summarize, KIP-88 suggests
> > adding the option of passing a "null" array in FetchOffset request to
> > query all existing offsets for a consumer group. It does not suggest any
> > modification to FetchOffset response.
> >
> > In the existing protocol, group or coordinator related errors are
> reported
> > along with each partition in the OffsetFetch response.
> >
> > If there are partitions in the request, they are guaranteed to appear in
> > the response (there could be an error code associated with each). So if
> > there is an error, it is reported back by being attached to some
> partition
> > in the request.
> > If an empty array is passed, no error is reported (no matter what the
> > group or coordinator status is). The response comes back with an empty
> > list.
> >
> > With the proposed change in KIP-88 we could have a scenario in which a
> > null array is sent in FetchOffset request, and due to some errors (for
> > example if coordinator just started and hasn't caught up yet with the
> > offset topic), an empty list is returned in the FetchOffset response
> (the
> > group may or may not actually be empty). The issue is in situations like
> > this no error can be returned in the response because there is no
> > partition to attach the error to.
> >
> > I'll update the KIP with more details and propose to add to OffsetFetch
> > response schema an "error_code" at the top level that can be used to
> > report group related errors (instead of reporting those errors with each
> > individual partition).
> >
> > I apologize if this causes any inconvenience.
> >
> > Feedback and comments are always welcome.
> >
> > Thanks.
> > --Vahid
> >
> >
>
>
>
>
>
>
>
>
>


Re: [DISCUSS] KIP 88: OffsetFetch Protocol Update

2017-01-03 Thread Vahid S Hashemian
One more try to ask for feedback on this KIP (that had to go through some 
more changes after approval): 
https://cwiki.apache.org/confluence/display/KAFKA/KIP-88%3A+OffsetFetch+Protocol+Update

If there is no concern I could start the vote, hoping it could make it to 
the 0.10.2.0 release.

Thanks.
 
Regards,
--Vahid




From:   Vahid S Hashemian/Silicon Valley/IBM@IBMUS
To: dev@kafka.apache.org
Date:   12/19/2016 01:25 PM
Subject:Re: [DISCUSS] KIP 88: OffsetFetch Protocol Update



Happy Monday,

Jason, thanks for further explaining the issue.

I have updated the KIP and reflected the recent discussions in there: 
https://cwiki.apache.org/confluence/display/KAFKA/KIP-88%3A+OffsetFetch+Protocol+Update

You can also see the modifications to the KIP compared to the approved 
version here: 
https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=66849788=26=25


Feedback and comments are welcome.

Thanks.
--Vahid



From:   Jason Gustafson <ja...@confluent.io>
To: dev@kafka.apache.org
Date:   12/16/2016 11:11 AM
Subject:        Re: [DISCUSS] KIP 88: OffsetFetch Protocol Update



Thanks Vahid. To clarify the impact of this issue, since we have no way to
send an error code in the OffsetFetchResponse when requesting all offsets,
we cannot detect when the coordinator has moved to another broker or when
it is still in the process of loading the offsets. This means we cannot
tell if there were was an error or if there were just no offsets stored 
for
the group. We've considered a few options:

1. Include an error code at the top level of the response. This seems like
the cleanest approach. The downside is that clients need to look for 
errors
in two locations for response errors. One small benefit is that many
OffsetFetch errors are group-level, so in that case, we can save the need
to return responses for all the requested partitions.
2. Sort of hacky, but we could insert a "dummy" partition into the 
response
so that we have somewhere to return an error code.
3. Include no error code, but use a null array in the response to indicate
that there was some error. If there was no error, and the group simply had
no partitions, then we return an empty array. I guess in this case, if the
client receives a null array in the response, it should assume the worst
and rediscover the coordinator and try again.

My preference is the first one. Not sure if there are any other ideas?

-Jason

On Thu, Dec 15, 2016 at 3:02 PM, Vahid S Hashemian <
vahidhashem...@us.ibm.com> wrote:

> Hi all,
>
> Even though KIP-88 was recently approved, due to a limitation that comes
> with the proposed protocol change in KIP-88 I'll have to re-open it to
> address the problem.
> I'd like to thank Jason Gustafson for catching this issue.
>
> I'll explain this in the KIP as well, but to summarize, KIP-88 suggests
> adding the option of passing a "null" array in FetchOffset request to
> query all existing offsets for a consumer group. It does not suggest any
> modification to FetchOffset response.
>
> In the existing protocol, group or coordinator related errors are 
reported
> along with each partition in the OffsetFetch response.
>
> If there are partitions in the request, they are guaranteed to appear in
> the response (there could be an error code associated with each). So if
> there is an error, it is reported back by being attached to some 
partition
> in the request.
> If an empty array is passed, no error is reported (no matter what the
> group or coordinator status is). The response comes back with an empty
> list.
>
> With the proposed change in KIP-88 we could have a scenario in which a
> null array is sent in FetchOffset request, and due to some errors (for
> example if coordinator just started and hasn't caught up yet with the
> offset topic), an empty list is returned in the FetchOffset response 
(the
> group may or may not actually be empty). The issue is in situations like
> this no error can be returned in the response because there is no
> partition to attach the error to.
>
> I'll update the KIP with more details and propose to add to OffsetFetch
> response schema an "error_code" at the top level that can be used to
> report group related errors (instead of reporting those errors with each
> individual partition).
>
> I apologize if this causes any inconvenience.
>
> Feedback and comments are always welcome.
>
> Thanks.
> --Vahid
>
>










Re: [DISCUSS] KIP 88: OffsetFetch Protocol Update

2016-12-19 Thread Vahid S Hashemian
Happy Monday,

Jason, thanks for further explaining the issue.

I have updated the KIP and reflected the recent discussions in there: 
https://cwiki.apache.org/confluence/display/KAFKA/KIP-88%3A+OffsetFetch+Protocol+Update
You can also see the modifications to the KIP compared to the approved 
version here: 
https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=66849788=26=25

Feedback and comments are welcome.

Thanks.
--Vahid



From:   Jason Gustafson <ja...@confluent.io>
To: dev@kafka.apache.org
Date:   12/16/2016 11:11 AM
Subject:        Re: [DISCUSS] KIP 88: OffsetFetch Protocol Update



Thanks Vahid. To clarify the impact of this issue, since we have no way to
send an error code in the OffsetFetchResponse when requesting all offsets,
we cannot detect when the coordinator has moved to another broker or when
it is still in the process of loading the offsets. This means we cannot
tell if there were was an error or if there were just no offsets stored 
for
the group. We've considered a few options:

1. Include an error code at the top level of the response. This seems like
the cleanest approach. The downside is that clients need to look for 
errors
in two locations for response errors. One small benefit is that many
OffsetFetch errors are group-level, so in that case, we can save the need
to return responses for all the requested partitions.
2. Sort of hacky, but we could insert a "dummy" partition into the 
response
so that we have somewhere to return an error code.
3. Include no error code, but use a null array in the response to indicate
that there was some error. If there was no error, and the group simply had
no partitions, then we return an empty array. I guess in this case, if the
client receives a null array in the response, it should assume the worst
and rediscover the coordinator and try again.

My preference is the first one. Not sure if there are any other ideas?

-Jason

On Thu, Dec 15, 2016 at 3:02 PM, Vahid S Hashemian <
vahidhashem...@us.ibm.com> wrote:

> Hi all,
>
> Even though KIP-88 was recently approved, due to a limitation that comes
> with the proposed protocol change in KIP-88 I'll have to re-open it to
> address the problem.
> I'd like to thank Jason Gustafson for catching this issue.
>
> I'll explain this in the KIP as well, but to summarize, KIP-88 suggests
> adding the option of passing a "null" array in FetchOffset request to
> query all existing offsets for a consumer group. It does not suggest any
> modification to FetchOffset response.
>
> In the existing protocol, group or coordinator related errors are 
reported
> along with each partition in the OffsetFetch response.
>
> If there are partitions in the request, they are guaranteed to appear in
> the response (there could be an error code associated with each). So if
> there is an error, it is reported back by being attached to some 
partition
> in the request.
> If an empty array is passed, no error is reported (no matter what the
> group or coordinator status is). The response comes back with an empty
> list.
>
> With the proposed change in KIP-88 we could have a scenario in which a
> null array is sent in FetchOffset request, and due to some errors (for
> example if coordinator just started and hasn't caught up yet with the
> offset topic), an empty list is returned in the FetchOffset response 
(the
> group may or may not actually be empty). The issue is in situations like
> this no error can be returned in the response because there is no
> partition to attach the error to.
>
> I'll update the KIP with more details and propose to add to OffsetFetch
> response schema an "error_code" at the top level that can be used to
> report group related errors (instead of reporting those errors with each
> individual partition).
>
> I apologize if this causes any inconvenience.
>
> Feedback and comments are always welcome.
>
> Thanks.
> --Vahid
>
>






Re: [DISCUSS] KIP 88: OffsetFetch Protocol Update

2016-12-16 Thread Jason Gustafson
Thanks Vahid. To clarify the impact of this issue, since we have no way to
send an error code in the OffsetFetchResponse when requesting all offsets,
we cannot detect when the coordinator has moved to another broker or when
it is still in the process of loading the offsets. This means we cannot
tell if there were was an error or if there were just no offsets stored for
the group. We've considered a few options:

1. Include an error code at the top level of the response. This seems like
the cleanest approach. The downside is that clients need to look for errors
in two locations for response errors. One small benefit is that many
OffsetFetch errors are group-level, so in that case, we can save the need
to return responses for all the requested partitions.
2. Sort of hacky, but we could insert a "dummy" partition into the response
so that we have somewhere to return an error code.
3. Include no error code, but use a null array in the response to indicate
that there was some error. If there was no error, and the group simply had
no partitions, then we return an empty array. I guess in this case, if the
client receives a null array in the response, it should assume the worst
and rediscover the coordinator and try again.

My preference is the first one. Not sure if there are any other ideas?

-Jason

On Thu, Dec 15, 2016 at 3:02 PM, Vahid S Hashemian <
vahidhashem...@us.ibm.com> wrote:

> Hi all,
>
> Even though KIP-88 was recently approved, due to a limitation that comes
> with the proposed protocol change in KIP-88 I'll have to re-open it to
> address the problem.
> I'd like to thank Jason Gustafson for catching this issue.
>
> I'll explain this in the KIP as well, but to summarize, KIP-88 suggests
> adding the option of passing a "null" array in FetchOffset request to
> query all existing offsets for a consumer group. It does not suggest any
> modification to FetchOffset response.
>
> In the existing protocol, group or coordinator related errors are reported
> along with each partition in the OffsetFetch response.
>
> If there are partitions in the request, they are guaranteed to appear in
> the response (there could be an error code associated with each). So if
> there is an error, it is reported back by being attached to some partition
> in the request.
> If an empty array is passed, no error is reported (no matter what the
> group or coordinator status is). The response comes back with an empty
> list.
>
> With the proposed change in KIP-88 we could have a scenario in which a
> null array is sent in FetchOffset request, and due to some errors (for
> example if coordinator just started and hasn't caught up yet with the
> offset topic), an empty list is returned in the FetchOffset response (the
> group may or may not actually be empty). The issue is in situations like
> this no error can be returned in the response because there is no
> partition to attach the error to.
>
> I'll update the KIP with more details and propose to add to OffsetFetch
> response schema an "error_code" at the top level that can be used to
> report group related errors (instead of reporting those errors with each
> individual partition).
>
> I apologize if this causes any inconvenience.
>
> Feedback and comments are always welcome.
>
> Thanks.
> --Vahid
>
>


Re: [DISCUSS] KIP 88: OffsetFetch Protocol Update

2016-12-15 Thread Vahid S Hashemian
Hi all,

Even though KIP-88 was recently approved, due to a limitation that comes 
with the proposed protocol change in KIP-88 I'll have to re-open it to 
address the problem.
I'd like to thank Jason Gustafson for catching this issue.

I'll explain this in the KIP as well, but to summarize, KIP-88 suggests 
adding the option of passing a "null" array in FetchOffset request to 
query all existing offsets for a consumer group. It does not suggest any 
modification to FetchOffset response.

In the existing protocol, group or coordinator related errors are reported 
along with each partition in the OffsetFetch response.

If there are partitions in the request, they are guaranteed to appear in 
the response (there could be an error code associated with each). So if 
there is an error, it is reported back by being attached to some partition 
in the request.
If an empty array is passed, no error is reported (no matter what the 
group or coordinator status is). The response comes back with an empty 
list.

With the proposed change in KIP-88 we could have a scenario in which a 
null array is sent in FetchOffset request, and due to some errors (for 
example if coordinator just started and hasn't caught up yet with the 
offset topic), an empty list is returned in the FetchOffset response (the 
group may or may not actually be empty). The issue is in situations like 
this no error can be returned in the response because there is no 
partition to attach the error to.

I'll update the KIP with more details and propose to add to OffsetFetch 
response schema an "error_code" at the top level that can be used to 
report group related errors (instead of reporting those errors with each 
individual partition).

I apologize if this causes any inconvenience.

Feedback and comments are always welcome.

Thanks.
--Vahid



Re: [DISCUSS] KIP 88: OffsetFetch Protocol Update

2016-11-30 Thread Vahid S Hashemian
Hi Jason,

Thanks for the feedback. I believe the proposal now covers what you're 
suggesting.
I also like the AdminClient option better, and that what made me update 
the KIP to take that approach, and suggest the modification to OffsetFetch 
protocol as you pointed out.

I'll wait until Monday, and then put it up for a vote if no concern is 
raised by then.

Regards,
--Vahid



From:   Jason Gustafson <ja...@confluent.io>
To: dev@kafka.apache.org
Date:   11/30/2016 04:00 PM
Subject:        Re: [DISCUSS] KIP 88: OffsetFetch Protocol Update



Hey Vahid,

Sorry to getting to this so late. I don't have a strong preference for
keeping it out of the KafkaConsumer API. Having a no-arg committed() 
method
which returns all the committed offsets seems natural and could be useful.
Since no one has asked for it, however, I'd lean toward supporting this
functionality only in the AdminClient, which appears to be what you chose.
If that's the case, then the only public API change here is that
OffsetFetchRequest can accept null for the list of partitions, which seems
reasonable. I also think it's a nice improvement not to have to create the
"dummy" consumer to lookup offsets.

Since this KIP has been out for a while, maybe put it to a vote if no one
else has feedback in the next couple days?

Thanks,
Jason

On Tue, Nov 15, 2016 at 12:58 PM, Vahid S Hashemian <
vahidhashem...@us.ibm.com> wrote:

> Hi Jason,
>
> I updated the KIP one more time to make use of AdminClient to access the
> updated API.
> I made a note of the previous two versions of the KIP under "Rejected
> Alternatives".
>
> Thanks.
> --Vahid
>
>
>
> From:   Vahid S Hashemian/Silicon Valley/IBM@IBMUS
> To:     dev@kafka.apache.org
> Date:   11/09/2016 11:21 AM
> Subject:Re: [DISCUSS] KIP 88: OffsetFetch Protocol Update
>
>
>
> Jason,
> For some reason I did not receive your earlier response to the thread.
> I just saw it when I went to
> https://www.mail-archive.com/dev@kafka.apache.org/msg59608.html
> In the updated KIP I exposed the capability via KafkaConsumer (your 
first
> suggestion), but would be happy to look into adding it to AdminClient in
> the next round if you think that's the better approach.
> Thanks.
> --Vahid
>
>
>
>
>
>
>
>
>
>
>
>
>






Re: [DISCUSS] KIP 88: OffsetFetch Protocol Update

2016-11-30 Thread Jason Gustafson
Hey Vahid,

Sorry to getting to this so late. I don't have a strong preference for
keeping it out of the KafkaConsumer API. Having a no-arg committed() method
which returns all the committed offsets seems natural and could be useful.
Since no one has asked for it, however, I'd lean toward supporting this
functionality only in the AdminClient, which appears to be what you chose.
If that's the case, then the only public API change here is that
OffsetFetchRequest can accept null for the list of partitions, which seems
reasonable. I also think it's a nice improvement not to have to create the
"dummy" consumer to lookup offsets.

Since this KIP has been out for a while, maybe put it to a vote if no one
else has feedback in the next couple days?

Thanks,
Jason

On Tue, Nov 15, 2016 at 12:58 PM, Vahid S Hashemian <
vahidhashem...@us.ibm.com> wrote:

> Hi Jason,
>
> I updated the KIP one more time to make use of AdminClient to access the
> updated API.
> I made a note of the previous two versions of the KIP under "Rejected
> Alternatives".
>
> Thanks.
> --Vahid
>
>
>
> From:   Vahid S Hashemian/Silicon Valley/IBM@IBMUS
> To: dev@kafka.apache.org
> Date:   11/09/2016 11:21 AM
> Subject:Re: [DISCUSS] KIP 88: OffsetFetch Protocol Update
>
>
>
> Jason,
> For some reason I did not receive your earlier response to the thread.
> I just saw it when I went to
> https://www.mail-archive.com/dev@kafka.apache.org/msg59608.html
> In the updated KIP I exposed the capability via KafkaConsumer (your first
> suggestion), but would be happy to look into adding it to AdminClient in
> the next round if you think that's the better approach.
> Thanks.
> --Vahid
>
>
>
>
>
>
>
>
>
>
>
>
>


Re: [DISCUSS] KIP 88: OffsetFetch Protocol Update

2016-11-15 Thread Vahid S Hashemian
Hi Jason,

I updated the KIP one more time to make use of AdminClient to access the 
updated API.
I made a note of the previous two versions of the KIP under "Rejected 
Alternatives".

Thanks.
--Vahid



From:   Vahid S Hashemian/Silicon Valley/IBM@IBMUS
To: dev@kafka.apache.org
Date:   11/09/2016 11:21 AM
Subject:    Re: [DISCUSS] KIP 88: OffsetFetch Protocol Update



Jason,
For some reason I did not receive your earlier response to the thread.
I just saw it when I went to 
https://www.mail-archive.com/dev@kafka.apache.org/msg59608.html
In the updated KIP I exposed the capability via KafkaConsumer (your first 
suggestion), but would be happy to look into adding it to AdminClient in 
the next round if you think that's the better approach.
Thanks.
--Vahid














Re: [DISCUSS] KIP 88: OffsetFetch Protocol Update

2016-11-09 Thread Vahid S Hashemian
Jason,
For some reason I did not receive your earlier response to the thread.
I just saw it when I went to 
https://www.mail-archive.com/dev@kafka.apache.org/msg59608.html
In the updated KIP I exposed the capability via KafkaConsumer (your first 
suggestion), but would be happy to look into adding it to AdminClient in 
the next round if you think that's the better approach.
Thanks.
--Vahid



From:   Vahid S Hashemian/Silicon Valley/IBM@IBMUS
To: dev@kafka.apache.org
Date:   11/09/2016 11:12 AM
Subject:Re: [DISCUSS] KIP 88: OffsetFetch Protocol Update



Hi Jason (et al.),

I modified the KIP towards a change in OffsetFetch protocol, as per your 
suggestion.
https://cwiki.apache.org/confluence/display/KAFKA/KIP-88%3A+OffsetFetch+Protocol+Update


I'd appreciate it if you could take another look and share your thoughts.
Thanks.
--Vahid



From:   Vahid S Hashemian/Silicon Valley/IBM@IBMUS
To: dev@kafka.apache.org
Date:   11/07/2016 03:28 PM
Subject:Re: [DISCUSS] KIP 88: DescribeGroups Protocol Update



Hi Jason,

Thanks for your feedback.

Yes, the intent of the KIP is to make existing offsets of the group 
available even when there is no active consumers in the group consuming 
from one or more topic partitions.
Your suggestion should also work. I'm not yet sure how to obtain group's 
all topic partitions and need to look more closely at your 'null=all' 
suggestion, as I couldn't immediately see an obvious way to extract that 
info in the OffsetFetch handler. I'll dig deeper.
Just a quick note that using the OffsetFetch API would not address the 
second (but minor) problem described in the current KIP (the dummy group 
member). I assume that is not a big concern.

Thanks.
--Vahid




From:   Jason Gustafson <ja...@confluent.io>
To: dev@kafka.apache.org
Date:   11/07/2016 09:19 AM
Subject:Re: [DISCUSS] KIP 88: DescribeGroups Protocol Update



Hey Vahid,

Thanks for the KIP. If I understand correctly, the problem is how to fetch
existing offsets for a group which has no active members, right? I'm not
totally clear why we need to modify the DescribeGroups API in order to
achieve this since we already have the OffsetFetch API. I think the
limitation currently is that you need to know the partitions to fetch
offsets for, but perhaps we could modify it to support the "null=all"
semantics that we used for the TopicMetadata API?

Thanks,
Jason

On Thu, Nov 3, 2016 at 11:09 AM, Vahid S Hashemian <
vahidhashem...@us.ibm.com> wrote:

> Hi all,
>
> I started a new KIP under
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 88%3A+DescribeGroups+Protocol+Update
> .
>
> The KIP is a proposal to update the DescribeGroups protocol to address
> KAFKA-3853 (https://issues.apache.org/jira/browse/KAFKA-3853).
>
> I appreciate your feedback.
>
> Thanks.
> --Vahid
>
>














Re: [DISCUSS] KIP 88: OffsetFetch Protocol Update

2016-11-09 Thread Vahid S Hashemian
Hi Jason (et al.),

I modified the KIP towards a change in OffsetFetch protocol, as per your 
suggestion.
https://cwiki.apache.org/confluence/display/KAFKA/KIP-88%3A+OffsetFetch+Protocol+Update

I'd appreciate it if you could take another look and share your thoughts.
Thanks.
--Vahid



From:   Vahid S Hashemian/Silicon Valley/IBM@IBMUS
To: dev@kafka.apache.org
Date:   11/07/2016 03:28 PM
Subject:Re: [DISCUSS] KIP 88: DescribeGroups Protocol Update



Hi Jason,

Thanks for your feedback.

Yes, the intent of the KIP is to make existing offsets of the group 
available even when there is no active consumers in the group consuming 
from one or more topic partitions.
Your suggestion should also work. I'm not yet sure how to obtain group's 
all topic partitions and need to look more closely at your 'null=all' 
suggestion, as I couldn't immediately see an obvious way to extract that 
info in the OffsetFetch handler. I'll dig deeper.
Just a quick note that using the OffsetFetch API would not address the 
second (but minor) problem described in the current KIP (the dummy group 
member). I assume that is not a big concern.

Thanks.
--Vahid




From:   Jason Gustafson 
To: dev@kafka.apache.org
Date:   11/07/2016 09:19 AM
Subject:Re: [DISCUSS] KIP 88: DescribeGroups Protocol Update



Hey Vahid,

Thanks for the KIP. If I understand correctly, the problem is how to fetch
existing offsets for a group which has no active members, right? I'm not
totally clear why we need to modify the DescribeGroups API in order to
achieve this since we already have the OffsetFetch API. I think the
limitation currently is that you need to know the partitions to fetch
offsets for, but perhaps we could modify it to support the "null=all"
semantics that we used for the TopicMetadata API?

Thanks,
Jason

On Thu, Nov 3, 2016 at 11:09 AM, Vahid S Hashemian <
vahidhashem...@us.ibm.com> wrote:

> Hi all,
>
> I started a new KIP under
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 88%3A+DescribeGroups+Protocol+Update
> .
>
> The KIP is a proposal to update the DescribeGroups protocol to address
> KAFKA-3853 (https://issues.apache.org/jira/browse/KAFKA-3853).
>
> I appreciate your feedback.
>
> Thanks.
> --Vahid
>
>