Thanks for looking at the KIP, Manikumar. On Mon, Jan 31, 2022, at 11:21, Manikumar wrote: > Thanks for the KIP. LGTM once we get consensus on bootstrapping logic. >
The current plan for bootstrapping is to use super.users for now. Later, we'll have a separate KIP that will allow us to bootstrap a lot of KIP-500 / KRaft things by putting some records in the metadata log when we format the broker and controller nodes. > 1. When do we say, ACL is created (or deleted)? Is it after we persist it > durably by a majority of peers?. Yes. From the perspective of the brokers, the ACL is created when the record is persisted durably by a majority of peers (controllers). > 2. Can we add a metric for total acl count (This was missing in ZK > Authoriser). Good idea. I added a new metric to the KIP. best, Colin > > On Thu, Jan 13, 2022 at 2:26 AM David Arthur > <david.art...@confluent.io.invalid> wrote: > >> Sounds good on the ordering, and yea I agree we can look at atomic ACL >> modifications in the future. Thanks! >> >> On Wed, Jan 12, 2022 at 3:53 PM Colin McCabe <cmcc...@apache.org> wrote: >> >> > Hi David, >> > >> > On Wed, Dec 15, 2021, at 07:28, David Arthur wrote: >> > > 5) Ok, gotcha. So will the StandardAuthorizer be replaying records >> > > directly, or will it get an *Image like other metadata consumers on the >> > > broker? >> > > >> > >> > It reads the information out of the MetadataDelta, being careful to >> > preserve the ordering of the changes. >> > >> > The current implementation uses a LinkedHashMap to preserve that >> ordering. >> > You can take a look at the PR here: >> > https://github.com/apache/kafka/pull/11649/files >> > >> > > 6) I was thinking since the CreateAcl and DeleteAcl requests can modify >> > > multiple ACL in one request, that we should reflect that by committing >> > the >> > > resulting records as an atomic batch. I think from an operators >> > > perspective, they would expect the ACLs sent in their request to be >> > > enacted together atomically. >> > > >> > >> > That's never been guaranteed, though. Creating multiple ACLs in ZK >> > requires changing multiple znodes, which is not atomic. Given that users >> > haven't asked for this and it would add substantial complexity, can be >> > discuss it later once we have feature parity with the ZK version? >> > >> > best, >> > Colin >> > >> > >> > > >> > > >> > > On Tue, Dec 14, 2021 at 4:20 PM Colin McCabe <cmcc...@apache.org> >> wrote: >> > > >> > >> On Tue, Dec 14, 2021, at 08:27, José Armando García Sancio wrote: >> > >> > Thanks for the additional information Colin. >> > >> > >> > >> ... >> > >> > >> > >> > It sounds to me like KIP-801 is assuming that this "bootstrapping >> KIP" >> > >> > will at least generate a snapshot with this information in all of >> the >> > >> > controllers. I would like to understand this a bit better. Do you >> > >> > think that we need to write this "bootstrapping KIP" as soon as >> > >> > possible? >> > >> > >> > >> >> > >> Hi José, >> > >> >> > >> I don't know about "as soon as possible." The authorizer is useful >> even >> > >> without the bootstrapping KIP, as I mentioned (just using >> super.users). >> > But >> > >> I do think we'll need the bootstrapping KIP before KRaft goes GA. >> > >> >> > >> best, >> > >> Colin >> > >> >> > > >> > > >> > > -- >> > > -David >> > >> >> >> -- >> -David >>
