Hi Colin, Yes, it makes sense to reduce response size by using bit fields. Updated the KIP.
I have also updated the KIP to say that clients will ignore any bits set by the broker that are unknown to the client, so there will be no UNKNOWN operations in the set returned by AdminClient. Brokers may however set bits regardless of client version. Does that match your expectation? Thank you, Rajini On Sat, Feb 23, 2019 at 1:03 AM Colin McCabe <cmcc...@apache.org> wrote: > Hi Rajini, > > Thanks for the explanations. > > On Fri, Feb 22, 2019, at 11:59, Rajini Sivaram wrote: > > Hi Colin, > > > > Thanks for the review. Sorry I meant that an array of INT8's, each of > which > > is an AclOperation code will be returned. I have clarified that in the > KIP. > > Do you think it's worth considering a bitfield here still? An array will > take up at least 4 bytes for the length, plus whatever length the elements > are. A 32-bit bitfield would pretty much always take up less space. And > we can have a new version of the RPC with 64 bits or whatever if we outgrow > 32 operations. MetadataResponse for a big cluster could contain quite a > lot of topics, tens or hundreds of thousands. So the space savings could > be considerable. > > > > > All permitted operations will be returned from the set of supported > > operations on each resource. This is regardless of whether the access was > > implicitly or explicitly granted. Have clarified that in the KIP. > > Thanks. > > > > > Since the values returned are INT8 codes, clients can simply ignore any > > they don't recognize. Java clients convert these into > AclOperation.UNKNOWN. > > That way we don't need to update Metadata/describe request versions when > > new operations are added to a resource. This is consistent with > > DescribeAcls behaviour. Have added this to the compatibility section of > the > > KIP. > > Displaying "unknown" for new AclOperations made sense for DescribeAcls, > since the ACL is explicitly referencing the new AclOperation. For > example, if you upgrade your Kafka cluster to a new version that supports > DESCRIBE_CONFIGS, your old ACLs still don't reference DESCRIBE_CONFIGS. > > In contrast, in the case here, existing topics (or other resources) might > pick up the new ACLOperation just by upgrading Kafka. For example, if you > had ALL permission on a topic and you upgrade to a new version with > DESCRIBE_CONFIGS, you now have DESCRIBE_CONFIGS permission on that topic. > This would result in a lot of "unknowns" being displayed here, which might > not be ideal. > > Also, there is an argument from intent-- the intention here is to let you > know what you can do with a resource that already exists. Knowing that you > can do an unknown thing isn't very useful. In contrast, for DescribeAcls, > knowing that an ACL references an operation your software is too old to > understand is useful (you may choose not to modify that ACL, since you > don't know what it does, for example.) What do you think? > > cheers, > Colin > > > > > > Thank you, > > > > Rajini > > > > > > > > On Fri, Feb 22, 2019 at 6:46 PM Colin McCabe <cmcc...@apache.org> wrote: > > > > > Hi Rajini, > > > > > > Thanks for the KIP! > > > > > > The KIP specifies that "Authorized operations will be returned as [an] > > > INT8 consistent with [the] AclOperation used in ACL requests and > > > responses." But there may be more than one AclOperation that is > applied to > > > a given resource. For example, a principal may have both READ and > WRITE > > > permission on a topic. > > > > > > One option for representing this would be a bitfield. A 32-bit > bitfield > > > could have the appropriate bits set. For example, if READ and WRITE > > > operations were permitted, bits 3 and 4 could be set. > > > > > > Another thing to think about here is that certain AclOperations imply > > > certain others. For example, having WRITE on a topic gives you > DESCRIBE on > > > that topic as well automatically. Does that mean that a topic with > WRITE > > > on it should automatically get DESCRIBE set in the bitfield? I would > argue > > > that the answer is yes, for consistency's sake. > > > > > > We will inevitably add new AclOperations over time, and we have to > think > > > about how to do this in a compatible way. The simplest approach would > be > > > to just leave out the new AclOperations when a describe request comes > in > > > from an older version client. This should be spelled out in the > > > compatibility section. > > > > > > best, > > > Colin > > > > > > > > > On Thu, Feb 21, 2019, at 02:28, Rajini Sivaram wrote: > > > > I would like to start vote on KIP-430 to optionally obtain authorized > > > > operations when describing resources: > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-430+-+Return+Authorized+Operations+in+Describe+Responses > > > > > > > > Thank you, > > > > > > > > Rajini > > > > > > > > > >
