Ismael, Thank you for the review.
"*Would it be better to assign an id to each mechanism and pass that instead **of the String? That would be more space-efficient.*" Since we want to support arbitrary custom mechanisms, it feels better to use mechanism names rather than Strings. IDs would require ensuring that client and server have the same mapping. Since the mechanism name Strings are not particularly long and they are useful for debugging, I feel it is worthwhile to retain Strings rather than IDs. " *The currently proposed way has the benefit that newer clients would support older brokers _if_ thesasl.mechanism was GSSAPI. Is this important? For Kafka, brokers have to be upgraded before clients, generally. Brokers would still support older **clients that didn't send the mechanism either way.*" I wasn't sure whether Kafka attempted to retain compatibility in both directions. I would have preferred to send the mechanism even for GSSAPI to keep the code consistent, but went for version compatibility instead. I was thinking mainly of replication. If you have a cluster that uses 0.9.0.x with SASL replication, it would be easier to upgrade if new clients worked with old brokers. Thoughts? On Tue, Mar 1, 2016 at 11:44 AM, Ismael Juma <ism...@juma.me.uk> wrote: > Also, with regards to the client flow, it says: > > "If sasl.mechanism is not GSSAPI, send a packet with the mechanism name to > the server. Otherwise go to Step 3." > > It sounds like it would be more regular and simpler for clients if they > always sent the mechanism, even if GSSAPI, right? The currently proposed > way has the benefit that newer clients would support older brokers _if_ the > sasl.mechanism was GSSAPI. Is this important? For Kafka, brokers have to be > upgraded before clients, generally. Brokers would still support older > clients that didn't send the mechanism either way. > > Ismael > > > On Tue, Mar 1, 2016 at 3:11 AM, Ismael Juma <ism...@juma.me.uk> wrote: > > > Hi Rajini, > > > > One question below. > > > > On Tue, Mar 1, 2016 at 2:47 AM, Rajini Sivaram < > > rajinisiva...@googlemail.com> wrote: > > > >> 1. With GSSAPI, the first context establishment packet starts with > the > >> byte 0x60 (tag for APPLICATION-0) followed by a variable-length > encoded > >> size, followed by various tags and contents. And the packet also > >> contains a > >> checksum. This is completely different from the mechanism packet from > >> Kafka > >> clients which start with a two-byte version set to zero currently, > >> followed > >> by just a String mechanism. > >> > > > > Would it be better to assign an id to each mechanism and pass that > instead > > of the String? That would be more space-efficient. > > > > Ismael > > > -- Regards, Rajini
