From dev-return-113128-archive-asf-public=cust-asf.ponee.io@kafka.apache.org Thu Apr 16 01:32:24 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 42C5E18065C for ; Thu, 16 Apr 2020 03:32:23 +0200 (CEST) Received: (qmail 24421 invoked by uid 500); 16 Apr 2020 01:32:21 -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 24372 invoked by uid 99); 16 Apr 2020 01:32:20 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd4-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 16 Apr 2020 01:32:20 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd4-us-west.apache.org (ASF Mail Server at spamd4-us-west.apache.org) with ESMTP id C5E63C2122 for ; Thu, 16 Apr 2020 01:32:18 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd4-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 0.001 X-Spam-Level: X-Spam-Status: No, score=0.001 tagged_above=-999 required=6.31 tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, HTML_MESSAGE=0.2, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=disabled Authentication-Results: spamd4-us-west.apache.org (amavisd-new); dkim=pass (2048-bit key) header.d=confluent.io Received: from mx1-he-de.apache.org ([10.40.0.8]) by localhost (spamd4-us-west.apache.org [10.40.0.11]) (amavisd-new, port 10024) with ESMTP id ktqhkx_1h1AH for ; Thu, 16 Apr 2020 01:32:11 +0000 (UTC) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=2607:f8b0:4864:20::1044; helo=mail-pj1-x1044.google.com; envelope-from=kprakasam@confluent.io; receiver= Received: from mail-pj1-x1044.google.com (mail-pj1-x1044.google.com [IPv6:2607:f8b0:4864:20::1044]) by mx1-he-de.apache.org (ASF Mail Server at mx1-he-de.apache.org) with ESMTPS id 72C2A7F721 for ; Thu, 16 Apr 2020 01:24:22 +0000 (UTC) Received: by mail-pj1-x1044.google.com with SMTP id cl8so630334pjb.3 for ; Wed, 15 Apr 2020 18:24:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=confluent.io; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=J9DKrsn6lVAkGak4EbU3/NzlSNJcRG9wa3JiaxB8kmU=; b=lLu1W8q7YNJQJfdDlfzarBuq9X0I6a2O1qSCbTionstZjdNrYIynqBbo/4YR2p59VO oRORqvwZxF9GU2COozLZTNmzwpbLveHejE1Z5clcQbF6hrG7c3Y03BDCCp/6zeoSPySH LaO2hAnI2yhjTWYI1FcfydnVMasrGd0SENgxIHGnMhKNA4EQESsm5rMKRXc+rCMCnul6 PvnMvHADW/jLFSdGxuaOF64hkvbyCuKIdMY24ZYxIEJRnwxnGWNZcEojtvxPbK8K6CI4 DXrGfjBw3yBwIuuZqrc4AYE9xJkPKXmAXXaoONeNKozLE4Yyo24POODq6aXxyw7kW9c/ IDCA== 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=J9DKrsn6lVAkGak4EbU3/NzlSNJcRG9wa3JiaxB8kmU=; b=b4jANJIfqbeFaFCnl81ifXmuMgBWcW7CrsiFPjzrTJrtzUjZY/qtZvtkqUodfvbeic GiokZKIw/J61Z0fD4Kx+/r8Nn1P+lTgABOBE8PFHZFmEO1ygd28a2x7mviJ5yD5tO8CR lo/iKWtqT3uSBxuuoddM6rgn27LUzNr1ZW/ZJ010mKpHwo/nXVBUDMXzreWC5mhYKMxm DGH/mpl6byfO9r5jZ7Dv/4/1q0FakE1vQ01E/VMskMthVPl1xYN6AnOSGO0HL9pZel4r feEknogojU+7f2gEwVX/U6S68jZ+VNYYL+xp5lrzbw5WVH6oP8oT+sOQMbauS9K3Q1OT 2Vow== X-Gm-Message-State: AGi0PuZvJuLMsbejC2DbSa3rH/NMY7k0UWek9g0WgD9bE+lvg5hr2KdH Ql3Z+TZFwKlMoFHPOzni7o8BUFKHSi+kp9s2rOk4zI3oBUs= X-Google-Smtp-Source: APiQypK2tTzz2r/QmyQaiOy0dBXcYlOeTGgOrJGuNcuBpueM5vxUdA8pvTjGjCFK+q2/gMgr6Yw7AmZJSGPUICqGKJg= X-Received: by 2002:a17:902:8ec7:: with SMTP id x7mr7583030plo.3.1587000253869; Wed, 15 Apr 2020 18:24:13 -0700 (PDT) MIME-Version: 1.0 References: <68a04d9b-51c2-4525-8191-cce818a9b7f0@www.fastmail.com> <0741f127-2922-4efb-92e0-a4049fe725cd@www.fastmail.com> In-Reply-To: From: Kowshik Prakasam Date: Wed, 15 Apr 2020 18:24:02 -0700 Message-ID: Subject: Re: [DISCUSS] KIP-584: Versioning scheme for features To: dev@kafka.apache.org Content-Type: multipart/alternative; boundary="00000000000064211505a35e4a79" --00000000000064211505a35e4a79 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Jun, Thanks for the feedback! I have addressed the comments in the KIP. > 200. In the validation section, there is still the text "*from* > {"max_version_level": > X} *to* {"max_version_level": X=E2=80=99}". It seems that it should say "= from X to > Y"? (Kowshik): Done. I have reworded it a bit to make it clearer now in this section: https://cwiki.apache.org/confluence/display/KAFKA/KIP-584%3A+Versioning+sch= eme+for+features#KIP-584:Versioningschemeforfeatures-Validations > 110. Could we add that we need to document the bumped version of each > feature in the upgrade section of a release? (Kowshik): Great point! Done, I have mentioned it in #3 this section: https://cwiki.apache.org/confluence/display/KAFKA/KIP-584 %3A+Versioning+scheme+for+features#KIP-584 :Versioningschemeforfeatures-Whentouseversionedfeatureflags? Cheers, Kowshik On Wed, Apr 15, 2020 at 4:00 PM Jun Rao wrote: > Hi, Kowshik, > > Looks good to me now. Just a couple of minor things below. > > 200. In the validation section, there is still the text "*from* > {"max_version_level": > X} *to* {"max_version_level": X=E2=80=99}". It seems that it should say "= from X to > Y"? > > 110. Could we add that we need to document the bumped version of each > feature in the upgrade section of a release? > > Thanks, > > Jun > > On Wed, Apr 15, 2020 at 1:08 PM Kowshik Prakasam > wrote: > > > Hi Jun, > > > > Thank you for the suggestion! I have updated the KIP, please find my > > response below. > > > > > 200. I guess you are saying only when the allowDowngrade field is set= , > > the > > > finalized feature version can go backward. Otherwise, it can only go > up. > > > That makes sense. It would be useful to make that clear when explaini= ng > > > the usage of the allowDowngrade field. In the validation section, we > > have " > > > /features' from {"max_version_level": X} to {"max_version_level": X= =E2=80=99}", > > it > > > seems that we need to mention Y there. > > > > (Kowshik): Great point! Yes, that is correct. Done, I have updated the > > validations > > section explaining the above. Here is a link to this section: > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-584%3A+Versioning+s= cheme+for+features#KIP-584:Versioningschemeforfeatures-Validations > > > > > > Cheers, > > Kowshik > > > > > > > > > > On Wed, Apr 15, 2020 at 11:05 AM Jun Rao wrote: > > > > > Hi, Kowshik, > > > > > > 200. I guess you are saying only when the allowDowngrade field is set= , > > the > > > finalized feature version can go backward. Otherwise, it can only go > up. > > > That makes sense. It would be useful to make that clear when explaini= ng > > > the usage of the allowDowngrade field. In the validation section, we > have > > > " > > > /features' from {"max_version_level": X} to {"max_version_level": X= =E2=80=99}", > > it > > > seems that we need to mention Y there. > > > > > > Thanks, > > > > > > Jun > > > > > > On Wed, Apr 15, 2020 at 10:44 AM Kowshik Prakasam < > > kprakasam@confluent.io> > > > wrote: > > > > > > > Hi Jun, > > > > > > > > Great question! Please find my response below. > > > > > > > > > 200. My understanding is that If the CLI tool passes the > > > > > '--allow-downgrade' flag when updating a specific feature, then a > > > future > > > > > downgrade is possible. Otherwise, the feature is now downgradable= . > If > > > so, > > > > I > > > > > was wondering how the controller remembers this since it can be > > > restarted > > > > > over time? > > > > > > > > (Kowshik): The purpose of the flag was to just restrict the user > intent > > > for > > > > a specific request. > > > > It seems to me that to avoid confusion, I could call the flag as > > > > `--try-downgrade` instead. > > > > Then this makes it clear, that, the controller just has to consider > the > > > ask > > > > from > > > > the user as an explicit request to attempt a downgrade. > > > > > > > > The flag does not act as an override on controller's decision makin= g > > that > > > > decides whether > > > > a flag is downgradable (these decisions on whether to allow a flag = to > > be > > > > downgraded > > > > from a specific version level, can be embedded in the controller > code). > > > > > > > > Please let me know what you think. > > > > Sorry if I misunderstood the original question. > > > > > > > > > > > > Cheers, > > > > Kowshik > > > > > > > > > > > > On Wed, Apr 15, 2020 at 9:40 AM Jun Rao wrote: > > > > > > > > > Hi, Kowshik, > > > > > > > > > > Thanks for the reply. Makes sense. Just one more question. > > > > > > > > > > 200. My understanding is that If the CLI tool passes the > > > > > '--allow-downgrade' flag when updating a specific feature, then a > > > future > > > > > downgrade is possible. Otherwise, the feature is now downgradable= . > If > > > > so, I > > > > > was wondering how the controller remembers this since it can be > > > restarted > > > > > over time? > > > > > > > > > > Jun > > > > > > > > > > > > > > > On Tue, Apr 14, 2020 at 6:49 PM Kowshik Prakasam < > > > kprakasam@confluent.io > > > > > > > > > > wrote: > > > > > > > > > > > Hi Jun, > > > > > > > > > > > > Thanks a lot for the feedback and the questions! > > > > > > Please find my response below. > > > > > > > > > > > > > 200. The UpdateFeaturesRequest includes an AllowDowngrade > field. > > It > > > > > seems > > > > > > > that field needs to be persisted somewhere in ZK? > > > > > > > > > > > > (Kowshik): Great question! Below is my explanation. Please help > me > > > > > > understand, > > > > > > if you feel there are cases where we would need to still persis= t > it > > > in > > > > > ZK. > > > > > > > > > > > > Firstly I have updated my thoughts into the KIP now, under the > > > > > 'guidelines' > > > > > > section: > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-584%3A+Versioning+s= cheme+for+features#KIP-584:Versioningschemeforfeatures-Guidelinesonfeaturev= ersionsandworkflows > > > > > > > > > > > > The allowDowngrade boolean field is just to restrict the user > > intent, > > > > and > > > > > > to remind > > > > > > them to double check their intent before proceeding. It should = be > > set > > > > to > > > > > > true > > > > > > by the user in a request, only when the user intent is to > > forcefully > > > > > > "attempt" a > > > > > > downgrade of a specific feature's max version level, to the > > provided > > > > > value > > > > > > in > > > > > > the request. > > > > > > > > > > > > We can extend this safeguard. The controller (on it's end) can > > > maintain > > > > > > rules in the code, that, for safety reasons would outright reje= ct > > > > certain > > > > > > downgrades > > > > > > from a specific max_version_level for a specific feature. Such > > > > rejections > > > > > > may > > > > > > happen depending on the feature being downgraded, and from what > > > version > > > > > > level. > > > > > > > > > > > > The CLI tool only allows a downgrade attempt in conjunction wit= h > > > > specific > > > > > > flags and sub-commands. For example, in the CLI tool, if the us= er > > > uses > > > > > the > > > > > > 'downgrade-all' command, or passes '--allow-downgrade' flag whe= n > > > > > updating a > > > > > > specific feature, only then the tool will translate this ask to > > > setting > > > > > > 'allowDowngrade' field in the request to the server. > > > > > > > > > > > > > 201. UpdateFeaturesResponse has the following top level field= s. > > > > Should > > > > > > > those fields be per feature? > > > > > > > > > > > > > > "fields": [ > > > > > > > { "name": "ErrorCode", "type": "int16", "versions": "0+", > > > > > > > "about": "The error code, or 0 if there was no error." = }, > > > > > > > { "name": "ErrorMessage", "type": "string", "versions": > "0+", > > > > > > > "about": "The error message, or null if there was no > > error." > > > } > > > > > > > ] > > > > > > > > > > > > (Kowshik): Great question! > > > > > > As such, the API is transactional, as explained in the sections > > > linked > > > > > > below. > > > > > > Either all provided FeatureUpdate was applied, or none. > > > > > > It's the reason I felt we can have just one error code + messag= e. > > > > > > Happy to extend this if you feel otherwise. Please let me know. > > > > > > > > > > > > Link to sections: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-584%3A+Versioning+s= cheme+for+features#KIP-584:Versioningschemeforfeatures-ChangestoKafkaContro= ller > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-584%3A+Versioning+s= cheme+for+features#KIP-584:Versioningschemeforfeatures-Guarantees > > > > > > > > > > > > > 202. The /features path in ZK has a field min_version_level. > > Which > > > > API > > > > > > and > > > > > > > tool can change that value? > > > > > > > > > > > > (Kowshik): Great question! Currently this cannot be modified by > > using > > > > the > > > > > > API or the tool. > > > > > > Feature version deprecation (by raising min_version_level) can = be > > > done > > > > > only > > > > > > by the Controller directly. The rationale is explained in this > > > section: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-584%3A+Versioning+s= cheme+for+features#KIP-584:Versioningschemeforfeatures-Featureversiondeprec= ation > > > > > > > > > > > > > > > > > > Cheers, > > > > > > Kowshik > > > > > > > > > > > > On Tue, Apr 14, 2020 at 5:33 PM Jun Rao > wrote: > > > > > > > > > > > > > Hi, Kowshik, > > > > > > > > > > > > > > Thanks for addressing those comments. Just a few more minor > > > comments. > > > > > > > > > > > > > > 200. The UpdateFeaturesRequest includes an AllowDowngrade > field. > > It > > > > > seems > > > > > > > that field needs to be persisted somewhere in ZK? > > > > > > > > > > > > > > 201. UpdateFeaturesResponse has the following top level field= s. > > > > Should > > > > > > > those fields be per feature? > > > > > > > > > > > > > > "fields": [ > > > > > > > { "name": "ErrorCode", "type": "int16", "versions": "0+", > > > > > > > "about": "The error code, or 0 if there was no error." = }, > > > > > > > { "name": "ErrorMessage", "type": "string", "versions": > "0+", > > > > > > > "about": "The error message, or null if there was no > > error." > > > } > > > > > > > ] > > > > > > > > > > > > > > 202. The /features path in ZK has a field min_version_level. > > Which > > > > API > > > > > > and > > > > > > > tool can change that value? > > > > > > > > > > > > > > Jun > > > > > > > > > > > > > > On Mon, Apr 13, 2020 at 5:12 PM Kowshik Prakasam < > > > > > kprakasam@confluent.io > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > Hi Jun, > > > > > > > > > > > > > > > > Thanks for the feedback! I have updated the KIP-584 > addressing > > > your > > > > > > > > comments. > > > > > > > > Please find my response below. > > > > > > > > > > > > > > > > > 100.6 You can look for the sentence "This operation > requires > > > > ALTER > > > > > on > > > > > > > > > CLUSTER." in KIP-455. Also, you can check its usage in > > > > > > > > > KafkaApis.authorize(). > > > > > > > > > > > > > > > > (Kowshik): Done. Great point! For the newly introduced > > > > > UPDATE_FEATURES > > > > > > > api, > > > > > > > > I have added a > > > > > > > > requirement that AclOperation.ALTER is required on > > > > > > ResourceType.CLUSTER. > > > > > > > > > > > > > > > > > 110. Keeping the feature version as int is probably fine.= I > > > just > > > > > felt > > > > > > > > that > > > > > > > > > for some of the common user interactions, it's more > > convenient > > > to > > > > > > > > > relate that to a release version. For example, if a user > > wants > > > to > > > > > > > > downgrade > > > > > > > > > to a release 2.5, it's easier for the user to use the too= l > > like > > > > > "tool > > > > > > > > > --downgrade 2.5" instead of "tool --downgrade --feature X > > > > --version > > > > > > 6". > > > > > > > > > > > > > > > > (Kowshik): Great point. Generally, maximum feature version > > levels > > > > are > > > > > > not > > > > > > > > downgradable after > > > > > > > > they are finalized in the cluster. This is because, as a > > > guideline > > > > > > > bumping > > > > > > > > feature version level usually is used mainly to convey > > important > > > > > > breaking > > > > > > > > changes. > > > > > > > > Despite the above, there may be some extreme/rare cases > where a > > > > user > > > > > > > wants > > > > > > > > to downgrade > > > > > > > > all features to a specific previous release. The user may > want > > to > > > > do > > > > > > this > > > > > > > > just > > > > > > > > prior to rolling back a Kafka cluster to a previous release= . > > > > > > > > > > > > > > > > To support the above, I have made a change to the KIP > > explaining > > > > that > > > > > > the > > > > > > > > CLI tool is versioned. > > > > > > > > The CLI tool internally has knowledge about a map of featur= es > > to > > > > > their > > > > > > > > respective max > > > > > > > > versions supported by the Broker. The tool's knowledge of > > > features > > > > > and > > > > > > > > their version values, > > > > > > > > is limited to the version of the CLI tool itself i.e. the > > > > information > > > > > > is > > > > > > > > packaged into the CLI tool > > > > > > > > when it is released. Whenever a Kafka release introduces a > new > > > > > feature > > > > > > > > version, or modifies > > > > > > > > an existing feature version, the CLI tool shall also be > updated > > > > with > > > > > > this > > > > > > > > information, > > > > > > > > Newer versions of the CLI tool will be released as part of > the > > > > Kafka > > > > > > > > releases. > > > > > > > > > > > > > > > > Therefore, to achieve the downgrade need, the user just nee= ds > > to > > > > run > > > > > > the > > > > > > > > version of > > > > > > > > the CLI tool that's part of the particular previous release > > that > > > > > he/she > > > > > > > is > > > > > > > > downgrading to. > > > > > > > > To help the user with this, there is a new command added to > the > > > CLI > > > > > > tool > > > > > > > > called `downgrade-all`. > > > > > > > > This essentially downgrades max version levels of all > features > > in > > > > the > > > > > > > > cluster to the versions > > > > > > > > known to the CLI tool internally. > > > > > > > > > > > > > > > > I have explained the above in the KIP under these sections: > > > > > > > > > > > > > > > > Tooling support (have explained that the CLI tool is > > versioned): > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-584%3A+Versioning+s= cheme+for+features#KIP-584:Versioningschemeforfeatures-Toolingsupport > > > > > > > > > > > > > > > > Regular CLI tool usage (please refer to point #3, and see t= he > > > > tooling > > > > > > > > example) > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-584%3A+Versioning+s= cheme+for+features#KIP-584:Versioningschemeforfeatures-RegularCLItoolusage > > > > > > > > > > > > > > > > > 110. Similarly, if the client library finds a feature > > mismatch > > > > with > > > > > > the > > > > > > > > broker, > > > > > > > > > the client likely needs to log some error message for the > > user > > > to > > > > > > take > > > > > > > > some > > > > > > > > > actions. It's much more actionable if the error message i= s > > > > "upgrade > > > > > > the > > > > > > > > > broker to release version 2.6" than just "upgrade the > broker > > to > > > > > > feature > > > > > > > > > version 7". > > > > > > > > > > > > > > > > (Kowshik): That's a really good point! If we use ints for > > feature > > > > > > > versions, > > > > > > > > the best > > > > > > > > message that client can print for debugging is "broker > doesn't > > > > > support > > > > > > > > feature version 7", and alongside that print the supported > > > version > > > > > > range > > > > > > > > returned > > > > > > > > by the broker. Then, does it sound reasonable that the user > > could > > > > > then > > > > > > > > reference > > > > > > > > Kafka release logs to figure out which version of the broke= r > > > > release > > > > > is > > > > > > > > required > > > > > > > > be deployed, to support feature version 7? I couldn't think > of > > a > > > > > better > > > > > > > > strategy here. > > > > > > > > > > > > > > > > > 120. When should a developer bump up the version of a > > feature? > > > > > > > > > > > > > > > > (Kowshik): Great question! In the KIP, I have added a > section: > > > > > > > 'Guidelines > > > > > > > > on feature versions and workflows' > > > > > > > > providing some guidelines on when to use the versioned > feature > > > > flags, > > > > > > and > > > > > > > > what > > > > > > > > are the regular workflows with the CLI tool. > > > > > > > > > > > > > > > > Link to the relevant sections: > > > > > > > > Guidelines: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-584%3A+Versioning+s= cheme+for+features#KIP-584:Versioningschemeforfeatures-Guidelinesonfeaturev= ersionsandworkflows > > > > > > > > > > > > > > > > Regular CLI tool usage: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-584%3A+Versioning+s= cheme+for+features#KIP-584:Versioningschemeforfeatures-RegularCLItoolusage > > > > > > > > > > > > > > > > Advanced CLI tool usage: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-584%3A+Versioning+s= cheme+for+features#KIP-584:Versioningschemeforfeatures-AdvancedCLItoolusage > > > > > > > > > > > > > > > > > > > > > > > > Cheers, > > > > > > > > Kowshik > > > > > > > > > > > > > > > > > > > > > > > > On Fri, Apr 10, 2020 at 4:25 PM Jun Rao > > > wrote: > > > > > > > > > > > > > > > > > Hi, Kowshik, > > > > > > > > > > > > > > > > > > Thanks for the reply. A few more comments. > > > > > > > > > > > > > > > > > > 110. Keeping the feature version as int is probably fine.= I > > > just > > > > > felt > > > > > > > > that > > > > > > > > > for some of the common user interactions, it's more > > convenient > > > to > > > > > > > > > relate that to a release version. For example, if a user > > wants > > > to > > > > > > > > downgrade > > > > > > > > > to a release 2.5, it's easier for the user to use the too= l > > like > > > > > "tool > > > > > > > > > --downgrade 2.5" instead of "tool --downgrade --feature X > > > > --version > > > > > > 6". > > > > > > > > > Similarly, if the client library finds a feature mismatch > > with > > > > the > > > > > > > > broker, > > > > > > > > > the client likely needs to log some error message for the > > user > > > to > > > > > > take > > > > > > > > some > > > > > > > > > actions. It's much more actionable if the error message i= s > > > > "upgrade > > > > > > the > > > > > > > > > broker to release version 2.6" than just "upgrade the > broker > > to > > > > > > feature > > > > > > > > > version 7". > > > > > > > > > > > > > > > > > > 111. Sounds good. > > > > > > > > > > > > > > > > > > 120. When should a developer bump up the version of a > > feature? > > > > > > > > > > > > > > > > > > Jun > > > > > > > > > > > > > > > > > > On Tue, Apr 7, 2020 at 12:26 AM Kowshik Prakasam < > > > > > > > kprakasam@confluent.io > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > Hi Jun, > > > > > > > > > > > > > > > > > > > > I have updated the KIP for the item 111. > > > > > > > > > > I'm in the process of addressing 100.6, and will provid= e > an > > > > > update > > > > > > > > soon. > > > > > > > > > > I think item 110 is still under discussion given we are > now > > > > > > > providing a > > > > > > > > > way > > > > > > > > > > to finalize > > > > > > > > > > all features to their latest version levels. In any cas= e, > > > > please > > > > > > let > > > > > > > us > > > > > > > > > > know > > > > > > > > > > how you feel in response to Colin's comments on this > topic. > > > > > > > > > > > > > > > > > > > > > 111. To put this in context, when we had IBP, the > default > > > > value > > > > > > is > > > > > > > > the > > > > > > > > > > > current released version. So, if you are a brand new > > user, > > > > you > > > > > > > don't > > > > > > > > > need > > > > > > > > > > > to configure IBP and all new features will be > immediately > > > > > > available > > > > > > > > in > > > > > > > > > > the > > > > > > > > > > > new cluster. If you are upgrading from an old version= , > > you > > > do > > > > > > need > > > > > > > to > > > > > > > > > > > understand and configure IBP. I see a similar pattern > > here > > > > for > > > > > > > > > > > features. From the ease of use perspective, ideally, = we > > > > > shouldn't > > > > > > > > > require > > > > > > > > > > a > > > > > > > > > > > new user to have an extra step such as running a > > bootstrap > > > > > script > > > > > > > > > unless > > > > > > > > > > > it's truly necessary. If someone has a special need > (all > > > the > > > > > > cases > > > > > > > > you > > > > > > > > > > > mentioned seem special cases?), they can configure a > mode > > > > such > > > > > > that > > > > > > > > > > > features are enabled/disabled manually. > > > > > > > > > > > > > > > > > > > > (Kowshik): That makes sense, thanks for the idea! Sorry > if > > I > > > > > didn't > > > > > > > > > > understand > > > > > > > > > > this need earlier. I have updated the KIP with the > approach > > > > that > > > > > > > > whenever > > > > > > > > > > the '/features' node is absent, the controller by defau= lt > > > will > > > > > > > > bootstrap > > > > > > > > > > the node > > > > > > > > > > to contain the latest feature levels. Here is the new > > section > > > > in > > > > > > the > > > > > > > > KIP > > > > > > > > > > describing > > > > > > > > > > the same: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-584%3A+Versioning+s= cheme+for+features#KIP-584:Versioningschemeforfeatures-Controller:ZKnodeboo= tstrapwithdefaultvalues > > > > > > > > > > > > > > > > > > > > Next, as I explained in my response to Colin's > suggestions, > > > we > > > > > are > > > > > > > now > > > > > > > > > > providing a `--finalize-latest-features` flag with the > > > tooling. > > > > > > This > > > > > > > > lets > > > > > > > > > > the sysadmin finalize all features known to the > controller > > to > > > > > their > > > > > > > > > latest > > > > > > > > > > version > > > > > > > > > > levels. Please look at this section (point #3 and the > > tooling > > > > > > example > > > > > > > > > > later): > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-584%3A+Versioning+s= cheme+for+features#KIP-584:Versioningschemeforfeatures-Toolingsupport > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Do you feel this addresses your comment/concern? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Cheers, > > > > > > > > > > Kowshik > > > > > > > > > > > > > > > > > > > > On Mon, Apr 6, 2020 at 12:06 PM Jun Rao < > jun@confluent.io> > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > Hi, Kowshik, > > > > > > > > > > > > > > > > > > > > > > Thanks for the reply. A few more replies below. > > > > > > > > > > > > > > > > > > > > > > 100.6 You can look for the sentence "This operation > > > requires > > > > > > ALTER > > > > > > > on > > > > > > > > > > > CLUSTER." in KIP-455. Also, you can check its usage i= n > > > > > > > > > > > KafkaApis.authorize(). > > > > > > > > > > > > > > > > > > > > > > 110. From the external client/tooling perspective, it= 's > > > more > > > > > > > natural > > > > > > > > to > > > > > > > > > > use > > > > > > > > > > > the release version for features. If we can use the > same > > > > > release > > > > > > > > > version > > > > > > > > > > > for internal representation, it seems simpler (easier > to > > > > > > > understand, > > > > > > > > no > > > > > > > > > > > mapping overhead, etc). Is there a benefit with > separate > > > > > external > > > > > > > and > > > > > > > > > > > internal versioning schemes? > > > > > > > > > > > > > > > > > > > > > > 111. To put this in context, when we had IBP, the > default > > > > value > > > > > > is > > > > > > > > the > > > > > > > > > > > current released version. So, if you are a brand new > > user, > > > > you > > > > > > > don't > > > > > > > > > need > > > > > > > > > > > to configure IBP and all new features will be > immediately > > > > > > available > > > > > > > > in > > > > > > > > > > the > > > > > > > > > > > new cluster. If you are upgrading from an old version= , > > you > > > do > > > > > > need > > > > > > > to > > > > > > > > > > > understand and configure IBP. I see a similar pattern > > here > > > > for > > > > > > > > > > > features. From the ease of use perspective, ideally, = we > > > > > shouldn't > > > > > > > > > > require a > > > > > > > > > > > new user to have an extra step such as running a > > bootstrap > > > > > script > > > > > > > > > unless > > > > > > > > > > > it's truly necessary. If someone has a special need > (all > > > the > > > > > > cases > > > > > > > > you > > > > > > > > > > > mentioned seem special cases?), they can configure a > mode > > > > such > > > > > > that > > > > > > > > > > > features are enabled/disabled manually. > > > > > > > > > > > > > > > > > > > > > > Jun > > > > > > > > > > > > > > > > > > > > > > On Fri, Apr 3, 2020 at 5:54 PM Kowshik Prakasam < > > > > > > > > > kprakasam@confluent.io> > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > Hi Jun, > > > > > > > > > > > > > > > > > > > > > > > > Thanks for the feedback and suggestions. Please fin= d > my > > > > > > response > > > > > > > > > below. > > > > > > > > > > > > > > > > > > > > > > > > > 100.6 For every new request, the admin needs to > > control > > > > who > > > > > > is > > > > > > > > > > allowed > > > > > > > > > > > to > > > > > > > > > > > > > issue that request if security is enabled. So, we > > need > > > to > > > > > > > assign > > > > > > > > > the > > > > > > > > > > > new > > > > > > > > > > > > > request a ResourceType and possible AclOperations= . > > See > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-455%3A+Create+an+Ad= ministrative+API+for+Replica+Reassignment > > > > > > > > > > > > > as an example. > > > > > > > > > > > > > > > > > > > > > > > > (Kowshik): I don't see any reference to the words > > > > > ResourceType > > > > > > or > > > > > > > > > > > > AclOperations > > > > > > > > > > > > in the KIP. Please let me know how I can use the KI= P > > that > > > > you > > > > > > > > linked > > > > > > > > > to > > > > > > > > > > > > know how to > > > > > > > > > > > > setup the appropriate ResourceType and/or > > > ClusterOperation? > > > > > > > > > > > > > > > > > > > > > > > > > 105. If we change delete to disable, it's better = to > > do > > > > this > > > > > > > > > > > consistently > > > > > > > > > > > > in > > > > > > > > > > > > > request protocol and admin api as well. > > > > > > > > > > > > > > > > > > > > > > > > (Kowshik): The API shouldn't be called 'disable' wh= en > > it > > > is > > > > > > > > deleting > > > > > > > > > a > > > > > > > > > > > > feature. > > > > > > > > > > > > I've just changed the KIP to use 'delete'. I don't > > have a > > > > > > strong > > > > > > > > > > > > preference. > > > > > > > > > > > > > > > > > > > > > > > > > 110. The minVersion/maxVersion for features use > > int64. > > > > > > > Currently, > > > > > > > > > our > > > > > > > > > > > > > release version schema is major.minor.bugfix (e.g= . > > > > 2.5.0). > > > > > > It's > > > > > > > > > > > possible > > > > > > > > > > > > > for new features to be included in minor releases > > too. > > > > > Should > > > > > > > we > > > > > > > > > make > > > > > > > > > > > the > > > > > > > > > > > > > feature versioning match the release versioning? > > > > > > > > > > > > > > > > > > > > > > > > (Kowshik): The release version can be mapped to a s= et > > of > > > > > > feature > > > > > > > > > > > versions, > > > > > > > > > > > > and this can be done, for example in the tool (or > even > > > > > external > > > > > > > to > > > > > > > > > the > > > > > > > > > > > > tool). > > > > > > > > > > > > Can you please clarify what I'm missing? > > > > > > > > > > > > > > > > > > > > > > > > > 111. "During regular operations, the data in the = ZK > > > node > > > > > can > > > > > > be > > > > > > > > > > mutated > > > > > > > > > > > > > only via a specific admin API served only by the > > > > > > controller." I > > > > > > > > am > > > > > > > > > > > > > wondering why can't the controller auto finalize = a > > > > feature > > > > > > > > version > > > > > > > > > > > after > > > > > > > > > > > > > all brokers are upgraded? For new users who > download > > > the > > > > > > latest > > > > > > > > > > version > > > > > > > > > > > > to > > > > > > > > > > > > > build a new cluster, it's inconvenient for them t= o > > have > > > > to > > > > > > > > manually > > > > > > > > > > > > enable > > > > > > > > > > > > > each feature. > > > > > > > > > > > > > > > > > > > > > > > > (Kowshik): I agree that there is a trade-off here, > but > > it > > > > > will > > > > > > > help > > > > > > > > > > > > to decide whether the automation can be thought > through > > > in > > > > > the > > > > > > > > future > > > > > > > > > > > > in a follow up KIP, or right now in this KIP. We ma= y > > > invest > > > > > > > > > > > > in automation, but we have to decide whether we > should > > do > > > > it > > > > > > > > > > > > now or later. > > > > > > > > > > > > > > > > > > > > > > > > For the inconvenience that you mentioned, do you > think > > > the > > > > > > > problem > > > > > > > > > that > > > > > > > > > > > you > > > > > > > > > > > > mentioned can be overcome by asking for the cluste= r > > > > operator > > > > > > to > > > > > > > > run > > > > > > > > > a > > > > > > > > > > > > bootstrap script when he/she knows that a specific > AK > > > > > release > > > > > > > has > > > > > > > > > been > > > > > > > > > > > > almost completely deployed in a cluster for the fir= st > > > time? > > > > > > Idea > > > > > > > is > > > > > > > > > > that > > > > > > > > > > > > the > > > > > > > > > > > > bootstrap script will know how to map a specific AK > > > release > > > > > to > > > > > > > > > > finalized > > > > > > > > > > > > feature versions, and run the `kafka-features.sh` > tool > > > > > > > > appropriately > > > > > > > > > > > > against > > > > > > > > > > > > the cluster. > > > > > > > > > > > > > > > > > > > > > > > > Now, coming back to your automation > proposal/question. > > > > > > > > > > > > I do see the value of automated feature version > > > > finalization, > > > > > > > but I > > > > > > > > > > also > > > > > > > > > > > > see > > > > > > > > > > > > that this will open up several questions and some > > risks, > > > as > > > > > > > > explained > > > > > > > > > > > > below. > > > > > > > > > > > > The answers to these depend on the definition of th= e > > > > > automation > > > > > > > we > > > > > > > > > > choose > > > > > > > > > > > > to build, and how well does it fit into a kafka > > > deployment. > > > > > > > > > > > > Basically, it can be unsafe for the controller to > > > finalize > > > > > > > feature > > > > > > > > > > > version > > > > > > > > > > > > upgrades automatically, without learning about the > > intent > > > > of > > > > > > the > > > > > > > > > > cluster > > > > > > > > > > > > operator. > > > > > > > > > > > > 1. We would sometimes want to lock feature versions > > only > > > > when > > > > > > we > > > > > > > > have > > > > > > > > > > > > externally verified > > > > > > > > > > > > the stability of the broker binary. > > > > > > > > > > > > 2. Sometimes only the cluster operator knows that a > > > cluster > > > > > > > upgrade > > > > > > > > > is > > > > > > > > > > > > complete, > > > > > > > > > > > > and new brokers are highly unlikely to join the > > cluster. > > > > > > > > > > > > 3. Only the cluster operator knows that the intent = is > > to > > > > > deploy > > > > > > > the > > > > > > > > > > same > > > > > > > > > > > > version > > > > > > > > > > > > of the new broker release across the entire cluster > > (i.e. > > > > the > > > > > > > > latest > > > > > > > > > > > > downloaded version). > > > > > > > > > > > > 4. For downgrades, it appears the controller still > > needs > > > > some > > > > > > > > > external > > > > > > > > > > > > input > > > > > > > > > > > > (such as the proposed tool) to finalize a feature > > version > > > > > > > > downgrade. > > > > > > > > > > > > > > > > > > > > > > > > If we have automation, that automation can end up > > failing > > > > in > > > > > > some > > > > > > > > of > > > > > > > > > > the > > > > > > > > > > > > cases > > > > > > > > > > > > above. Then, we need a way to declare that the > cluster > > is > > > > > "not > > > > > > > > ready" > > > > > > > > > > if > > > > > > > > > > > > the > > > > > > > > > > > > controller cannot automatically finalize some basic > > > > required > > > > > > > > feature > > > > > > > > > > > > version > > > > > > > > > > > > upgrades across the cluster. We need to make the > > cluster > > > > > > operator > > > > > > > > > aware > > > > > > > > > > > in > > > > > > > > > > > > such a scenario (raise an alert or alike). > > > > > > > > > > > > > > > > > > > > > > > > > 112. DeleteFeaturesResponse: It seems the apiKey > > should > > > > be > > > > > 49 > > > > > > > > > instead > > > > > > > > > > > of > > > > > > > > > > > > 48. > > > > > > > > > > > > > > > > > > > > > > > > (Kowshik): Done. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Cheers, > > > > > > > > > > > > Kowshik > > > > > > > > > > > > > > > > > > > > > > > > On Fri, Apr 3, 2020 at 11:24 AM Jun Rao < > > > jun@confluent.io> > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > Hi, Kowshik, > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks for the reply. A few more comments below. > > > > > > > > > > > > > > > > > > > > > > > > > > 100.6 For every new request, the admin needs to > > control > > > > who > > > > > > is > > > > > > > > > > allowed > > > > > > > > > > > to > > > > > > > > > > > > > issue that request if security is enabled. So, we > > need > > > to > > > > > > > assign > > > > > > > > > the > > > > > > > > > > > new > > > > > > > > > > > > > request a ResourceType and possible AclOperations= . > > See > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-455%3A+Create+an+Ad= ministrative+API+for+Replica+Reassignment > > > > > > > > > > > > > as > > > > > > > > > > > > > an example. > > > > > > > > > > > > > > > > > > > > > > > > > > 105. If we change delete to disable, it's better = to > > do > > > > this > > > > > > > > > > > consistently > > > > > > > > > > > > in > > > > > > > > > > > > > request protocol and admin api as well. > > > > > > > > > > > > > > > > > > > > > > > > > > 110. The minVersion/maxVersion for features use > > int64. > > > > > > > Currently, > > > > > > > > > our > > > > > > > > > > > > > release version schema is major.minor.bugfix (e.g= . > > > > 2.5.0). > > > > > > It's > > > > > > > > > > > possible > > > > > > > > > > > > > for new features to be included in minor releases > > too. > > > > > Should > > > > > > > we > > > > > > > > > make > > > > > > > > > > > the > > > > > > > > > > > > > feature versioning match the release versioning? > > > > > > > > > > > > > > > > > > > > > > > > > > 111. "During regular operations, the data in the = ZK > > > node > > > > > can > > > > > > be > > > > > > > > > > mutated > > > > > > > > > > > > > only via a specific admin API served only by the > > > > > > controller." I > > > > > > > > am > > > > > > > > > > > > > wondering why can't the controller auto finalize = a > > > > feature > > > > > > > > version > > > > > > > > > > > after > > > > > > > > > > > > > all brokers are upgraded? For new users who > download > > > the > > > > > > latest > > > > > > > > > > version > > > > > > > > > > > > to > > > > > > > > > > > > > build a new cluster, it's inconvenient for them t= o > > have > > > > to > > > > > > > > manually > > > > > > > > > > > > enable > > > > > > > > > > > > > each feature. > > > > > > > > > > > > > > > > > > > > > > > > > > 112. DeleteFeaturesResponse: It seems the apiKey > > should > > > > be > > > > > 49 > > > > > > > > > instead > > > > > > > > > > > of > > > > > > > > > > > > > 48. > > > > > > > > > > > > > > > > > > > > > > > > > > Jun > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Fri, Apr 3, 2020 at 1:27 AM Kowshik Prakasam < > > > > > > > > > > > kprakasam@confluent.io> > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > Hey Jun, > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks a lot for the great feedback! Please not= e > > that > > > > the > > > > > > > > design > > > > > > > > > > > > > > has changed a little bit on the KIP, and we now > > > > propagate > > > > > > the > > > > > > > > > > > finalized > > > > > > > > > > > > > > features metadata only via ZK watches (instead = of > > > > > > > > > > > UpdateMetadataRequest > > > > > > > > > > > > > > from the controller). > > > > > > > > > > > > > > > > > > > > > > > > > > > > Please find below my response to your > > > > questions/feedback, > > > > > > > with > > > > > > > > > the > > > > > > > > > > > > prefix > > > > > > > > > > > > > > "(Kowshik):". > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 100. > UpdateFeaturesRequest/UpdateFeaturesResponse > > > > > > > > > > > > > > > 100.1 Since this request waits for responses > from > > > > > > brokers, > > > > > > > > > should > > > > > > > > > > > we > > > > > > > > > > > > > add > > > > > > > > > > > > > > a > > > > > > > > > > > > > > > timeout in the request (like > createTopicRequest)? > > > > > > > > > > > > > > > > > > > > > > > > > > > > (Kowshik): Great point! Done. I have added a > > timeout > > > > > field. > > > > > > > > Note: > > > > > > > > > > we > > > > > > > > > > > no > > > > > > > > > > > > > > longer > > > > > > > > > > > > > > wait for responses from brokers, since the desi= gn > > has > > > > > been > > > > > > > > > changed > > > > > > > > > > so > > > > > > > > > > > > > that > > > > > > > > > > > > > > the > > > > > > > > > > > > > > features information is propagated via ZK. > > > > Nevertheless, > > > > > it > > > > > > > is > > > > > > > > > > right > > > > > > > > > > > to > > > > > > > > > > > > > > have a timeout > > > > > > > > > > > > > > for the request. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 100.2 The response schema is a bit weird. > > > Typically, > > > > > the > > > > > > > > > response > > > > > > > > > > > > just > > > > > > > > > > > > > > > shows an error code and an error message, > instead > > > of > > > > > > > echoing > > > > > > > > > the > > > > > > > > > > > > > request. > > > > > > > > > > > > > > > > > > > > > > > > > > > > (Kowshik): Great point! Yeah, I have modified i= t > to > > > > just > > > > > > > return > > > > > > > > > an > > > > > > > > > > > > error > > > > > > > > > > > > > > code and a message. > > > > > > > > > > > > > > Previously it was not echoing the "request", > rather > > > it > > > > > was > > > > > > > > > > returning > > > > > > > > > > > > the > > > > > > > > > > > > > > latest set of > > > > > > > > > > > > > > cluster-wide finalized features (after applying > the > > > > > > updates). > > > > > > > > But > > > > > > > > > > you > > > > > > > > > > > > are > > > > > > > > > > > > > > right, > > > > > > > > > > > > > > the additional info is not required, so I have > > > removed > > > > it > > > > > > > from > > > > > > > > > the > > > > > > > > > > > > > response > > > > > > > > > > > > > > schema. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 100.3 Should we add a separate request to > > > > list/describe > > > > > > the > > > > > > > > > > > existing > > > > > > > > > > > > > > > features? > > > > > > > > > > > > > > > > > > > > > > > > > > > > (Kowshik): This is already present in the KIP v= ia > > the > > > > > > > > > > > > 'DescribeFeatures' > > > > > > > > > > > > > > Admin API, > > > > > > > > > > > > > > which, underneath covers uses the > > ApiVersionsRequest > > > to > > > > > > > > > > list/describe > > > > > > > > > > > > the > > > > > > > > > > > > > > existing features. Please read the 'Tooling > > support' > > > > > > section. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 100.4 We are mixing ADD_OR_UPDATE and DELETE > in a > > > > > single > > > > > > > > > request. > > > > > > > > > > > For > > > > > > > > > > > > > > > DELETE, the version field doesn't make sense. > > So, I > > > > > guess > > > > > > > the > > > > > > > > > > > broker > > > > > > > > > > > > > just > > > > > > > > > > > > > > > ignores this? An alternative way is to have a > > > > separate > > > > > > > > > > > > > > DeleteFeaturesRequest > > > > > > > > > > > > > > > > > > > > > > > > > > > > (Kowshik): Great point! I have modified the KIP > now > > > to > > > > > > have 2 > > > > > > > > > > > separate > > > > > > > > > > > > > > controller APIs > > > > > > > > > > > > > > serving these different purposes: > > > > > > > > > > > > > > 1. updateFeatures > > > > > > > > > > > > > > 2. deleteFeatures > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 100.5 In UpdateFeaturesResponse, we have "The > > > > > > monotonically > > > > > > > > > > > > increasing > > > > > > > > > > > > > > > version of the metadata for finalized > features." > > I > > > am > > > > > > > > wondering > > > > > > > > > > why > > > > > > > > > > > > the > > > > > > > > > > > > > > > ordering is important? > > > > > > > > > > > > > > > > > > > > > > > > > > > > (Kowshik): In the latest KIP write-up, it is > called > > > > epoch > > > > > > > > > (instead > > > > > > > > > > of > > > > > > > > > > > > > > version), and > > > > > > > > > > > > > > it is just the ZK node version. Basically, this > is > > > the > > > > > > epoch > > > > > > > > for > > > > > > > > > > the > > > > > > > > > > > > > > cluster-wide > > > > > > > > > > > > > > finalized feature version metadata. This metada= ta > > is > > > > > served > > > > > > > to > > > > > > > > > > > clients > > > > > > > > > > > > > via > > > > > > > > > > > > > > the > > > > > > > > > > > > > > ApiVersionsResponse (for reads). We propagate > > updates > > > > > from > > > > > > > the > > > > > > > > > > > > > '/features' > > > > > > > > > > > > > > ZK node > > > > > > > > > > > > > > to all brokers, via ZK watches setup by each > broker > > > on > > > > > the > > > > > > > > > > > '/features' > > > > > > > > > > > > > > node. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Now here is why the ordering is important: > > > > > > > > > > > > > > ZK watches don't propagate at the same time. As= a > > > > result, > > > > > > the > > > > > > > > > > > > > > ApiVersionsResponse > > > > > > > > > > > > > > is eventually consistent across brokers. This c= an > > > > > introduce > > > > > > > > cases > > > > > > > > > > > > > > where clients see an older lower epoch of the > > > features > > > > > > > > metadata, > > > > > > > > > > > after > > > > > > > > > > > > a > > > > > > > > > > > > > > more recent > > > > > > > > > > > > > > higher epoch was returned at a previous point i= n > > > time. > > > > We > > > > > > > > expect > > > > > > > > > > > > clients > > > > > > > > > > > > > > to always employ the rule that the latest > received > > > > higher > > > > > > > epoch > > > > > > > > > of > > > > > > > > > > > > > metadata > > > > > > > > > > > > > > always trumps an older smaller epoch. Those > clients > > > > that > > > > > > are > > > > > > > > > > external > > > > > > > > > > > > to > > > > > > > > > > > > > > Kafka should strongly consider discovering the > > latest > > > > > > > metadata > > > > > > > > > once > > > > > > > > > > > > > during > > > > > > > > > > > > > > startup from the brokers, and if required refre= sh > > the > > > > > > > metadata > > > > > > > > > > > > > periodically > > > > > > > > > > > > > > (to get the latest metadata). > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 100.6 Could you specify the required ACL for > this > > > new > > > > > > > > request? > > > > > > > > > > > > > > > > > > > > > > > > > > > > (Kowshik): What is ACL, and how could I find ou= t > > > which > > > > > one > > > > > > to > > > > > > > > > > > specify? > > > > > > > > > > > > > > Please could you provide me some pointers? I'll > be > > > glad > > > > > to > > > > > > > > update > > > > > > > > > > the > > > > > > > > > > > > > > KIP once I know the next steps. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 101. For the broker registration ZK node, > should > > we > > > > > bump > > > > > > up > > > > > > > > the > > > > > > > > > > > > version > > > > > > > > > > > > > > in > > > > > > > > > > > > > > the json? > > > > > > > > > > > > > > > > > > > > > > > > > > > > (Kowshik): Great point! Done. I've increased th= e > > > > version > > > > > in > > > > > > > the > > > > > > > > > > > broker > > > > > > > > > > > > > json > > > > > > > > > > > > > > by 1. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 102. For the /features ZK node, not sure if w= e > > need > > > > the > > > > > > > epoch > > > > > > > > > > > field. > > > > > > > > > > > > > Each > > > > > > > > > > > > > > > ZK node has an internal version field that is > > > > > incremented > > > > > > > on > > > > > > > > > > every > > > > > > > > > > > > > > update. > > > > > > > > > > > > > > > > > > > > > > > > > > > > (Kowshik): Great point! Done. I'm using the ZK > node > > > > > version > > > > > > > > now, > > > > > > > > > > > > instead > > > > > > > > > > > > > of > > > > > > > > > > > > > > explicitly > > > > > > > > > > > > > > incremented epoch. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 103. "Enabling the actual semantics of a > feature > > > > > version > > > > > > > > > > > cluster-wide > > > > > > > > > > > > > is > > > > > > > > > > > > > > > left to the discretion of the logic > implementing > > > the > > > > > > > feature > > > > > > > > > (ex: > > > > > > > > > > > can > > > > > > > > > > > > > be > > > > > > > > > > > > > > > done via dynamic broker config)." Does that > mean > > > the > > > > > > broker > > > > > > > > > > > > > registration > > > > > > > > > > > > > > ZK > > > > > > > > > > > > > > > node will be updated dynamically when this > > happens? > > > > > > > > > > > > > > > > > > > > > > > > > > > > (Kowshik): Not really. The text was just > conveying > > > > that a > > > > > > > > broker > > > > > > > > > > > could > > > > > > > > > > > > > > "know" of > > > > > > > > > > > > > > a new feature version, but it does not mean the > > > broker > > > > > > should > > > > > > > > > have > > > > > > > > > > > also > > > > > > > > > > > > > > activated the effects of the feature version. > > Knowing > > > > vs > > > > > > > > > activation > > > > > > > > > > > > are 2 > > > > > > > > > > > > > > separate things, > > > > > > > > > > > > > > and the latter can be achieved by dynamic > config. I > > > > have > > > > > > > > reworded > > > > > > > > > > the > > > > > > > > > > > > > text > > > > > > > > > > > > > > to > > > > > > > > > > > > > > make this clear to the reader. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 104. UpdateMetadataRequest > > > > > > > > > > > > > > > 104.1 It would be useful to describe when the > > > feature > > > > > > > > metadata > > > > > > > > > is > > > > > > > > > > > > > > included > > > > > > > > > > > > > > > in the request. My understanding is that it's > > only > > > > > > included > > > > > > > > if > > > > > > > > > > (1) > > > > > > > > > > > > > there > > > > > > > > > > > > > > is > > > > > > > > > > > > > > > a change to the finalized feature; (2) broker > > > > restart; > > > > > > (3) > > > > > > > > > > > controller > > > > > > > > > > > > > > > failover. > > > > > > > > > > > > > > > 104.2 The new fields have the following > versions. > > > Why > > > > > are > > > > > > > the > > > > > > > > > > > > versions > > > > > > > > > > > > > 3+ > > > > > > > > > > > > > > > when the top version is bumped to 6? > > > > > > > > > > > > > > > "fields": [ > > > > > > > > > > > > > > > {"name": "Name", "type": "string", > > > > "versions": > > > > > > > > "3+", > > > > > > > > > > > > > > > "about": "The name of the feature."= }, > > > > > > > > > > > > > > > {"name": "Version", "type": "int64"= , > > > > > > "versions": > > > > > > > > > "3+", > > > > > > > > > > > > > > > "about": "The finalized version for > the > > > > > > > feature."} > > > > > > > > > > > > > > > ] > > > > > > > > > > > > > > > > > > > > > > > > > > > > (Kowshik): With the new improved design, we hav= e > > > > > completely > > > > > > > > > > > eliminated > > > > > > > > > > > > > the > > > > > > > > > > > > > > need to > > > > > > > > > > > > > > use UpdateMetadataRequest. This is because we n= ow > > > rely > > > > on > > > > > > ZK > > > > > > > to > > > > > > > > > > > deliver > > > > > > > > > > > > > the > > > > > > > > > > > > > > notifications for changes to the '/features' ZK > > node. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 105. kafka-features.sh: Instead of using > > > > update/delete, > > > > > > > > perhaps > > > > > > > > > > > it's > > > > > > > > > > > > > > better > > > > > > > > > > > > > > > to use enable/disable? > > > > > > > > > > > > > > > > > > > > > > > > > > > > (Kowshik): For delete, yes, I have changed it s= o > > that > > > > we > > > > > > > > instead > > > > > > > > > > call > > > > > > > > > > > > it > > > > > > > > > > > > > > 'disable'. > > > > > > > > > > > > > > However for 'update', it can now also refer to > > either > > > > an > > > > > > > > upgrade > > > > > > > > > > or a > > > > > > > > > > > > > > forced downgrade. > > > > > > > > > > > > > > Therefore, I have left it the way it is, just > > calling > > > > it > > > > > as > > > > > > > > just > > > > > > > > > > > > > 'update'. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Cheers, > > > > > > > > > > > > > > Kowshik > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, Mar 31, 2020 at 6:51 PM Jun Rao < > > > > > jun@confluent.io> > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi, Kowshik, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks for the KIP. Looks good overall. A few > > > > comments > > > > > > > below. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 100. > UpdateFeaturesRequest/UpdateFeaturesResponse > > > > > > > > > > > > > > > 100.1 Since this request waits for responses > from > > > > > > brokers, > > > > > > > > > should > > > > > > > > > > > we > > > > > > > > > > > > > add > > > > > > > > > > > > > > a > > > > > > > > > > > > > > > timeout in the request (like > createTopicRequest)? > > > > > > > > > > > > > > > 100.2 The response schema is a bit weird. > > > Typically, > > > > > the > > > > > > > > > response > > > > > > > > > > > > just > > > > > > > > > > > > > > > shows an error code and an error message, > instead > > > of > > > > > > > echoing > > > > > > > > > the > > > > > > > > > > > > > request. > > > > > > > > > > > > > > > 100.3 Should we add a separate request to > > > > list/describe > > > > > > the > > > > > > > > > > > existing > > > > > > > > > > > > > > > features? > > > > > > > > > > > > > > > 100.4 We are mixing ADD_OR_UPDATE and DELETE > in a > > > > > single > > > > > > > > > request. > > > > > > > > > > > For > > > > > > > > > > > > > > > DELETE, the version field doesn't make sense. > > So, I > > > > > guess > > > > > > > the > > > > > > > > > > > broker > > > > > > > > > > > > > just > > > > > > > > > > > > > > > ignores this? An alternative way is to have a > > > > separate > > > > > > > > > > > > > > > DeleteFeaturesRequest > > > > > > > > > > > > > > > 100.5 In UpdateFeaturesResponse, we have "The > > > > > > monotonically > > > > > > > > > > > > increasing > > > > > > > > > > > > > > > version of the metadata for finalized > features." > > I > > > am > > > > > > > > wondering > > > > > > > > > > why > > > > > > > > > > > > the > > > > > > > > > > > > > > > ordering is important? > > > > > > > > > > > > > > > 100.6 Could you specify the required ACL for > this > > > new > > > > > > > > request? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 101. For the broker registration ZK node, > should > > we > > > > > bump > > > > > > up > > > > > > > > the > > > > > > > > > > > > version > > > > > > > > > > > > > > in > > > > > > > > > > > > > > > the json? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 102. For the /features ZK node, not sure if w= e > > need > > > > the > > > > > > > epoch > > > > > > > > > > > field. > > > > > > > > > > > > > Each > > > > > > > > > > > > > > > ZK node has an internal version field that is > > > > > incremented > > > > > > > on > > > > > > > > > > every > > > > > > > > > > > > > > update. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 103. "Enabling the actual semantics of a > feature > > > > > version > > > > > > > > > > > cluster-wide > > > > > > > > > > > > > is > > > > > > > > > > > > > > > left to the discretion of the logic > implementing > > > the > > > > > > > feature > > > > > > > > > (ex: > > > > > > > > > > > can > > > > > > > > > > > > > be > > > > > > > > > > > > > > > done via dynamic broker config)." Does that > mean > > > the > > > > > > broker > > > > > > > > > > > > > registration > > > > > > > > > > > > > > ZK > > > > > > > > > > > > > > > node will be updated dynamically when this > > happens? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 104. UpdateMetadataRequest > > > > > > > > > > > > > > > 104.1 It would be useful to describe when the > > > feature > > > > > > > > metadata > > > > > > > > > is > > > > > > > > > > > > > > included > > > > > > > > > > > > > > > in the request. My understanding is that it's > > only > > > > > > included > > > > > > > > if > > > > > > > > > > (1) > > > > > > > > > > > > > there > > > > > > > > > > > > > > is > > > > > > > > > > > > > > > a change to the finalized feature; (2) broker > > > > restart; > > > > > > (3) > > > > > > > > > > > controller > > > > > > > > > > > > > > > failover. > > > > > > > > > > > > > > > 104.2 The new fields have the following > versions. > > > Why > > > > > are > > > > > > > the > > > > > > > > > > > > versions > > > > > > > > > > > > > 3+ > > > > > > > > > > > > > > > when the top version is bumped to 6? > > > > > > > > > > > > > > > "fields": [ > > > > > > > > > > > > > > > {"name": "Name", "type": "string", > > > > "versions": > > > > > > > > "3+", > > > > > > > > > > > > > > > "about": "The name of the feature."= }, > > > > > > > > > > > > > > > {"name": "Version", "type": "int64"= , > > > > > > "versions": > > > > > > > > > "3+", > > > > > > > > > > > > > > > "about": "The finalized version for > the > > > > > > > feature."} > > > > > > > > > > > > > > > ] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 105. kafka-features.sh: Instead of using > > > > update/delete, > > > > > > > > perhaps > > > > > > > > > > > it's > > > > > > > > > > > > > > better > > > > > > > > > > > > > > > to use enable/disable? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Jun > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, Mar 31, 2020 at 5:29 PM Kowshik > Prakasam > > < > > > > > > > > > > > > > kprakasam@confluent.io > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hey Boyang, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks for the great feedback! I have updat= ed > > the > > > > KIP > > > > > > > based > > > > > > > > > on > > > > > > > > > > > your > > > > > > > > > > > > > > > > feedback. > > > > > > > > > > > > > > > > Please find my response below for your > > comments, > > > > look > > > > > > for > > > > > > > > > > > sentences > > > > > > > > > > > > > > > > starting > > > > > > > > > > > > > > > > with "(Kowshik)" below. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 1. "When is it safe for the brokers to > begin > > > > > handling > > > > > > > EOS > > > > > > > > > > > > traffic" > > > > > > > > > > > > > > > could > > > > > > > > > > > > > > > > be > > > > > > > > > > > > > > > > > converted as "When is it safe for the > brokers > > > to > > > > > > start > > > > > > > > > > serving > > > > > > > > > > > > new > > > > > > > > > > > > > > > > > Exactly-Once(EOS) semantics" since EOS is > not > > > > > > explained > > > > > > > > > > earlier > > > > > > > > > > > > in > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > context. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > (Kowshik): Great point! Done. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 2. In the *Explanation *section, the > metadata > > > > > version > > > > > > > > > number > > > > > > > > > > > part > > > > > > > > > > > > > > > seems a > > > > > > > > > > > > > > > > > bit blurred. Could you point a reference = to > > > later > > > > > > > section > > > > > > > > > > that > > > > > > > > > > > we > > > > > > > > > > > > > > going > > > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > store it in Zookeeper and update it every > > time > > > > when > > > > > > > there > > > > > > > > > is > > > > > > > > > > a > > > > > > > > > > > > > > feature > > > > > > > > > > > > > > > > > change? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > (Kowshik): Great point! Done. I've added a > > > > reference > > > > > in > > > > > > > the > > > > > > > > > > KIP. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 3. For the feature downgrade, although > it's a > > > > > > Non-goal > > > > > > > of > > > > > > > > > the > > > > > > > > > > > > KIP, > > > > > > > > > > > > > > for > > > > > > > > > > > > > > > > > features such as group coordinator > semantics, > > > > there > > > > > > is > > > > > > > no > > > > > > > > > > legal > > > > > > > > > > > > > > > scenario > > > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > perform a downgrade at all. So having > > downgrade > > > > > door > > > > > > > open > > > > > > > > > is > > > > > > > > > > > > pretty > > > > > > > > > > > > > > > > > error-prone as human faults happen all th= e > > > time. > > > > > I'm > > > > > > > > > assuming > > > > > > > > > > > as > > > > > > > > > > > > > new > > > > > > > > > > > > > > > > > features are implemented, it's not very > hard > > to > > > > > add a > > > > > > > > flag > > > > > > > > > > > during > > > > > > > > > > > > > > > feature > > > > > > > > > > > > > > > > > creation to indicate whether this feature > is > > > > > > > > > "downgradable". > > > > > > > > > > > > Could > > > > > > > > > > > > > > you > > > > > > > > > > > > > > > > > explain a bit more on the extra engineeri= ng > > > > effort > > > > > > for > > > > > > > > > > shipping > > > > > > > > > > > > > this > > > > > > > > > > > > > > > KIP > > > > > > > > > > > > > > > > > with downgrade protection in place? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > (Kowshik): Great point! I'd agree and > disagree > > > > here. > > > > > > > While > > > > > > > > I > > > > > > > > > > > agree > > > > > > > > > > > > > that > > > > > > > > > > > > > > > > accidental > > > > > > > > > > > > > > > > downgrades can cause problems, I also think > > > > sometimes > > > > > > > > > > downgrades > > > > > > > > > > > > > should > > > > > > > > > > > > > > > > be allowed for emergency reasons (not all > > > > downgrades > > > > > > > cause > > > > > > > > > > > issues). > > > > > > > > > > > > > > > > It is just subjective to the feature being > > > > > downgraded. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > To be more strict about feature version > > > > downgrades, I > > > > > > > have > > > > > > > > > > > modified > > > > > > > > > > > > > the > > > > > > > > > > > > > > > KIP > > > > > > > > > > > > > > > > proposing that we mandate a > `--force-downgrade` > > > > flag > > > > > be > > > > > > > > used > > > > > > > > > in > > > > > > > > > > > the > > > > > > > > > > > > > > > > UPDATE_FEATURES api > > > > > > > > > > > > > > > > and the tooling, whenever the human is > > > downgrading > > > > a > > > > > > > > > finalized > > > > > > > > > > > > > feature > > > > > > > > > > > > > > > > version. > > > > > > > > > > > > > > > > Hopefully this should cover the requirement= , > > > until > > > > we > > > > > > > find > > > > > > > > > the > > > > > > > > > > > need > > > > > > > > > > > > > for > > > > > > > > > > > > > > > > advanced downgrade support. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 4. "Each broker=E2=80=99s supported dicti= onary of > > > feature > > > > > > > > versions > > > > > > > > > > will > > > > > > > > > > > > be > > > > > > > > > > > > > > > > defined > > > > > > > > > > > > > > > > > in the broker code." So this means in ord= er > > to > > > > > > > restrict a > > > > > > > > > > > certain > > > > > > > > > > > > > > > > feature, > > > > > > > > > > > > > > > > > we need to start the broker first and the= n > > > send a > > > > > > > feature > > > > > > > > > > > gating > > > > > > > > > > > > > > > request > > > > > > > > > > > > > > > > > immediately, which introduces a time gap > and > > > the > > > > > > > > > > > > intended-to-close > > > > > > > > > > > > > > > > feature > > > > > > > > > > > > > > > > > could actually serve request during this > > phase. > > > > Do > > > > > > you > > > > > > > > > think > > > > > > > > > > we > > > > > > > > > > > > > > should > > > > > > > > > > > > > > > > also > > > > > > > > > > > > > > > > > support configurations as well so that > admin > > > user > > > > > > could > > > > > > > > > > freely > > > > > > > > > > > > roll > > > > > > > > > > > > > > up > > > > > > > > > > > > > > > a > > > > > > > > > > > > > > > > > cluster with all nodes complying the same > > > feature > > > > > > > gating, > > > > > > > > > > > without > > > > > > > > > > > > > > > > worrying > > > > > > > > > > > > > > > > > about the turnaround time to propagate th= e > > > > message > > > > > > only > > > > > > > > > after > > > > > > > > > > > the > > > > > > > > > > > > > > > cluster > > > > > > > > > > > > > > > > > starts up? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > (Kowshik): This is a great point/question. > One > > of > > > > the > > > > > > > > > > > expectations > > > > > > > > > > > > > out > > > > > > > > > > > > > > of > > > > > > > > > > > > > > > > this KIP, which is > > > > > > > > > > > > > > > > already followed in the broker, is the > > following. > > > > > > > > > > > > > > > > - Imagine at time T1 the broker starts up > and > > > > > > registers > > > > > > > > it=E2=80=99s > > > > > > > > > > > > > presence > > > > > > > > > > > > > > in > > > > > > > > > > > > > > > > ZK, > > > > > > > > > > > > > > > > along with advertising it=E2=80=99s supp= orted > > > features. > > > > > > > > > > > > > > > > - Imagine at a future time T2 the broker > > > receives > > > > > the > > > > > > > > > > > > > > > > UpdateMetadataRequest > > > > > > > > > > > > > > > > from the controller, which contains the > > latest > > > > > > > finalized > > > > > > > > > > > > features > > > > > > > > > > > > > as > > > > > > > > > > > > > > > > seen by > > > > > > > > > > > > > > > > the controller. The broker validates thi= s > > data > > > > > > against > > > > > > > > > it=E2=80=99s > > > > > > > > > > > > > > supported > > > > > > > > > > > > > > > > features to > > > > > > > > > > > > > > > > make sure there is no mismatch (it will > > > shutdown > > > > > if > > > > > > > > there > > > > > > > > > is > > > > > > > > > > > an > > > > > > > > > > > > > > > > incompatibility). > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > It is expected that during the time between > > the 2 > > > > > > events > > > > > > > T1 > > > > > > > > > and > > > > > > > > > > > T2, > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > broker is > > > > > > > > > > > > > > > > almost a silent entity in the cluster. It > does > > > not > > > > > add > > > > > > > any > > > > > > > > > > value > > > > > > > > > > > to > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > cluster, or carry > > > > > > > > > > > > > > > > out any important broker activities. By > > > > =E2=80=9Cimportant=E2=80=9D, > > > > > I > > > > > > > mean > > > > > > > > > it > > > > > > > > > > is > > > > > > > > > > > > not > > > > > > > > > > > > > > > doing > > > > > > > > > > > > > > > > mutations > > > > > > > > > > > > > > > > on it=E2=80=99s persistence, not mutating c= ritical > > > > in-memory > > > > > > > state, > > > > > > > > > > won=E2=80=99t > > > > > > > > > > > > be > > > > > > > > > > > > > > > > serving > > > > > > > > > > > > > > > > produce/fetch requests. Note it doesn=E2=80= =99t even > > know > > > > > it=E2=80=99s > > > > > > > > > assigned > > > > > > > > > > > > > > > partitions > > > > > > > > > > > > > > > > until > > > > > > > > > > > > > > > > it receives UpdateMetadataRequest from > > > controller. > > > > > > > Anything > > > > > > > > > the > > > > > > > > > > > > > broker > > > > > > > > > > > > > > is > > > > > > > > > > > > > > > > doing up > > > > > > > > > > > > > > > > until this point is not damaging/useful. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I=E2=80=99ve clarified the above in the KIP= , see this > > new > > > > > > > section: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-584%3A+Versioning+s= cheme+for+features#KIP-584:Versioningschemeforfeatures-Incompatiblebrokerli= fetime > > > > > > > > > > > > > > > > . > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 5. "adding a new Feature, updating or > > deleting > > > an > > > > > > > > existing > > > > > > > > > > > > > Feature", > > > > > > > > > > > > > > > may > > > > > > > > > > > > > > > > be > > > > > > > > > > > > > > > > > I misunderstood something, I thought the > > > features > > > > > are > > > > > > > > > defined > > > > > > > > > > > in > > > > > > > > > > > > > > broker > > > > > > > > > > > > > > > > > code, so admin could not really create a > new > > > > > feature? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > (Kowshik): Great point! You understood this > > > right. > > > > > Here > > > > > > > > > adding > > > > > > > > > > a > > > > > > > > > > > > > > feature > > > > > > > > > > > > > > > > means we are > > > > > > > > > > > > > > > > adding a cluster-wide finalized *max* versi= on > > > for a > > > > > > > feature > > > > > > > > > > that > > > > > > > > > > > > was > > > > > > > > > > > > > > > > previously never finalized. > > > > > > > > > > > > > > > > I have clarified this in the KIP now. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 6. I think we need a separate error code > like > > > > > > > > > > > > > > > FEATURE_UPDATE_IN_PROGRESS > > > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > reject a concurrent feature update reques= t. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > (Kowshik): Great point! I have modified the > KIP > > > > > adding > > > > > > > the > > > > > > > > > > above > > > > > > > > > > > > (see > > > > > > > > > > > > > > > > 'Tooling support -> Admin API changes'). > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 7. I think we haven't discussed the > > alternative > > > > > > > solution > > > > > > > > to > > > > > > > > > > > pass > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > feature information through Zookeeper. Is > > that > > > > > > > mentioned > > > > > > > > in > > > > > > > > > > the > > > > > > > > > > > > KIP > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > justify why using UpdateMetadata is more > > > > favorable? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > (Kowshik): Nice question! The broker reads > > > > finalized > > > > > > > > feature > > > > > > > > > > info > > > > > > > > > > > > > > stored > > > > > > > > > > > > > > > in > > > > > > > > > > > > > > > > ZK, > > > > > > > > > > > > > > > > only during startup when it does a > validation. > > > When > > > > > > > serving > > > > > > > > > > > > > > > > `ApiVersionsRequest`, the > > > > > > > > > > > > > > > > broker does not read this info from ZK > > directly. > > > > I'd > > > > > > > > imagine > > > > > > > > > > the > > > > > > > > > > > > risk > > > > > > > > > > > > > > is > > > > > > > > > > > > > > > > that it can increase > > > > > > > > > > > > > > > > the ZK read QPS which can be a bottleneck f= or > > the > > > > > > system. > > > > > > > > > > Today, > > > > > > > > > > > in > > > > > > > > > > > > > > Kafka > > > > > > > > > > > > > > > > we use the > > > > > > > > > > > > > > > > controller to fan out ZK updates to brokers > and > > > we > > > > > want > > > > > > > to > > > > > > > > > > stick > > > > > > > > > > > to > > > > > > > > > > > > > > that > > > > > > > > > > > > > > > > pattern to avoid > > > > > > > > > > > > > > > > the ZK read bottleneck when serving > > > > > > `ApiVersionsRequest`. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 8. I was under the impression that user > could > > > > > > > configure a > > > > > > > > > > range > > > > > > > > > > > > of > > > > > > > > > > > > > > > > > supported versions, what's the trade-off > for > > > > > allowing > > > > > > > > > single > > > > > > > > > > > > > > finalized > > > > > > > > > > > > > > > > > version only? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > (Kowshik): Great question! The finalized > > version > > > > of a > > > > > > > > feature > > > > > > > > > > > > > basically > > > > > > > > > > > > > > > > refers to > > > > > > > > > > > > > > > > the cluster-wide finalized feature "maximum= " > > > > version. > > > > > > For > > > > > > > > > > > example, > > > > > > > > > > > > if > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > 'group_coordinator' feature > > > > > > > > > > > > > > > > has the finalized version set to 10, then, = it > > > means > > > > > > that > > > > > > > > > > > > cluster-wide > > > > > > > > > > > > > > all > > > > > > > > > > > > > > > > versions upto v10 are > > > > > > > > > > > > > > > > supported for this feature. However, note > that > > if > > > > > some > > > > > > > > > version > > > > > > > > > > > (ex: > > > > > > > > > > > > > v0) > > > > > > > > > > > > > > > > gets deprecated > > > > > > > > > > > > > > > > for this feature, then we don=E2=80=99t con= vey that > > using > > > > > this > > > > > > > > scheme > > > > > > > > > > > (also > > > > > > > > > > > > > > > > supporting deprecation is a non-goal). > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > (Kowshik): I=E2=80=99ve now modified the KI= P at all > > > points, > > > > > > > > refering > > > > > > > > > to > > > > > > > > > > > > > > finalized > > > > > > > > > > > > > > > > feature "maximum" versions. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 9. One minor syntax fix: Note that here t= he > > > > > "client" > > > > > > > here > > > > > > > > > may > > > > > > > > > > > be > > > > > > > > > > > > a > > > > > > > > > > > > > > > > producer > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > (Kowshik): Great point! Done. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Cheers, > > > > > > > > > > > > > > > > Kowshik > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, Mar 31, 2020 at 1:17 PM Boyang Chen= < > > > > > > > > > > > > > > reluctanthero104@gmail.com> > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hey Kowshik, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > thanks for the revised KIP. Got a couple = of > > > > > > questions: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 1. "When is it safe for the brokers to > begin > > > > > handling > > > > > > > EOS > > > > > > > > > > > > traffic" > > > > > > > > > > > > > > > could > > > > > > > > > > > > > > > > be > > > > > > > > > > > > > > > > > converted as "When is it safe for the > brokers > > > to > > > > > > start > > > > > > > > > > serving > > > > > > > > > > > > new > > > > > > > > > > > > > > > > > Exactly-Once(EOS) semantics" since EOS is > not > > > > > > explained > > > > > > > > > > earlier > > > > > > > > > > > > in > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > context. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 2. In the *Explanation *section, the > metadata > > > > > version > > > > > > > > > number > > > > > > > > > > > part > > > > > > > > > > > > > > > seems a > > > > > > > > > > > > > > > > > bit blurred. Could you point a reference = to > > > later > > > > > > > section > > > > > > > > > > that > > > > > > > > > > > we > > > > > > > > > > > > > > going > > > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > store it in Zookeeper and update it every > > time > > > > when > > > > > > > there > > > > > > > > > is > > > > > > > > > > a > > > > > > > > > > > > > > feature > > > > > > > > > > > > > > > > > change? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 3. For the feature downgrade, although > it's a > > > > > > Non-goal > > > > > > > of > > > > > > > > > the > > > > > > > > > > > > KIP, > > > > > > > > > > > > > > for > > > > > > > > > > > > > > > > > features such as group coordinator > semantics, > > > > there > > > > > > is > > > > > > > no > > > > > > > > > > legal > > > > > > > > > > > > > > > scenario > > > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > perform a downgrade at all. So having > > downgrade > > > > > door > > > > > > > open > > > > > > > > > is > > > > > > > > > > > > pretty > > > > > > > > > > > > > > > > > error-prone as human faults happen all th= e > > > time. > > > > > I'm > > > > > > > > > assuming > > > > > > > > > > > as > > > > > > > > > > > > > new > > > > > > > > > > > > > > > > > features are implemented, it's not very > hard > > to > > > > > add a > > > > > > > > flag > > > > > > > > > > > during > > > > > > > > > > > > > > > feature > > > > > > > > > > > > > > > > > creation to indicate whether this feature > is > > > > > > > > > "downgradable". > > > > > > > > > > > > Could > > > > > > > > > > > > > > you > > > > > > > > > > > > > > > > > explain a bit more on the extra engineeri= ng > > > > effort > > > > > > for > > > > > > > > > > shipping > > > > > > > > > > > > > this > > > > > > > > > > > > > > > KIP > > > > > > > > > > > > > > > > > with downgrade protection in place? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 4. "Each broker=E2=80=99s supported dicti= onary of > > > feature > > > > > > > > versions > > > > > > > > > > will > > > > > > > > > > > > be > > > > > > > > > > > > > > > > defined > > > > > > > > > > > > > > > > > in the broker code." So this means in ord= er > > to > > > > > > > restrict a > > > > > > > > > > > certain > > > > > > > > > > > > > > > > feature, > > > > > > > > > > > > > > > > > we need to start the broker first and the= n > > > send a > > > > > > > feature > > > > > > > > > > > gating > > > > > > > > > > > > > > > request > > > > > > > > > > > > > > > > > immediately, which introduces a time gap > and > > > the > > > > > > > > > > > > intended-to-close > > > > > > > > > > > > > > > > feature > > > > > > > > > > > > > > > > > could actually serve request during this > > phase. > > > > Do > > > > > > you > > > > > > > > > think > > > > > > > > > > we > > > > > > > > > > > > > > should > > > > > > > > > > > > > > > > also > > > > > > > > > > > > > > > > > support configurations as well so that > admin > > > user > > > > > > could > > > > > > > > > > freely > > > > > > > > > > > > roll > > > > > > > > > > > > > > up > > > > > > > > > > > > > > > a > > > > > > > > > > > > > > > > > cluster with all nodes complying the same > > > feature > > > > > > > gating, > > > > > > > > > > > without > > > > > > > > > > > > > > > > worrying > > > > > > > > > > > > > > > > > about the turnaround time to propagate th= e > > > > message > > > > > > only > > > > > > > > > after > > > > > > > > > > > the > > > > > > > > > > > > > > > cluster > > > > > > > > > > > > > > > > > starts up? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 5. "adding a new Feature, updating or > > deleting > > > an > > > > > > > > existing > > > > > > > > > > > > > Feature", > > > > > > > > > > > > > > > may > > > > > > > > > > > > > > > > be > > > > > > > > > > > > > > > > > I misunderstood something, I thought the > > > features > > > > > are > > > > > > > > > defined > > > > > > > > > > > in > > > > > > > > > > > > > > broker > > > > > > > > > > > > > > > > > code, so admin could not really create a > new > > > > > feature? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 6. I think we need a separate error code > like > > > > > > > > > > > > > > > FEATURE_UPDATE_IN_PROGRESS > > > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > reject a concurrent feature update reques= t. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 7. I think we haven't discussed the > > alternative > > > > > > > solution > > > > > > > > to > > > > > > > > > > > pass > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > feature information through Zookeeper. Is > > that > > > > > > > mentioned > > > > > > > > in > > > > > > > > > > the > > > > > > > > > > > > KIP > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > justify why using UpdateMetadata is more > > > > favorable? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 8. I was under the impression that user > could > > > > > > > configure a > > > > > > > > > > range > > > > > > > > > > > > of > > > > > > > > > > > > > > > > > supported versions, what's the trade-off > for > > > > > allowing > > > > > > > > > single > > > > > > > > > > > > > > finalized > > > > > > > > > > > > > > > > > version only? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 9. One minor syntax fix: Note that here t= he > > > > > "client" > > > > > > > here > > > > > > > > > may > > > > > > > > > > > be > > > > > > > > > > > > a > > > > > > > > > > > > > > > > producer > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Boyang > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Mar 30, 2020 at 4:53 PM Colin > McCabe > > < > > > > > > > > > > > cmccabe@apache.org > > > > > > > > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Thu, Mar 26, 2020, at 19:24, Kowshik > > > > Prakasam > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > Hi Colin, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks for the feedback! I've changed > the > > > KIP > > > > > to > > > > > > > > > address > > > > > > > > > > > your > > > > > > > > > > > > > > > > > > > suggestions. > > > > > > > > > > > > > > > > > > > Please find below my explanation. Her= e > > is a > > > > > link > > > > > > to > > > > > > > > KIP > > > > > > > > > > > 584: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-584%3A+Versioning+s= cheme+for+features > > > > > > > > > > > > > > > > > > > . > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 1. '__data_version__' is the version = of > > the > > > > > > > finalized > > > > > > > > > > > feature > > > > > > > > > > > > > > > > metadata > > > > > > > > > > > > > > > > > > > (i.e. actual ZK node contents), while > the > > > > > > > > > > > > '__schema_version__' > > > > > > > > > > > > > is > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > version of the schema of the data > > persisted > > > > in > > > > > > ZK. > > > > > > > > > These > > > > > > > > > > > > serve > > > > > > > > > > > > > > > > > different > > > > > > > > > > > > > > > > > > > purposes. '__data_version__' is is > useful > > > > > mainly > > > > > > to > > > > > > > > > > clients > > > > > > > > > > > > > > during > > > > > > > > > > > > > > > > > reads, > > > > > > > > > > > > > > > > > > > to differentiate between the 2 versio= ns > > of > > > > > > > eventually > > > > > > > > > > > > > consistent > > > > > > > > > > > > > > > > > > 'finalized > > > > > > > > > > > > > > > > > > > features' metadata (i.e. larger > metadata > > > > > version > > > > > > is > > > > > > > > > more > > > > > > > > > > > > > recent). > > > > > > > > > > > > > > > > > > > '__schema_version__' provides an > > additional > > > > > > degree > > > > > > > of > > > > > > > > > > > > > > flexibility, > > > > > > > > > > > > > > > > > where > > > > > > > > > > > > > > > > > > if > > > > > > > > > > > > > > > > > > > we decide to change the schema for > > > > '/features' > > > > > > node > > > > > > > > in > > > > > > > > > ZK > > > > > > > > > > > (in > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > future), > > > > > > > > > > > > > > > > > > > then we can manage broker roll outs > > > suitably > > > > > > (i.e. > > > > > > > > > > > > > > > > > > > serialization/deserialization of the = ZK > > > data > > > > > can > > > > > > be > > > > > > > > > > handled > > > > > > > > > > > > > > > safely). > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Kowshik, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > If you're talking about a number that > lets > > > you > > > > > know > > > > > > > if > > > > > > > > > data > > > > > > > > > > > is > > > > > > > > > > > > > more > > > > > > > > > > > > > > > or > > > > > > > > > > > > > > > > > > less recent, we would typically call th= at > > an > > > > > epoch, > > > > > > > and > > > > > > > > > > not a > > > > > > > > > > > > > > > version. > > > > > > > > > > > > > > > > > For > > > > > > > > > > > > > > > > > > the ZK data structures, the word > "version" > > is > > > > > > > typically > > > > > > > > > > > > reserved > > > > > > > > > > > > > > for > > > > > > > > > > > > > > > > > > describing changes to the overall schem= a > of > > > the > > > > > > data > > > > > > > > that > > > > > > > > > > is > > > > > > > > > > > > > > written > > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > > ZooKeeper. We don't even really change > the > > > > > > "version" > > > > > > > > of > > > > > > > > > > > those > > > > > > > > > > > > > > > schemas > > > > > > > > > > > > > > > > > that > > > > > > > > > > > > > > > > > > much, since most changes are > > > > > backwards-compatible. > > > > > > > But > > > > > > > > > we > > > > > > > > > > do > > > > > > > > > > > > > > include > > > > > > > > > > > > > > > > > that > > > > > > > > > > > > > > > > > > version field just in case. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I don't think we really need an epoch > here, > > > > > though, > > > > > > > > since > > > > > > > > > > we > > > > > > > > > > > > can > > > > > > > > > > > > > > just > > > > > > > > > > > > > > > > > look > > > > > > > > > > > > > > > > > > at the broker epoch. Whenever the brok= er > > > > > > registers, > > > > > > > > its > > > > > > > > > > > epoch > > > > > > > > > > > > > will > > > > > > > > > > > > > > > be > > > > > > > > > > > > > > > > > > greater than the previous broker epoch. > > And > > > > the > > > > > > > newly > > > > > > > > > > > > registered > > > > > > > > > > > > > > > data > > > > > > > > > > > > > > > > > will > > > > > > > > > > > > > > > > > > take priority. This will be a lot > simpler > > > than > > > > > > > adding > > > > > > > > a > > > > > > > > > > > > separate > > > > > > > > > > > > > > > epoch > > > > > > > > > > > > > > > > > > system, I think. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 2. Regarding admin client needing min > and > > > max > > > > > > > > > > information - > > > > > > > > > > > > you > > > > > > > > > > > > > > are > > > > > > > > > > > > > > > > > > right! > > > > > > > > > > > > > > > > > > > I've changed the KIP such that the > Admin > > > API > > > > > also > > > > > > > > > allows > > > > > > > > > > > the > > > > > > > > > > > > > user > > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > read > > > > > > > > > > > > > > > > > > > 'supported features' from a specific > > > broker. > > > > > > Please > > > > > > > > > look > > > > > > > > > > at > > > > > > > > > > > > the > > > > > > > > > > > > > > > > section > > > > > > > > > > > > > > > > > > > "Admin API changes". > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 3. Regarding the use of `long` vs > `Long` > > - > > > it > > > > > was > > > > > > > not > > > > > > > > > > > > > deliberate. > > > > > > > > > > > > > > > > I've > > > > > > > > > > > > > > > > > > > improved the KIP to just use `long` a= t > > all > > > > > > places. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Sounds good. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 4. Regarding kafka.admin.FeatureComma= nd > > > tool > > > > - > > > > > > you > > > > > > > > are > > > > > > > > > > > right! > > > > > > > > > > > > > > I've > > > > > > > > > > > > > > > > > > updated > > > > > > > > > > > > > > > > > > > the KIP sketching the functionality > > > provided > > > > by > > > > > > > this > > > > > > > > > > tool, > > > > > > > > > > > > with > > > > > > > > > > > > > > > some > > > > > > > > > > > > > > > > > > > examples. Please look at the section > > > "Tooling > > > > > > > support > > > > > > > > > > > > > examples". > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thank you! > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks, Kowshik. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > cheers, > > > > > > > > > > > > > > > > > > Colin > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Cheers, > > > > > > > > > > > > > > > > > > > Kowshik > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Mar 25, 2020 at 11:31 PM Coli= n > > > > McCabe < > > > > > > > > > > > > > > cmccabe@apache.org> > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks, Kowshik, this looks good. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > In the "Schema" section, do we real= ly > > > need > > > > > both > > > > > > > > > > > > > > > __schema_version__ > > > > > > > > > > > > > > > > > and > > > > > > > > > > > > > > > > > > > > __data_version__? Can we just have= a > > > > single > > > > > > > > version > > > > > > > > > > > field > > > > > > > > > > > > > > here? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Shouldn't the Admin(Client) functio= n > > have > > > > > some > > > > > > > way > > > > > > > > to > > > > > > > > > > get > > > > > > > > > > > > the > > > > > > > > > > > > > > min > > > > > > > > > > > > > > > > and > > > > > > > > > > > > > > > > > > max > > > > > > > > > > > > > > > > > > > > information that we're exposing as > > > well? I > > > > > > guess > > > > > > > > we > > > > > > > > > > > could > > > > > > > > > > > > > have > > > > > > > > > > > > > > > > min, > > > > > > > > > > > > > > > > > > max, > > > > > > > > > > > > > > > > > > > > and current. Unrelated: is the use > of > > > Long > > > > > > > rather > > > > > > > > > than > > > > > > > > > > > > long > > > > > > > > > > > > > > > > > deliberate > > > > > > > > > > > > > > > > > > > > here? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > It would be good to describe how th= e > > > > command > > > > > > line > > > > > > > > > tool > > > > > > > > > > > > > > > > > > > > kafka.admin.FeatureCommand will wor= k. > > > For > > > > > > > example > > > > > > > > > the > > > > > > > > > > > > flags > > > > > > > > > > > > > > that > > > > > > > > > > > > > > > > it > > > > > > > > > > > > > > > > > > will > > > > > > > > > > > > > > > > > > > > take and the output that it will > > generate > > > > to > > > > > > > > STDOUT. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > cheers, > > > > > > > > > > > > > > > > > > > > Colin > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, Mar 24, 2020, at 17:08, > Kowshik > > > > > > Prakasam > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I've opened KIP-584 > > > > > > > > > > > > > > > > > > < > > > > > > > > > > > > > > > > > https://issues.apache.org/jira/browse/KIP-584 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > which > > > > > > > > > > > > > > > > > > > > > is intended to provide a versioni= ng > > > > scheme > > > > > > for > > > > > > > > > > > features. > > > > > > > > > > > > > I'd > > > > > > > > > > > > > > > like > > > > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > > use > > > > > > > > > > > > > > > > > > > > > this thread to discuss the same. > I'd > > > > > > appreciate > > > > > > > > any > > > > > > > > > > > > > feedback > > > > > > > > > > > > > > on > > > > > > > > > > > > > > > > > this. > > > > > > > > > > > > > > > > > > > > > Here > > > > > > > > > > > > > > > > > > > > > is a link to KIP-584 > > > > > > > > > > > > > > > >: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-584%3A+Versioning+s= cheme+for+features > > > > > > > > > > > > > > > > > > > > > . > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thank you! > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Cheers, > > > > > > > > > > > > > > > > > > > > > Kowshik > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > --00000000000064211505a35e4a79--