Thanks for the KIP. LGTM once we get consensus on bootstrapping logic. 1. When do we say, ACL is created (or deleted)? Is it after we persist it durably by a majority of peers?. 2. Can we add a metric for total acl count (This was missing in ZK Authoriser).
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 >
