From dev-return-115564-archive-asf-public=cust-asf.ponee.io@kafka.apache.org Thu Jun 18 17:21:26 2020 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id 7137218065B for ; Thu, 18 Jun 2020 19:21:26 +0200 (CEST) Received: (qmail 47183 invoked by uid 500); 18 Jun 2020 17:21:25 -0000 Mailing-List: contact dev-help@kafka.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@kafka.apache.org Delivered-To: mailing list dev@kafka.apache.org Received: (qmail 47147 invoked by uid 99); 18 Jun 2020 17:21:24 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd3-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 18 Jun 2020 17:21:24 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd3-us-west.apache.org (ASF Mail Server at spamd3-us-west.apache.org) with ESMTP id D3B14180665 for ; Thu, 18 Jun 2020 17:21:23 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd3-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 1.21 X-Spam-Level: * X-Spam-Status: No, score=1.21 tagged_above=-999 required=6.31 tests=[HEADER_FROM_DIFFERENT_DOMAINS=0.25, HTML_MESSAGE=0.2, KAM_DMARC_STATUS=0.01, KAM_INFOUSMEBIZ=0.75, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=disabled Received: from mx1-he-de.apache.org ([10.40.0.8]) by localhost (spamd3-us-west.apache.org [10.40.0.10]) (amavisd-new, port 10024) with ESMTP id zYF1WcrLT8_i for ; Thu, 18 Jun 2020 17:21:21 +0000 (UTC) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=209.85.222.172; helo=mail-qk1-f172.google.com; envelope-from=ismaelj@gmail.com; receiver= Received: from mail-qk1-f172.google.com (mail-qk1-f172.google.com [209.85.222.172]) by mx1-he-de.apache.org (ASF Mail Server at mx1-he-de.apache.org) with ESMTPS id 858397D3FB for ; Thu, 18 Jun 2020 17:21:20 +0000 (UTC) Received: by mail-qk1-f172.google.com with SMTP id q2so6341169qkb.2 for ; Thu, 18 Jun 2020 10:21:20 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=ut1PZW1U6rvVRHfHLcOyePEMhqDuIg2g0bgJ7rXQNdA=; b=FpiXo8yuMFM2l7u2DazS5Ts+pquEiWtVlvB2k54lvHk6v6szNGZlUPL/TOpQQxScB4 /5zIZu5HVTgb40ZrfxpuOXeUl6I2Fi/Yg1r/UVv+crBZQ9J2E2LjlFTCIwUnRBFkrEYW krZaYnZ28KXIZs1PNIto2KVWmLgn/udBnG/vIW/UHgu/J/OuW9OPHm8KCQPcQO95Tf8u 37tB8Vgv0ck4bqTh2o/HTiCqqWsfTteX1WSKa0Z77iVgVtaN1j7r0f6rnZDCDQ4pzHlt SxI+SKtJjYnCpLNPdcvEL4r4roOYU9ZyLRzq3s4qqSqYd46ZeokmlVhfrvQ9x1XmGT+c YyJg== X-Gm-Message-State: AOAM531WIJY1C3Zxe5eKsq7nAQcwfzA3Z4TM8KDQJ7US5ERvgc87EVeQ tgiY39X3YHoZDmma2UrwXyfihSVBDk5m+sw0sgCj5fGZug8= X-Google-Smtp-Source: ABdhPJxfoVQHqS0H6Jr9Qyghwdy0d8/Yyq/PSUUMAWIwUepSGq8vwEARo4FySvp85+vgzbaBK9GRc5D5NOoFPeq8F1c= X-Received: by 2002:a05:620a:3cc:: with SMTP id r12mr5000193qkm.44.1592500872527; Thu, 18 Jun 2020 10:21:12 -0700 (PDT) MIME-Version: 1.0 References: <6b77e37e-2f0d-405a-a98c-a65ece382b23@www.fastmail.com> In-Reply-To: From: Ismael Juma Date: Thu, 18 Jun 2020 10:20:36 -0700 Message-ID: Subject: Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller To: dev Content-Type: multipart/alternative; boundary="000000000000cfc3d105a85f00a9" --000000000000cfc3d105a85f00a9 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Boyang, Sorry for being late on this. I'm generally in favor of the KIP, but it seems like it has two missing things: 1. It doesn't cover how this new channel will be configured. That's a critical part of having a KIP that comprises all that is needed to merge the relevant PR. 2. It doesn't state the compatibility impact of enforcing the create topics policy on auto topic creation. Furthermore, it doesn't explain the error code that will be returned if the topic policy fails. Older clients won't expect a PolicyValidationError during metadata requests, so we need to discuss the approach there and what will happen for newer clients. Ismael On Thu, Jun 18, 2020 at 7:57 AM Boyang Chen wrote: > Thanks everyone for the vote! We already got 3 binding votes (Colin, > Guozhang, Rajini) and 2 non-binding votes (Jose, David). The KIP is now > approved. > > Best, > Boyang > > On Thu, Jun 18, 2020 at 1:43 AM Rajini Sivaram > wrote: > > > +1 (binding) > > > > Thanks for the KIP, Boyang! > > > > Regards, > > > > Rajini > > > > > > > > > > On Thu, Jun 18, 2020 at 8:15 AM David Jacot wrote= : > > > > > +1 (non-binding) > > > > > > Thanks for the KIP! > > > > > > On Thu, Jun 18, 2020 at 12:00 AM Jose Garcia Sancio < > > jsancio@confluent.io> > > > wrote: > > > > > > > +1. > > > > > > > > Thanks for the KIP and looking forward to the improvement > > implementation. > > > > > > > > On Wed, Jun 17, 2020 at 2:24 PM Guozhang Wang > > > wrote: > > > > > > > > > > Thanks for the KIP Boyang, +1 from me. > > > > > > > > > > > > > > > Guozhang > > > > > > > > > > On Wed, Jun 17, 2020 at 1:40 PM Colin McCabe > > > wrote: > > > > > > > > > > > Thanks, Boyang! +1 (binding) > > > > > > > > > > > > best, > > > > > > Colin > > > > > > > > > > > > On Mon, Jun 15, 2020, at 12:59, Boyang Chen wrote: > > > > > > > Thanks for more feedback Colin! I have addressed them in the > KIP. > > > > > > > > > > > > > > Boyang > > > > > > > > > > > > > > On Mon, Jun 15, 2020 at 11:29 AM Colin McCabe < > > cmccabe@apache.org> > > > > > > wrote: > > > > > > > > > > > > > > > On Fri, Jun 12, 2020, at 15:30, Boyang Chen wrote: > > > > > > > > > Thanks Colin for the suggestions! > > > > > > > > > > > > > > > > > > On Fri, Jun 12, 2020 at 2:40 PM Colin McCabe < > > > cmccabe@apache.org > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > Hi Boyang, > > > > > > > > > > > > > > > > > > > > Thanks for the KIP! I think it's getting close. > > > > > > > > > > > > > > > > > > > > > For older requests that need redirection, forwarding > > > > > > > > > > > broker will just use its own authorizer to verify th= e > > > > > > principals. > > > > > > > > When > > > > > > > > > > the > > > > > > > > > > > request looks good, it will just forward the request > > with > > > > its > > > > > > own > > > > > > > > > > > credentials, no second validation needed > > > > > > > > > > > > > > > > > > > > Just to be clear, the controller will still validate th= e > > > > request, > > > > > > > > right? > > > > > > > > > > But at that point the principal will be the broker > > principal. > > > > It > > > > > > > > would be > > > > > > > > > > good to note that here. > > > > > > > > > > > > > > > > > > > > Sounds good, cleared in the KIP. > > > > > > > > > > > > > > > > > > > Internal CreateTopicsRequest Routing > > > > > > > > > > > > > > > > > > > > The forwarding broker is sending the request as the > latest > > > > version, > > > > > > > > > > right? It would be good to add a note of this. This > also > > > > prevents > > > > > > > > routing > > > > > > > > > > loops since the latest version is not forwardable > (another > > > good > > > > > > thing > > > > > > > > to > > > > > > > > > > add, I think...) > > > > > > > > > > > > > > > > > > > We are not bumping the CreateTopic RPC here, so it should > be > > > the > > > > > > latest > > > > > > > > > by default. > > > > > > > > > > > > > > > > > > > > > > > > > Sorry, CreateTopics was a bad example here, since it alread= y > > must > > > > be > > > > > > sent > > > > > > > > to the controller. Oops. > > > > > > > > > > > > > > > > > > > > > > > > > > And just to be clear, we are not "forwarding" but actuall= y > > > > > > > > > sending a CreateTopicRequest from the receiving broker to > the > > > > > > controller > > > > > > > > > broker. > > > > > > > > > > > > > > > > > > > > > > > > > Right. I think we agree on this point. But we do need a > term > > to > > > > > > describe > > > > > > > > the broker which initially receives the user request and > > resends > > > > it to > > > > > > the > > > > > > > > controller. Resending broker? > > > > > > > > > > > > > > > > And I do think it's important to note that the request we > send > > to > > > > the > > > > > > > > controller can't be itself resent. > > > > > > > > > > > > > > > > > > > > > > > > > > > As we discussed in the request routing section, to wor= k > > with > > > > an > > > > > > older > > > > > > > > > > > client, the first contacted broker need to act as a > > proxy > > > to > > > > > > > > redirect > > > > > > > > > > the > > > > > > > > > > > write request to the controller. To support the prox= y > of > > > > > > requests, > > > > > > > > we > > > > > > > > > > need > > > > > > > > > > > to build a channel for brokers to talk directly to t= he > > > > > > controller. > > > > > > > > This > > > > > > > > > > > part of the design is internal change only and won= =E2=80=99t > > block > > > > the > > > > > > KIP > > > > > > > > > > > progress. > > > > > > > > > > > > > > > > > > > > I think it's good to note that we eventually want a > > separate > > > > > > controller > > > > > > > > > > endpoint in KIP-500. However, we don't need it to > > implement > > > > > > KIP-590, > > > > > > > > > > right? The other brokers could forward to the existing > > > > internal > > > > > > > > endpoint > > > > > > > > > > for the controller. So maybe it's best to discuss the > > > separate > > > > > > > > endpoint in > > > > > > > > > > "future work" rather than here. > > > > > > > > > > > > > > > > > > > > I moved the new endpoint part towards the future work a= nd > > > > > > addressed the > > > > > > > > > > usage of controller internal endpoint for routing > requests. > > > > > > > > > > > > > > > > > > > > > > > > > Thanks. > > > > > > > > > > > > > > > > > > > > > > > > > > > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D Start O= ld Proposal =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > > > > > > > > > > > > > > > > > > > I'm glad the old proposal shows up here, but I think th= is > > is > > > > too > > > > > > much > > > > > > > > > > detail. It would be better to just have a one or two > > > paragraph > > > > > > > > summary of > > > > > > > > > > the main points. As it is, the old proposal takes up 4= 0% > > of > > > > the > > > > > > doc > > > > > > > > which > > > > > > > > > > is pretty confusing for someone reading through. Let's > > also > > > > not > > > > > > forget > > > > > > > > > > that someone can just read the old version by using the > > "page > > > > > > history" > > > > > > > > > > function on the wiki. So there's no need to keep that > all > > > > here. > > > > > > > > > > > > > > > > > > > > Make sense, removed. > > > > > > > > > > > > > > > > > > > > > > > > > Thanks again. > > > > > > > > > > > > > > > > > > > > > > > > > > { "name": "PrincipalName", "type": "string", "tag": 0, > > > > > > > > "taggedVersions": "2+", "ignorable": true, > > > > > > > > > "about": "Optional value of the principal name when > the > > > > request > > > > > > is > > > > > > > > redirected by a broker." }, > > > > > > > > > > > > > > > > > > > > > > > > > Maybe "InitialPrincipalName" would be better here? > > PrincipalName > > > > is a > > > > > > bit > > > > > > > > confusing since the message already has a principal name, > after > > > > all... > > > > > > > > > > > > > > > > cheers, > > > > > > > > Colin > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > -- Guozhang > > > > > > > > > > > > > > > > -- > > > > -Jose > > > > > > > > > > --000000000000cfc3d105a85f00a9--