Gwen/Jay,

Have you had a chance to look at the updated KIP? It will be good to get
your feedback as well before restarting vote on the updated KIP.

If there are no objections, I will start the vote tomorrow.


On Fri, Jun 17, 2016 at 6:59 PM, Rajini Sivaram <
rajinisiva...@googlemail.com> wrote:

> Thank you, Jun. I have removed user_principal from the KIP.
>
> On Fri, Jun 17, 2016 at 6:00 PM, Jun Rao <j...@confluent.io> wrote:
>
>> Rajini,
>>
>> 10. Yes, then we can probably leave out the user_principal field and keep
>> the version to be 1.
>>
>> Other than that, the KIP looks good to me.
>>
>> Thanks,
>>
>> Jun
>>
>> On Fri, Jun 17, 2016 at 3:29 AM, Rajini Sivaram <
>> rajinisiva...@googlemail.com> wrote:
>>
>> > Jun,
>> >
>> > 10. Since entity_type "users" is new, shouldn't the JSON for these
>> entities
>> > have version 1? I have moved "user_principal" out of the config in the
>> > samples and added to the <user, client> entries as well. But actually,
>> do
>> > we need to store the non-encoded principal at all? The node name is
>> > URL-encoded user principal, so it is fairly readable if you are looking
>> in
>> > ZK and *kafka_configs.sh* will show the non-encoded principal by
>> decoding
>> > the name from the path (since it needs to do encoding anyway because the
>> > names specified on the command line will be non-encoded principals, it
>> can
>> > do decoding too). Perhaps that is sufficient?
>> >
>> > 11. I liked the second approach since it looks neat and future-proof.
>> Have
>> > updated the KIP.
>> >
>> > 12. Yes, that is correct.
>> >
>> > Many thanks,
>> >
>> > Rajini
>> >
>> >
>> > On Thu, Jun 16, 2016 at 11:36 PM, Jun Rao <j...@confluent.io> wrote:
>> >
>> > > Rajini,
>> > >
>> > > Thanks for the update. A few more questions/comments.
>> > >
>> > > 10. For the quota value stored in ZK, since we are adding an optional
>> > > user_principal field in the json, we should bump the version from 1
>> to 2.
>> > > Also, user_principal is not really part of the config values. So,
>> perhaps
>> > > we should represent it as the following?
>> > > {
>> > >     "version":2,
>> > >     "config": {
>> > >         "producer_byte_rate":"1024",
>> > >         "consumer_byte_rate":"2048"
>> > >     },
>> > >     "user_principal" : "user1"
>> > > }
>> > >
>> > >  Also, we should store user_principal in the following json too,
>> right?
>> > > // Zookeeper persistence path /users/<encoded-user2>/clients/clientA
>> > > {
>> > >     "version":1,
>> > >     "config": {
>> > >         "producer_byte_rate":"10",
>> > >         "consumer_byte_rate":"30"
>> > >     }
>> > > }
>> > >
>> > > 11. For the change notification path, would it be better to change it
>> to
>> > > something like the following and bump up version to 2?
>> > > // Change notification for quota of <user2, clientA>
>> > > {
>> > >     "version":2,
>> > >     [
>> > >       { "entity_type": "users",
>> > >         "entity_name": "user2"
>> > >       },
>> > >       { "entity_type": "clients",
>> > >         "entity_name": "clientA"
>> > >       }
>> > >     ]
>> > >  }
>> > >
>> > > Alternatively, we could change it to
>> > > // Change notification for quota of <user2, clientA>
>> > > {
>> > >     "version":2,
>> > >     "entity_path": "users/user2"
>> > > }
>> > >
>> > > {
>> > >     "version":2,
>> > >     "entity_path": "users/user2/clients/clientA"
>> > > }
>> > >
>> > > 12. Just to clarify on the meaning of remainder quota. If you have
>> quotas
>> > > like the following,
>> > >  <user1, client1> = 5
>> > >  <user1, client2> = 10
>> > >  <user1> = 12
>> > > it means that all connections with user1 whose client-id is neither
>> > client1
>> > > nor client2 will be sharing a quota of 12, right? In other words, the
>> > quota
>> > > of <user1> doesn't include the quota for <user1, client1> and <user1,
>> > > client2>.
>> > >
>> > > Thanks,
>> > >
>> > > Jun
>> > >
>> > >
>> > > On Thu, Jun 16, 2016 at 5:03 AM, Rajini Sivaram <
>> > > rajinisiva...@googlemail.com> wrote:
>> > >
>> > > > Jun,
>> > > >
>> > > > Actually, with quotas stored in different nodes in ZK, it is better
>> to
>> > > > store remainder quota rather than total quota under /users/<user> so
>> > that
>> > > > quota calculations are not dependent on the order of notifications.
>> I
>> > > have
>> > > > updated the KIP to reflect that. So the quotas in ZK now always
>> reflect
>> > > the
>> > > > quota applied to a group of client connections and use the same
>> format
>> > as
>> > > > client-id quotas. But it is not hierarchical, making the
>> configuration
>> > > > simpler.
>> > > >
>> > > > On Thu, Jun 16, 2016 at 11:49 AM, Rajini Sivaram <
>> > > > rajinisiva...@googlemail.com> wrote:
>> > > >
>> > > > > Jun,
>> > > > >
>> > > > > Thank you for the review. I have updated the KIP:
>> > > > >
>> > > > >
>> > > > >    1. Added an overview section. Slightly reworded since it is
>> better
>> > > to
>> > > > >    treat user and client-id as different levels rather than the
>> same
>> > > > level.
>> > > > >    2. Yes, it is neater to store quota for each entity in a
>> different
>> > > > >    path in Zookeeper. I put clients under users rather than the
>> other
>> > > way
>> > > > >    round since that reflects the hierarchy and also keeps a user's
>> > > quotas
>> > > > >    together under a single sub-tree. I had initially used a single
>> > node
>> > > > to
>> > > > >    keep quotas and sub-quotas of a user together so that updates
>> are
>> > > > atomic
>> > > > >    since changes to sub-quotas also affect remainder quotas for
>> other
>> > > > clients.
>> > > > >    But I imagine, updates to configs are rare and it is not a big
>> > > issue.
>> > > > >    3. I haven't modified the JSON for configuration change
>> > > notifications.
>> > > > >    The entity_name can now be a subpath that has both user and
>> > client.
>> > > > Have
>> > > > >    added an example to the KIP. The downside of keeping clients
>> under
>> > > > users in
>> > > > >    ZK in 2) is that the change notification for sub-quota has
>> > > entity_type
>> > > > >    "users". I could extend the JSON to include client separately,
>> but
>> > > > since
>> > > > >    changes to a client sub-quota does impact other clients of the
>> > user
>> > > > as well
>> > > > >    due to change in remainder quota, it may be ok as it is. Do
>> let me
>> > > > know if
>> > > > >    it looks confusing in the example.
>> > > > >    4. Agree, updated.
>> > > > >
>> > > > >
>> > > > > On Wed, Jun 15, 2016 at 10:27 PM, Jun Rao <j...@confluent.io>
>> wrote:
>> > > > >
>> > > > >> Hi, Rajini,
>> > > > >>
>> > > > >> Thanks for the updated wiki. Overall, I like the new approach. It
>> > > covers
>> > > > >> the common use cases now, is extensible, and is backward
>> > compatible. A
>> > > > few
>> > > > >> comments below.
>> > > > >>
>> > > > >> 1. It would be useful to describe a bit at the high level, how
>> the
>> > new
>> > > > >> approach works. I think this can be summarized as follows. Quotas
>> > can
>> > > be
>> > > > >> set at user, client-id or <user, client-id> levels. For a given
>> > client
>> > > > >> connection, the most specific quota matching the connection will
>> be
>> > > > >> applied. For example, if both a <user, client-id> and a <user>
>> quota
>> > > > match
>> > > > >> a connection, the <user, client-id> quota will be used. If more
>> > than 1
>> > > > >> quota at the same level (e.g., a quota on a user and another
>> quota
>> > on
>> > > a
>> > > > >> client-id) match the connection, the user level quota will be
>> used,
>> > > > i.e.,
>> > > > >> user quota takes precedence over client-id quota.
>> > > > >>
>> > > > >> 2. For the ZK data structure, would it be better to store <user,
>> > > > >> client-id>
>> > > > >> quota as the following. Then the format of the value in each
>> path is
>> > > the
>> > > > >> same. The wiki also mentions that we want to include the original
>> > user
>> > > > >> name
>> > > > >> in the ZK value. Could you describe that in an example?
>> > > > >> // Zookeeper persistence path
>> /clients/clientA/users/<encoded-user2>
>> > > > >> {
>> > > > >>     "version":1,
>> > > > >>     "config": {
>> > > > >>         "producer_byte_rate":"10",
>> > > > >>         "consumer_byte_rate":"20"
>> > > > >>     }
>> > > > >> }
>> > > > >>
>> > > > >> 3. Could you document the format change of the ZK value in
>> > > > >> /config/changes/config_change_xxx, if any?
>> > > > >>
>> > > > >> 4. For the config command, could we specify the sub-quota like
>> the
>> > > > >> following, instead of in the config value? This seems more
>> > intuitive.
>> > > > >>
>> > > > >> bin/kafka-configs  --zookeeper localhost:2181 --alter
>> --add-config
>> > > > >> 'producer_byte_rate=1024,consumer_byte_rate=2048' --entity-name
>> > > > >> clientA --entity-type clients --entity-name user2 --entity-type
>> > users
>> > > > >>
>> > > > >> Thanks,
>> > > > >>
>> > > > >> Jun
>> > > > >>
>> > > > >> On Wed, Jun 15, 2016 at 10:35 AM, Rajini Sivaram <
>> > > > >> rajinisiva...@googlemail.com> wrote:
>> > > > >>
>> > > > >> > Harsha,
>> > > > >> >
>> > > > >> > The sample configuration under
>> > > > >> >
>> > > > >> >
>> > > > >>
>> > > >
>> > >
>> >
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-55%3A+Secure+Quotas+for+Authenticated+Users#KIP-55:SecureQuotasforAuthenticatedUsers-QuotaConfiguration
>> > > > >> > shows
>> > > > >> > the Zookeeper data for different scenarios.
>> > > > >> >
>> > > > >> >    - *user1* (/users/user1 in ZK) has only user-level quotas
>> > > > >> >    - *user2* (/users/user2 in ZK) defines user-level quotas and
>> > > > >> sub-quotas
>> > > > >> >    for clients *clientA* and *clientB*. Other client-ids of
>> > *user2*
>> > > > >> share
>> > > > >> >    the remaining quota of *user2*.
>> > > > >> >
>> > > > >> >
>> > > > >> > On Wed, Jun 15, 2016 at 5:30 PM, Harsha <ka...@harsha.io>
>> wrote:
>> > > > >> >
>> > > > >> > > Rajini,
>> > > > >> > >           How does sub-quotas works in case of authenticated
>> > > users.
>> > > > >> > >           Where are we maintaining the relation between users
>> > and
>> > > > >> their
>> > > > >> > >           client Ids. Can you add an example of zk data under
>> > > > /users.
>> > > > >> > > Thanks,
>> > > > >> > > Harsha
>> > > > >> > >
>> > > > >> > > On Mon, Jun 13, 2016, at 05:01 AM, Rajini Sivaram wrote:
>> > > > >> > > > I have updated KIP-55 to reflect the changes from the
>> > > discussions
>> > > > in
>> > > > >> > the
>> > > > >> > > > voting thread (
>> > > > >> > > >
>> > https://www.mail-archive.com/dev@kafka.apache.org/msg51610.html
>> > > ).
>> > > > >> > > >
>> > > > >> > > > Jun/Gwen,
>> > > > >> > > >
>> > > > >> > > > Existing client-id quotas will be used as default client-id
>> > > quotas
>> > > > >> for
>> > > > >> > > > users when no user quotas are configured - i.e., default
>> user
>> > > > quota
>> > > > >> is
>> > > > >> > > > unlimited and no user-specific quota override is specified.
>> > This
>> > > > >> > enables
>> > > > >> > > > user rate limits to be configured for ANONYMOUS if required
>> > in a
>> > > > >> > cluster
>> > > > >> > > > that has both PLAINTEXT and SSL/SASL. By default, without
>> any
>> > > user
>> > > > >> rate
>> > > > >> > > > limits set, rate limits for client-ids will apply,
>> retaining
>> > the
>> > > > >> > current
>> > > > >> > > > client-id quota configuration for single-user clusters.
>> > > > >> > > >
>> > > > >> > > > Zookeeper will have two paths /clients with client-id
>> quotas
>> > > that
>> > > > >> apply
>> > > > >> > > > only when user quota is unlimited similar to now. And
>> /users
>> > > which
>> > > > >> > > > persists
>> > > > >> > > > user quotas for any user including ANONYMOUS.
>> > > > >> > > >
>> > > > >> > > > Comments and feedback are appreciated.
>> > > > >> > > >
>> > > > >> > > > Regards,
>> > > > >> > > >
>> > > > >> > > > Rajini
>> > > > >> > > >
>> > > > >> > > >
>> > > > >> > > > On Wed, Jun 8, 2016 at 9:00 PM, Rajini Sivaram
>> > > > >> > > > <rajinisiva...@googlemail.com
>> > > > >> > > > > wrote:
>> > > > >> > > >
>> > > > >> > > > > Jun,
>> > > > >> > > > >
>> > > > >> > > > > Oops, sorry, I hadn't realized that the last note was on
>> the
>> > > > >> discuss
>> > > > >> > > > > thread. Thank you for pointing it out. I have sent
>> another
>> > > note
>> > > > >> for
>> > > > >> > > voting.
>> > > > >> > > > >
>> > > > >> > > > >
>> > > > >> > > > > On Wed, Jun 8, 2016 at 4:30 PM, Jun Rao <
>> j...@confluent.io>
>> > > > wrote:
>> > > > >> > > > >
>> > > > >> > > > >> Rajini,
>> > > > >> > > > >>
>> > > > >> > > > >> Perhaps it will be clearer if you start the voting in a
>> new
>> > > > >> thread
>> > > > >> > > (with
>> > > > >> > > > >> VOTE in the subject).
>> > > > >> > > > >>
>> > > > >> > > > >> Thanks,
>> > > > >> > > > >>
>> > > > >> > > > >> Jun
>> > > > >> > > > >>
>> > > > >> > > > >> On Tue, Jun 7, 2016 at 1:55 PM, Rajini Sivaram <
>> > > > >> > > > >> rajinisiva...@googlemail.com
>> > > > >> > > > >> > wrote:
>> > > > >> > > > >>
>> > > > >> > > > >> > I would like to initiate the vote for KIP-55.
>> > > > >> > > > >> >
>> > > > >> > > > >> > The KIP details are here: KIP-55: Secure quotas for
>> > > > >> authenticated
>> > > > >> > > users
>> > > > >> > > > >> > <
>> > > > >> > > > >> >
>> > > > >> > > > >>
>> > > > >> > >
>> > > > >> >
>> > > > >>
>> > > >
>> > >
>> >
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-55%3A+Secure+Quotas+for+Authenticated+Users
>> > > > >> > > > >> > >
>> > > > >> > > > >> > .
>> > > > >> > > > >> >
>> > > > >> > > > >> > The JIRA  KAFKA-3492  <
>> > > > >> > > https://issues.apache.org/jira/browse/KAFKA-3492
>> > > > >> > > > >> > >has
>> > > > >> > > > >> > a draft PR here:
>> > https://github.com/apache/kafka/pull/1256
>> > > .
>> > > > >> > > > >> >
>> > > > >> > > > >> > Thank you...
>> > > > >> > > > >> >
>> > > > >> > > > >> >
>> > > > >> > > > >> > Regards,
>> > > > >> > > > >> >
>> > > > >> > > > >> > Rajini
>> > > > >> > > > >> >
>> > > > >> > > > >>
>> > > > >> > > > >
>> > > > >> > > > >
>> > > > >> > > > >
>> > > > >> > > > > --
>> > > > >> > > > > Regards,
>> > > > >> > > > >
>> > > > >> > > > > Rajini
>> > > > >> > > > >
>> > > > >> > > >
>> > > > >> > > >
>> > > > >> > > >
>> > > > >> > > > --
>> > > > >> > > > Regards,
>> > > > >> > > >
>> > > > >> > > > Rajini
>> > > > >> > >
>> > > > >> >
>> > > > >> >
>> > > > >> >
>> > > > >> > --
>> > > > >> > Regards,
>> > > > >> >
>> > > > >> > Rajini
>> > > > >> >
>> > > > >>
>> > > > >
>> > > > >
>> > > > >
>> > > > > --
>> > > > > Regards,
>> > > > >
>> > > > > Rajini
>> > > > >
>> > > >
>> > > >
>> > > >
>> > > > --
>> > > > Regards,
>> > > >
>> > > > Rajini
>> > > >
>> > >
>> >
>> >
>> >
>> > --
>> > Regards,
>> >
>> > Rajini
>> >
>>
>
>
>
> --
> Regards,
>
> Rajini
>



-- 
Regards,

Rajini

Reply via email to