From dev-return-92954-archive-asf-public=cust-asf.ponee.io@kafka.apache.org Tue Apr 3 22:59:32 2018 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 [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id 67C6818064D for ; Tue, 3 Apr 2018 22:59:31 +0200 (CEST) Received: (qmail 1559 invoked by uid 500); 3 Apr 2018 20:59: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 1543 invoked by uid 99); 3 Apr 2018 20:59: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; Tue, 03 Apr 2018 20:59: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 48AE21813DE for ; Tue, 3 Apr 2018 20:59:24 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd3-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 1.879 X-Spam-Level: * X-Spam-Status: No, score=1.879 tagged_above=-999 required=6.31 tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, HTML_MESSAGE=2, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, SPF_PASS=-0.001] autolearn=disabled Authentication-Results: spamd3-us-west.apache.org (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com Received: from mx1-lw-us.apache.org ([10.40.0.8]) by localhost (spamd3-us-west.apache.org [10.40.0.10]) (amavisd-new, port 10024) with ESMTP id p8QZu3U0zLB3 for ; Tue, 3 Apr 2018 20:59:22 +0000 (UTC) Received: from mail-lf0-f45.google.com (mail-lf0-f45.google.com [209.85.215.45]) by mx1-lw-us.apache.org (ASF Mail Server at mx1-lw-us.apache.org) with ESMTPS id 900515F1B4 for ; Tue, 3 Apr 2018 20:59:21 +0000 (UTC) Received: by mail-lf0-f45.google.com with SMTP id z143-v6so22202732lff.3 for ; Tue, 03 Apr 2018 13:59:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to; bh=qtRfVbc4J5MnU25NS3m+m29RnC1IjCbq/LzNg3JycmQ=; b=bZYUsObpo811+MoCAQvZPKyNASAI5DWsuTRWoaBgj76YGToqIhQ7QVaBid3M50hRNO 5UyOWNExopp/jhWt3zZoZNpvambdVYELDuHEZBkX1pW+zRFGMXy+jmPEO4RYwpkc2rIP nkX7oANccMNS4hCGM4ehvQlw82JZFHb5O5QQVtTQGkA9mtIpEBk/ySA+qSd+bA1Z2ai8 m7lFgyJ9WQaiCP9o2P1IQzvBkeRiKT/Z0zYUSQtKMLD6lomdxJTKmgI4E1P3MZX6ZPSd ifgKx8hd5h/KY8jZywoFxmkM0Dbm54Du1Isb8Mr/DTgYfYUBurlUDyECKnDUA7x5snpy 8HCg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to; bh=qtRfVbc4J5MnU25NS3m+m29RnC1IjCbq/LzNg3JycmQ=; b=eLXsJCLNpFJXonIxxgfK26SDsbX9eGvEyW+0+bAjzJ4P8FPHw4vDoPrHpWkzYQyL3/ /yx4Yd/zn0p1luHH1FMuajWlH0uxxdQpR/Gr6qkdOKI59ESELol0hB3+B9e+j4eEAnZq BPIMCTRqU3eoEWnEDZQ22sKqCiWyMhbhPirzEJGKi07IPKj8lCyc7z+1LIGwN4fDaogP HcCiH6Nr+3lCQrzRxJRZAuzTu4AtYm3t43/ziNya3GWNS1iS9smh+X3cM0idAKmiy4/S SWkNXggHfekeUA6XLrotEANR2l2nkpj5yeIN9arK0qoi8bXzZgZiA6+ZnahRTvNPEz2G NORg== X-Gm-Message-State: ALQs6tDf7ELESvPFM6Qr+j9ahLInZzMCi311YjCCV9MmI1oPAyw5iii+ 9TWq4qJQqM3Im2C4/YYcuXUR0SJcWeKyrdsXzH5OVSu7 X-Google-Smtp-Source: AIpwx4/aDag4UEyd2lsyet+AXVvyPbaIhWUiteJlyL5xAPVhToyrtcSQqp3WfDgPtwYTSvJpSCQzO33xfPKrkbKnQZg= X-Received: by 2002:a19:6b09:: with SMTP id d9-v6mr9117688lfa.83.1522789160106; Tue, 03 Apr 2018 13:59:20 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a19:6910:0:0:0:0:0 with HTTP; Tue, 3 Apr 2018 13:59:19 -0700 (PDT) In-Reply-To: References: <1466f50d-f847-d066-fc43-95253131439a@confluent.io> From: Bill Bejeck Date: Tue, 3 Apr 2018 16:59:19 -0400 Message-ID: Subject: Re: [DISCUSS] KIP-276: Add StreamsConfig prefix for different consumers To: dev@kafka.apache.org Content-Type: multipart/alternative; boundary="000000000000f4c0a70568f7fafe" --000000000000f4c0a70568f7fafe Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Boyang, Thanks for making changes to the KIP, I'm +1 on the updated version. -Bill On Tue, Apr 3, 2018 at 1:14 AM, Boyang Chen wrote: > Hey friends, > > > both KIP 276+Add+StreamsConfig+prefix+for+different+consumers> and pull request< > https://github.com/apache/kafka/pull/4805> are updated. Feel free to take > another look. > > > > Thanks for your valuable feedback! > > > Best, > > Boyang > > ________________________________ > From: Boyang Chen > Sent: Tuesday, April 3, 2018 11:39 AM > To: dev@kafka.apache.org > Subject: Re: [DISCUSS] KIP-276: Add StreamsConfig prefix for different > consumers > > Thanks Matthias, Ted and Guozhang for the inputs. I shall address them in > next round. > > > ________________________________ > From: Matthias J. Sax > Sent: Tuesday, April 3, 2018 4:43 AM > To: dev@kafka.apache.org > Subject: Re: [DISCUSS] KIP-276: Add StreamsConfig prefix for different > consumers > > Yes, your examples make sense to me. That was the idea behind the proposa= l. > > > -Matthias > > On 4/2/18 11:25 AM, Guozhang Wang wrote: > > @Matthias > > > > That's a good question: I think adding another config for the main > consumer > > makes good tradeoffs between compatibility and new user convenience. Ju= st > > to clarify, from user's pov on upgrade: > > > > 1) I'm already overriding some consumer configs, and now I want to > override > > these values differently for restore consumers, I'd add one new line fo= r > > the restore consumer prefix. > > > > 2) I'm already overriding some consumer configs, and now I want to NOT > > overriding them for restore consumers, I'd change my override from > > `consumer.X` to `main.consumer.X`. > > > > 3) I'm new and have not any consumer overrides, and now if I want to > > override some, I'd use `main.consumer`, `restore.consumer` for specific > > consumer types, and ONLY consider `consumer` for the ones that I want t= o > > apply universally. > > > > 4) I'm already overriding some consumer configs and I'm happy with what= I > > get, I do not change anything. > > > > > > Guozhang > > > > On Mon, Apr 2, 2018 at 11:10 AM, Ted Yu wrote: > > > >> bq. to introduce one more prefix `main.consumer.` > >> > >> Makes sense. > >> > >> On Mon, Apr 2, 2018 at 11:06 AM, Matthias J. Sax > > >> wrote: > >> > >>> Boyang, > >>> > >>> thanks a lot for the KIP. > >>> > >>> Couple of questions: > >>> > >>>> (MODIFIED) public Map getRestoreConsumerConfigs(fina= l > >>> String clientId); > >>> > >>> You mean that the implementation/semantics of this method changes, bu= t > >>> not the API itself? Might be good to add this as "comment style" > instead > >>> of "(MODIFIED)" prefix. > >>> > >>>> By rewriting the getRestoreConsumerConfigs() function and adding the > >>> getGlobalConsumerConfigs() function, if one user uses > >>> restoreConsumerPrefix() or globalConsumerPrefix() when adding new > >>> configurations, the configs shall overwrite base consumer config. If > not > >>> specified, restore consumer and global consumer shall share the same > >> config > >>> with base consumer. > >>> > >>> While this does make sense for backward compatibility, I am wonder if > it > >>> makes the config "inheritance logic" (ie, hierarchy) too complex? We > >>> basically introduce a second level of overwrites. It might be simpler > to > >>> not introduce this hierarchy with the cost to break backward > >> compatibility. > >>> > >>> For example, config `request.timeout.ms`: > >>> > >>> User sets `request.timeout.ms=3D` > >>> To change it for the main consumer, user also sets > >>> `consumer.request.timeout.ms=3D` > >>> > >>> If user only wants to change the config for main consumer, but not fo= r > >>> global or restore consumer, user needs to add two more configs: > >>> > >>> `restore.consumer.request.timeout.ms=3D` > >>> and > >>> `global.consumer.request.timeout.ms=3D` > >>> > >>> to reset both back to the default config. IMHO, this is not an optima= l > >>> user experience. Thus, it might be worth to change the semantics for > >>> `consumer.` prefix to only apply those configs to the main consumer. > >>> > >>> > >>> Not sure what other think what the better solution is (I am not sure = by > >>> myself to be honest---just wanted to point it out and discuss the > >>> pros/cons for both). > >>> > >>> > >>> Another though would be, to introduce one more prefix `main.consumer.= ` > >>> -- using this, the existing `consumer.` prefix would apply to all > >>> consumers (keeping it's current semantics) while we have overwrites f= or > >>> all three consumers -- this allow to directly set `main.consumer` > >>> instead of `consumer` avoiding the weird pattern from my example abov= e > >>> and preserves backward compatibility. Ie, if we introduce an hierarch= y > >>> of overwrite, a "full" hierarchy might be better than a "partial" > >>> hierarchy. > >>> > >>> > >>> Looking forward to your thoughts. > >>> > >>> > >>> -Matthias > >>> > >>> > >>> On 4/1/18 5:55 PM, Guozhang Wang wrote: > >>>> Thanks for the KIP Boyang, the KIP looks good to me. > >>>> > >>>> For config values, we use underscore for keeping a single word; for > >>> config > >>>> keys, though, we do not use underscores or dashes. I'd suggest using > >> dots > >>>> to be consistent with others. > >>>> > >>>> > >>>> Otherwise I'm +1 on the KIP. > >>>> > >>>> > >>>> Guozhang > >>>> > >>>> > >>>> On Sun, Apr 1, 2018 at 10:56 AM, Ted Yu wrote: > >>>> > >>>>> Looks good overall. > >>>>> > >>>>> public static final String RESTORE_CONSUMER_PREFIX =3D > >>> "restore-consumer."; > >>>>> > >>>>> For other constants in StreamsConfig, underscore is used instead of > >>> dash. > >>>>> > >>>>> Cheers > >>>>> > >>>>> On Sun, Apr 1, 2018 at 9:38 AM, Boyang Chen > >>> wrote: > >>>>> > >>>>>> Hey friends, > >>>>>> > >>>>>> > >>>>>> I would like to start a discussion thread for KIP 276: > >>>>>> > >>>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP- > >>>>>> 276+Add+StreamsConfig+prefix+for+different+consumers > >>>>>> > >>>>>> > >>>>>> And pull request is here: > >>>>>> > >>>>>> https://github.com/apache/kafka/pull/4805 > >>>>>> > >>>>>> [https://avatars3.githubusercontent.com/u/5845561?s=3D400&v=3D4 > >> ] >>>>>> github.com/apache/kafka/pull/4805> > >>>>>> > >>>>>> KAFKA-6657: Add StreamsConfig prefix for different consumers by > >>> abbccdda > >>>>> =C2=B7 > >>>>>> Pull Request #4805 =C2=B7 apache/kafka >>>>>> com/apache/kafka/pull/4805> > >>>>>> github.com > >>>>>> This pull request is for jira 6657. The KIP proposal is here Added > >> unit > >>>>>> tests for new getGlobalConsumerConfigs API and make sure existing > >>> restore > >>>>>> consumer tests are passing. > >>>>>> > >>>>>> > >>>>>> > >>>>>> > >>>>>> > >>>>>> Thanks, > >>>>>> > >>>>>> Boyang > >>>>>> > >>>>> > >>>> > >>>> > >>>> > >>> > >>> > >> > > > > > > > > --000000000000f4c0a70568f7fafe--