Sounds good. Just pointing out that the metrics scope likely overlap with https://issues.apache.org/jira/browse/KAFKA-3556, so may better document that in the wiki page.
Guozhang On Mon, Jul 29, 2019 at 2:48 PM Jason Gustafson <ja...@confluent.io> wrote: > Hi Guozhang, > > I have added metrics to the KIP. Please take a look. This gave me an excuse > to also add a metric for the group rebalance rate, which probably would > have made detecting KAFKA-8653 easier. > > Since this is a relatively straightforward KIP, I will go ahead and start a > vote later this week if there are no further comments. > > Thanks, > Jason > > On Mon, Jul 29, 2019 at 9:10 AM Guozhang Wang <wangg...@gmail.com> wrote: > > > Thanks for the replies Jason! > > > > 2. No I do not see any problems, just trying to understand how restrict > we > > are applying this rule :) Piggy-backing on the existing background thread > > and check interval mechanism means we are not "eagerly" expiring either, > > but I think this is fine. > > > > > > Guozhang > > > > > > On Thu, Jul 25, 2019 at 3:16 PM Jason Gustafson <ja...@confluent.io> > > wrote: > > > > > 1. Fixed, thanks! > > > > > > 2. Yes, that is what I was thinking. Do you see any problems? > > > > > > 3. Good point. Do you think a meter for expired and deleted offsets > would > > > be sufficient? > > > > > > 4. I considered it. I thought that might be a little dangerous for > > dynamic > > > groups which have subscriptions changing. If the first member to > > discover a > > > subscription change falls out, then offsets would be lost. Also, it > > seemed > > > a little more consistent with empty group expiration. From an offset > > > expiration perspective, an empty group is just treated as a case where > > the > > > subscription is empty, which makes all offsets subject to expiration. > > > > > > > > > Thanks, > > > Jason > > > > > > On Thu, Jul 25, 2019 at 1:41 PM Guozhang Wang <wangg...@gmail.com> > > wrote: > > > > > > > Hi Jason, > > > > > > > > Thanks for the KIP! I've made a pass on it and here are few comments: > > > > > > > > 1. " before the clients which ." --> incomplete sentence? > > > > > > > > 2. " Any committed offset for a partition which is not currently > > > subscribed > > > > to is subject to expiration." --> this may be an implementation > detail, > > > but > > > > are we going to piggy-back on the same > offsetsRetentionCheckIntervalMs > > to > > > > check for expirable offsets as well? > > > > > > > > Some meta comment: > > > > > > > > 3. Looking into the current broker-side metrics, we do not have a > good > > > user > > > > visibility yet for offset removal either due to expiration or > deletion, > > > > maybe we should consider adding one? > > > > > > > > 4. Playing the devil's advocate here: for cases where deleting > > expirable > > > > offsets is needed (like you mentioned lag monitoring), should we just > > > > by-pass the offset retention ms (by default it's one day) and remove > > > > immediately? What scenarios would require those non-subscribed > > partition > > > > offsets to be retained longer? > > > > > > > > > > > > Guozhang > > > > > > > > > > > > On Tue, Jul 23, 2019 at 10:11 AM Jason Gustafson <ja...@confluent.io > > > > > > wrote: > > > > > > > > > Hi All, > > > > > > > > > > I have a short KIP to add an api for consumer offset deletion: > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-496%3A+Administrative+API+to+delete+consumer+offsets > > > > > . > > > > > Please take a look and let me know what you think. > > > > > > > > > > Thanks, > > > > > Jason > > > > > > > > > > > > > > > > > -- > > > > -- Guozhang > > > > > > > > > > > > > -- > > -- Guozhang > > > -- -- Guozhang
