Hi Stan, That's an interesting thought. I'm wondering if it would be better to leave the current replication metrics counting for the total replication traffic and add a new metric for reassignment traffic?
By the way, a further KIP-455 follow-up that I won't attempt here would be to have a separate throttle for reassignment traffic. -Jason On Thu, Aug 8, 2019 at 11:34 AM Stanislav Kozlovski <stanis...@confluent.io> wrote: > Hi Jason, > > I like the new ReassigningPartitions metric. Would it be easy to expand the > scope of the KIP to split the ReplicationIncoming/Outgoing metric to > distringuish between reassigning/non-reassigning traffic, or do you prefer > to keep this KIP nice and small? > > On Thu, Aug 8, 2019 at 12:08 AM Jason Gustafson <ja...@confluent.io> > wrote: > > > Hi All, > > > > Since KIP-455 is passed, I would like to revive this proposal. I have > > reduced the scope so that it is just changing the way we compute URP and > > adding a new metric for the number of reassigning partitions. Please > take a > > look: > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-352%3A+Distinguish+URPs+caused+by+reassignment > > . > > > > Thanks, > > Jason > > > > On Sun, Aug 5, 2018 at 3:41 PM Dong Lin <lindon...@gmail.com> wrote: > > > > > Hey Jason, > > > > > > Yes, I also think the right solution is probably to include the > > > expected_replication_factor in the per-topic znode and include this > > > information in the UpdateMetadataRequest. > > > > > > And I agree with Gwen and Ismael that it is reasonable to redefine URP > as > > > those partitions whose ISR set size < expected replication factor. > Given > > > that URP is being used for monitoring cluster availability and and > > > reassignment progress, it seems that one of them will break depending > on > > > the URP definition. It seems better to keep the legitimate use-case, > i.e. > > > monitoring cluster availability. Users who want to monitor reassignment > > > progress probably should use the "kafka-reassign-partitions.sh > --verify". > > > > > > > > > Thanks, > > > Dong > > > > > > > > > > > > On Fri, Aug 3, 2018 at 2:57 PM, Jason Gustafson <ja...@confluent.io> > > > wrote: > > > > > > > @Dong > > > > > > > > Ugh, you are right. This is indeed trickier than I imagined. You > could > > > > argue that the problem here is that there is no single source of > truth > > > for > > > > the expected replication factor. While a reassignment is in progress, > > the > > > > reassignment itself has the expected replica set. Otherwise, it is > > stored > > > > partition metadata itself. This is why manually deleting the > > reassignment > > > > is problematic. We lose the expected replica set and we depend on > users > > > to > > > > reinstall it. I guess I'm starting to think that the way we track > > > > reassignment state in the controller is problematic. In addition to > the > > > > problems caused by deletion, we cannot easily change an existing > > > > reassignment. > > > > > > > > High level what I'm thinking is that we need to move the pending > > > > reassignment state out of the single znode and into the individual > > > metadata > > > > of the reassigned partitions so that there is a single source of > truth > > > for > > > > the expected replicas (and hence the replication factor). This would > > also > > > > give us an easier mechanism to manage the batching of multiple > > > > reassignments. Let me think on it a bit and see if I can come up > with a > > > > proposal. > > > > > > > > @Gwen, @Ismael > > > > > > > > That is fair. I also prefer to redefine URP if we think the > > compatibility > > > > impact is a lower concern than continued misuse. > > > > > > > > -Jason > > > > > > > > > > > > > > > > On Fri, Aug 3, 2018 at 12:25 PM, Ismael Juma <ism...@juma.me.uk> > > wrote: > > > > > > > > > RIght, that was my thinking too. > > > > > > > > > > Ismael > > > > > > > > > > On Fri, Aug 3, 2018 at 12:04 PM Gwen Shapira <g...@confluent.io> > > > wrote: > > > > > > > > > > > On Fri, Aug 3, 2018 at 11:23 AM, Jason Gustafson < > > ja...@confluent.io > > > > > > > > > > wrote: > > > > > > > > > > > > > Hey Ismael, > > > > > > > > > > > > > > Yeah, my initial inclination was to redefine URP as well. My > only > > > > doubt > > > > > > was > > > > > > > how it would affect existing tools which might depend on URPs > to > > > > track > > > > > > the > > > > > > > progress of a reassignment. I decided to be conservative in the > > > end, > > > > > but > > > > > > > I'd reconsider if we think it is not a major concern. It is > > > annoying > > > > to > > > > > > > need a new category. > > > > > > > > > > > > > > > > > > > There are existing tools that use URP to track reassignment, but > > > there > > > > > are > > > > > > many more tools that use URP for monitoring and alerting. If I > > > > understand > > > > > > Ismael's suggestion correctly, a re-definition will improve the > > > > > reliability > > > > > > of the monitoring tools (since there won't be false alerts in > case > > of > > > > > > re-assignment) without having to switch to a new metric. > > > > > > > > > > > > I think we should choose the proposal that improves the more > common > > > > usage > > > > > > of the metric, in this case, failure monitoring rather than > > > > reassignment. > > > > > > > > > > > > > > > > > > > > > > > > > > About your question about storage in ZK, I can't think of > > anything > > > > > > > additional that we need. Probably the main difficulty is > getting > > > > access > > > > > > to > > > > > > > the replication factor in the topic utility. My basic thought > was > > > > just > > > > > to > > > > > > > collect the URPs (as we know them today) and use the config API > > to > > > > > > > partition them based on the replication factor. Do you see any > > > > problems > > > > > > > with this? > > > > > > > > > > > > > > -Jason > > > > > > > > > > > > > > > > > > > > > On Thu, Aug 2, 2018 at 12:14 PM, Ismael Juma < > ism...@juma.me.uk> > > > > > wrote: > > > > > > > > > > > > > > > Thanks Jason. This is definitely a pain point. I actually > > prefer > > > > the > > > > > > > option > > > > > > > > to redefine what under-replicated means (currently under > > rejected > > > > > > > > alternatives). Also, do we need to make changes to what we > > store > > > in > > > > > ZK? > > > > > > > If > > > > > > > > so, that should be in the KIP too. > > > > > > > > > > > > > > > > Ismael > > > > > > > > > > > > > > > > On Thu, Aug 2, 2018 at 11:45 AM Jason Gustafson < > > > > ja...@confluent.io> > > > > > > > > wrote: > > > > > > > > > > > > > > > > > Hey All, > > > > > > > > > > > > > > > > > > Another day, another KIP. This one is hopefully > > > straightforward: > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-352% > > > > > > > > 3A+Distinguish+URPs+caused+by+reassignment > > > > > > > > > . > > > > > > > > > Have a look and let me know what you think! > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > Jason > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > *Gwen Shapira* > > > > > > Product Manager | Confluent > > > > > > 650.450.2760 | @gwenshap > > > > > > Follow us: Twitter <https://twitter.com/ConfluentInc> | blog > > > > > > <http://www.confluent.io/blog> > > > > > > > > > > > > > > > > > > > > > > > -- > Best, > Stanislav >
