Hi, Yaodong,
Yes, what you proposed makes sense. A couple of more comments.
21. Could you add those examples to the KIP wiki? It would also be useful
to know how to set the ConfigEntry value for quotas at
DefaultClientInUser, DefaultClientDefaultUser
and DefaultUser level.
22. To support KIP-257, I guess we can just pass in some special string
value in ConfigEntry value through alterConfig and let the customized
implementation of ClientQuotaCallback parse it. It would be useful to
document this. Does that sound reasonable, Rajini?
Thanks,
Jun
On Fri, Apr 5, 2019 at 2:03 PM Yaodong Yang <yangyaodon...@gmail.com> wrote:
> Hi Jun,
>
> The proposal we have right now is to directly set the quota through
> existing admin client APIs. See following examples:
>
> Example 1: set a user quota
>
> AdminClient adminClient = mock(AdminClient.class);
> Map<ConfigResource, Config> configs = new HashMap();
> Config config = new Config(Arrays.asList(new ConfigEntry("user1",
> "producer_byte_rate=1024")));
> configs.put(singletonMap(ConfigResource.USER, config));
> adminClient.alterConfigs(configs);
>
>
> Example 2: set a client id quota
>
> AdminClient adminClient = mock(AdminClient.class);
> Map<ConfigResource, Config> configs = new HashMap();
> Config config = new Config(Arrays.asList(new ConfigEntry("client1",
> "producer_byte_rate=1024")));
> configs.put(singletonMap(ConfigResource.CLIENT, config));
> adminClient.alterConfigs(configs);
>
>
> Example 3: set a <user, client-id> quota
>
> AdminClient adminClient = mock(AdminClient.class);
> Map<ConfigResource, Config> configs = new HashMap();
> Config config = new Config(Arrays.asList(new
> ConfigEntry("user1/clients/client2", "producer_byte_rate=1024")));
> configs.put(singletonMap(ConfigResource.USER, config));
> adminClient.alterConfigs(configs);
>
>
> The current KIP is orthogonal to KIP 257. It only adds a new way to store
> the quotas in ZK through AdminClient. For customizable quotas, they will
> also be a property in User resources or Client resources.
>
> Quote from *CustomQuotaCallbackTest.scala::GroupedUserQuotaCallback* in the
> codebase, “Group quotas are configured in ZooKeeper as user quotas with the
> entity name "${group}_". Default group quotas are configured in ZooKeeper
> as user quotas with the entity name "_".”
> In this example, they are always stored as properties in the User resource,
> the property name is “$(group)_” and “_”. The client application needs to
> encode them correctly before store them in ZK through AdminClient, while
> the customizedCallback needs to decode them and apply during the process of
> each request.
>
> The advantage of the current KIP is simple and extensible for the future
> release. The alternative is to introduce a new set of quota related APIs in
> the AdminClient, similar to the ACL related. I'm not sure which one people
> prefer.
>
> Thanks!
> Yaodong
>
> On Wed, Mar 13, 2019 at 6:29 PM Jun Rao <j...@confluent.io> wrote:
>
> > 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?
> >
> git chec
>
> >
> > 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
> > >>>>> > > > > >>
> > >>>>> > > > > >
> > >>>>> > > > >
> > >>>>> > > >
> > >>>>> > >
> > >>>>> >
> > >>>>>
> > >>>>
> >
>
