Hi, Yaodong, Thanks for the updated KIP. A few more comments below.
11. For quotas, in addition to user and client, we need to be able to set a quota for <client, user> combination. Would that be a new resource type? How would the name look like for this resource? 12. Similarly, to support customizable quota ( https://cwiki.apache.org/confluence/display/KAFKA/KIP-257+-+Configurable+Quota+Management), do we need a new resource type? What would the name of the resource looks like? 13. You only listed 2 new ConfigSource. Could you list all the new ones? For example, https://cwiki.apache.org/confluence/display/KAFKA/KIP-248+-+Create+New+ConfigCommand+That+Uses+The+New+AdminClient listed a few others such as ClientInUser, DefaultClientInUser. 14. Could you list any new ACL that might be required? For example, what types of operations are allowed for the new Resource (User, Client, etc)? What are the permissions needed for the new operations? 15. Could you give a few examples on how to use the new API? Thanks, Jun On Tue, Mar 12, 2019 at 9:56 AM Yaodong Yang <yangyaodon...@gmail.com> wrote: > ping... > > On Thu, Feb 28, 2019 at 10:23 AM Yaodong Yang <yangyaodon...@gmail.com> > wrote: > >> Hello folks, >> >> Please share your comments for this KIP 😄 >> >> Thanks! >> Yaodong >> >> On Tue, Feb 26, 2019 at 6:53 PM Yaodong Yang <yangyaodon...@gmail.com> >> wrote: >> >>> Hello Colin, >>> >>> There is a POC PR for this KIP, and it contains most changes we are >>> proposing now. >>> >>> Best, >>> Yaodong >>> >>> On Tue, Feb 26, 2019 at 6:51 PM Yaodong Yang <yangyaodon...@gmail.com> >>> wrote: >>> >>>> Hello Colin, >>>> >>>> CIL, >>>> >>>> Thanks! >>>> Yaodong >>>> >>>> >>>> On Tue, Feb 26, 2019 at 9:59 AM Colin McCabe <cmcc...@apache.org> >>>> wrote: >>>> >>>>> Hi Yaodong, >>>>> >>>>> I don't understand how the proposed API would be used. It talks about >>>>> adding a ConfigResource type for clients and users, but doesn't explain >>>>> what can be done with these. >>>>> >>>> >>>> Sorry for the confusion. I just updated the KIP, and hopefully it will >>>> make it easier for you and other people. Looking forward to your feedback! >>>> >>>> >>>>> In the compatibility section (?) it says "We only add a new way to >>>>> configure the quotas" which suggests that quotas are involved somehow >>>>> What >>>>> relationship does this have with KIP-257? >>>>> >>>> >>>> Let me give you more context, feel free to correct me if I'm wrong 😁 >>>> >>>> 1. Originally we hit an issue that we can not config client quota >>>> through AdminClient. The only way available for us is directly talk to ZK >>>> and manage quota directly. >>>> >>>> 2. As our client service may not in the same DC as ZooKeeper, there >>>> could be some cross DC communication which is less desirable. >>>> >>>> 3. We deicide to add the quota configuration feature in the >>>> AdminClient, which will perfectly solve this issue for us. >>>> >>>> 4. In addition, we realized that this change can also serve as a way to >>>> config other users or clients configuration in Zookeeper. For instance, if >>>> we have a new client configuration introduced in the future and they need >>>> to be in the Zookeeper as well, we can mange it through the same API. >>>> Therefore, this KIP is renamed to manage users/clients configurations. >>>> Quota management is one use case for this configuration management. >>>> >>>> 5. KIP-257 is also compatible with the current KIP. For instance, if >>>> user want to update a quota for a metric, the client side need to parse it, >>>> and eventually pass in a user or client config to AdminClient. AdminClient >>>> will make sure such configuration changes are applied in the Zookeeper. >>>> >>>> >>>>> best, >>>>> Colin >>>>> >>>>> >>>>> On Fri, Feb 22, 2019, at 15:11, Yaodong Yang wrote: >>>>> > Hi Colin, >>>>> > >>>>> > CIL, >>>>> > >>>>> > Thanks! >>>>> > Yaodong >>>>> > >>>>> > >>>>> > On Fri, Feb 22, 2019 at 10:56 AM Colin McCabe <cmcc...@apache.org> >>>>> wrote: >>>>> > >>>>> > > Hi Yaodong, >>>>> > > >>>>> > > KIP-422 says that it would be good if "applications [could] >>>>> leverage the >>>>> > > unified KafkaAdminClient to manage their user/client >>>>> configurations, >>>>> > > instead of the direct dependency on Zookeeper." But the KIP >>>>> doesn't talk >>>>> > > about any changes to KafkaAdminClient. Instead, the only changes >>>>> proposed >>>>> > > are to AdminZKClient. But that is an internal class-- we don't >>>>> need a KIP >>>>> > > to change it, and it's not a public API that users can use. >>>>> > > >>>>> > >>>>> > Sorry for the confusion in the KIP. Actually there is no change to >>>>> > AdminZKClient needed for this KIP, we just leverage them to >>>>> configure the >>>>> > properties in the ZK. You can find the details from this PR >>>>> > https://github.com/apache/kafka/pull/6189 >>>>> > >>>>> > As you can see from the PR, we need the client side and server >>>>> process >>>>> > changes, so I feel like we still need the KIP for this change. >>>>> > >>>>> > >>>>> > > I realize that the naming might be a bit confusing, but >>>>> > > kafka.zk.AdminZKClient and kafka.admin.AdminClient are internal >>>>> classes. >>>>> > > As the JavaDoc says, kafka.admin.AdminClient is deprecated as >>>>> well. The >>>>> > > public class that we would be adding new methods to is >>>>> > > org.apache.kafka.clients.admin.AdminClient. >>>>> > > >>>>> > >>>>> > I agree. Thanks for pointing this out! >>>>> > >>>>> > >>>>> > > best, >>>>> > > Colin >>>>> > > >>>>> > > On Tue, Feb 19, 2019, at 15:21, Yaodong Yang wrote: >>>>> > > > Hello Jun, Viktor, Snoke and Stan, >>>>> > > > >>>>> > > > Thanks for taking time to look at this KIP-422! For some reason, >>>>> this >>>>> > > email >>>>> > > > was put in my spam folder. Sorry about that. >>>>> > > > >>>>> > > > Jun is right, the main motivation for this KIP-422 is to allow >>>>> users to >>>>> > > > config user/clientId quota through AdminClient. In addition, >>>>> this KIP-422 >>>>> > > > also allows users to set or update any config related to a user >>>>> or >>>>> > > clientId >>>>> > > > entity if needed in the future. >>>>> > > > >>>>> > > > For the KIP-257, I agree with Jun that we should add support for >>>>> it. I >>>>> > > will >>>>> > > > look at the current implementation and update the KIP-422 with >>>>> new >>>>> > > change. >>>>> > > > >>>>> > > > I will ping this thread once I updated the KIP. >>>>> > > > >>>>> > > > Thanks again! >>>>> > > > Yaodong >>>>> > > > >>>>> > > > On Fri, Feb 15, 2019 at 1:28 AM Viktor Somogyi-Vass < >>>>> > > viktorsomo...@gmail.com> >>>>> > > > wrote: >>>>> > > > >>>>> > > > > Hi Guys, >>>>> > > > > >>>>> > > > > I wanted to reject that KIP, split it up and revamp it as in >>>>> the >>>>> > > meantime >>>>> > > > > there were some overlapping works I just didn't get to it due >>>>> to other >>>>> > > > > higher priority work. >>>>> > > > > One of the splitted KIPs would have been the quota part of >>>>> that and >>>>> > > I'd be >>>>> > > > > happy if that lived in this KIP if Yaodong thinks it's worth to >>>>> > > > > incorporate. I'd be also happy to rebase that wire protocol and >>>>> > > contribute >>>>> > > > > it to this KIP. >>>>> > > > > >>>>> > > > > Viktor >>>>> > > > > >>>>> > > > > On Wed, Feb 13, 2019 at 7:14 PM Jun Rao <j...@confluent.io> >>>>> wrote: >>>>> > > > > >>>>> > > > > > Hi, Yaodong, >>>>> > > > > > >>>>> > > > > > Thanks for the KIP. As Stan mentioned earlier, it seems that >>>>> this is >>>>> > > > > > mostly covered by KIP-248, which was originally proposed by >>>>> Victor. >>>>> > > > > > >>>>> > > > > > Hi, Victor, >>>>> > > > > > >>>>> > > > > > Do you still plan to work on KIP-248? It seems that you >>>>> already got >>>>> > > > > pretty >>>>> > > > > > far on that. If not, would you mind letting Yaodong take >>>>> over this? >>>>> > > > > > >>>>> > > > > > For both KIP-248 and KIP-422, one thing that I found missing >>>>> is the >>>>> > > > > > support for customized quota ( >>>>> > > > > > >>>>> > > > > >>>>> > > >>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-257+-+Configurable+Quota+Management >>>>> > > > > ). >>>>> > > > > > With KIP-257, it's possible for one to construct a >>>>> customized quota >>>>> > > > > defined >>>>> > > > > > through a map of metric tags. It would be useful to support >>>>> that in >>>>> > > the >>>>> > > > > > AdminClient API and the wire protocol. >>>>> > > > > > >>>>> > > > > > Hi, Sonke, >>>>> > > > > > >>>>> > > > > > I think the proposal is to support the user/clientId level >>>>> quota >>>>> > > through >>>>> > > > > > an AdminClient api. The user can be obtained from any >>>>> existing >>>>> > > > > > authentication mechanisms. >>>>> > > > > > >>>>> > > > > > Thanks, >>>>> > > > > > >>>>> > > > > > Jun >>>>> > > > > > >>>>> > > > > > On Thu, Feb 7, 2019 at 5:59 AM Sönke Liebau >>>>> > > > > > <soenke.lie...@opencore.com.invalid> wrote: >>>>> > > > > > >>>>> > > > > >> Hi Yaodong, >>>>> > > > > >> >>>>> > > > > >> thanks for the KIP! >>>>> > > > > >> >>>>> > > > > >> If I understand your intentions correctly then this KIP >>>>> would only >>>>> > > > > >> address a fairly specific use case, namely SASL-PLAIN with >>>>> users >>>>> > > > > >> defined in Zookeeper. For all other authentication >>>>> mechanisms like >>>>> > > > > >> SSL, SASL-GSSAPI or SASL-PLAIN with users defined in jaas >>>>> files I >>>>> > > > > >> don't see how the AdminClient could directly create new >>>>> users. >>>>> > > > > >> Is this correct, or am I missing something? >>>>> > > > > >> >>>>> > > > > >> Best regards, >>>>> > > > > >> Sönke >>>>> > > > > >> >>>>> > > > > >> On Thu, Feb 7, 2019 at 2:47 PM Stanislav Kozlovski >>>>> > > > > >> <stanis...@confluent.io> wrote: >>>>> > > > > >> > >>>>> > > > > >> > This KIP seems to duplicate some of the functionality >>>>> proposed in >>>>> > > > > >> KIP-248 >>>>> > > > > >> > < >>>>> > > > > >> >>>>> > > > > >>>>> > > >>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-248+-+Create+New+ConfigCommand+That+Uses+The+New+AdminClient >>>>> > > > > >> >. >>>>> > > > > >> > KIP-248 has been stuck in a vote thread since July 2018. >>>>> > > > > >> > >>>>> > > > > >> > Viktor, do you plan on working on the KIP? >>>>> > > > > >> > >>>>> > > > > >> > On Thu, Feb 7, 2019 at 1:27 PM Stanislav Kozlovski < >>>>> > > > > >> stanis...@confluent.io> >>>>> > > > > >> > wrote: >>>>> > > > > >> > >>>>> > > > > >> > > Hey there Yaodong, thanks for the KIP! >>>>> > > > > >> > > >>>>> > > > > >> > > I'm not too familiar with the user/client >>>>> configurations we >>>>> > > > > currently >>>>> > > > > >> > > allow, is there a KIP describing the initial feature? >>>>> If there >>>>> > > is, >>>>> > > > > it >>>>> > > > > >> would >>>>> > > > > >> > > be useful to include in KIP-422. >>>>> > > > > >> > > >>>>> > > > > >> > > I also didn't see any authorization in the PR, have we >>>>> thought >>>>> > > about >>>>> > > > > >> > > needing to authorize the alter/describe requests per the >>>>> > > > > user/client? >>>>> > > > > >> > > >>>>> > > > > >> > > Thanks, >>>>> > > > > >> > > Stanislav >>>>> > > > > >> > > >>>>> > > > > >> > > On Fri, Jan 25, 2019 at 5:47 PM Yaodong Yang < >>>>> > > > > yangyaodon...@gmail.com >>>>> > > > > >> > >>>>> > > > > >> > > wrote: >>>>> > > > > >> > > >>>>> > > > > >> > >> Hi folks, >>>>> > > > > >> > >> >>>>> > > > > >> > >> I've published KIP-422 which is about adding support >>>>> for >>>>> > > > > user/client >>>>> > > > > >> > >> configurations in the Kafka Admin Client. >>>>> > > > > >> > >> >>>>> > > > > >> > >> Basically the story here is to allow KafkaAdminClient >>>>> to >>>>> > > configure >>>>> > > > > >> the >>>>> > > > > >> > >> user >>>>> > > > > >> > >> or client configurations for users, instead of >>>>> requiring users >>>>> > > to >>>>> > > > > >> directly >>>>> > > > > >> > >> talk to ZK. >>>>> > > > > >> > >> >>>>> > > > > >> > >> The link for this KIP is >>>>> > > > > >> > >> following: >>>>> > > > > >> > >> >>>>> > > > > >> >>>>> > > > > >>>>> > > >>>>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=97555704 >>>>> > > > > >> > >> >>>>> > > > > >> > >> I'd be happy to receive some feedback about the KIP I >>>>> > > published. >>>>> > > > > >> > >> >>>>> > > > > >> > >> -- >>>>> > > > > >> > >> Best, >>>>> > > > > >> > >> Yaodong Yang >>>>> > > > > >> > >> >>>>> > > > > >> > > >>>>> > > > > >> > > >>>>> > > > > >> > > -- >>>>> > > > > >> > > Best, >>>>> > > > > >> > > Stanislav >>>>> > > > > >> > > >>>>> > > > > >> > >>>>> > > > > >> > >>>>> > > > > >> > -- >>>>> > > > > >> > Best, >>>>> > > > > >> > Stanislav >>>>> > > > > >> >>>>> > > > > >> >>>>> > > > > >> >>>>> > > > > >> -- >>>>> > > > > >> Sönke Liebau >>>>> > > > > >> Partner >>>>> > > > > >> Tel. +49 179 7940878 >>>>> > > > > >> OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880 Wedel >>>>> - >>>>> > > Germany >>>>> > > > > >> >>>>> > > > > > >>>>> > > > > >>>>> > > > >>>>> > > >>>>> > >>>>> >>>>
