Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

That sounds great, thanks Jun! On Wed, Jun 10, 2020 at 11:44 AM Jun Rao wrote: > Hi, Boyang, > > Thanks for the reply. > > For the metric, we just need to define a metric of meter type and of name > NumRequestsForwardingToControllerPerSec. Meter exposes a few standard JMX > attributes including

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hi, Boyang, Thanks for the reply. For the metric, we just need to define a metric of meter type and of name NumRequestsForwardingToControllerPerSec. Meter exposes a few standard JMX attributes including an accumulated total and rates (

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Thanks Jun for the suggestions! I have addressed suggestion and simplified the metrics part. On Tue, Jun 9, 2020 at 5:46 PM Jun Rao wrote: > Hi, Boyang, > > Thanks for the KIP. Just a few comments on the metrics. > > 1. It would be useful to document the full JMX metric names (package, type, >

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hi, Boyang, Thanks for the KIP. Just a few comments on the metrics. 1. It would be useful to document the full JMX metric names (package, type, etc) of the new metrics. Also, for rates on the server side, we typically use Yammer Meter. 2. For num-messages-redirected-rate, would

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hey there, If no more question is raised, I will go ahead and start the vote shortly. On Thu, Jun 4, 2020 at 12:39 PM Boyang Chen wrote: > Hey there, > > bumping this thread for any further KIP-590 discussion, since it's been > quiet for a while. > > Boyang > > On Thu, May 21, 2020 at 10:31 AM

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hey there, bumping this thread for any further KIP-590 discussion, since it's been quiet for a while. Boyang On Thu, May 21, 2020 at 10:31 AM Boyang Chen wrote: > Thanks David, I agree the wording here is not clear, and the fellow broker > should just send a new CreateTopicRequest in this

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Thanks David, I agree the wording here is not clear, and the fellow broker should just send a new CreateTopicRequest in this case. In the meantime, we had some offline discussion about the Envelope API as well. Although it provides certain privileges like data embedding and principal embedding,

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hi Boyang, I've got another question regarding the auto topic creation case. The KIP says: "Currently the target broker shall just utilize its own ZK client to create internal topics, which is disallowed in the bridge release. For above scenarios, non-controller broker shall just forward a

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Just to adds a bit more FYI here related to the last question from David: in KIP-595 while implementing the new requests we are also adding a "KafkaNetworkChannel" which is used for brokers to send vote / fetch records, so besides the discussion on listeners I think implementation wise we can also

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hey David, thanks for the feedbacks! On Wed, May 6, 2020 at 2:06 AM David Jacot wrote: > Hi Boyang, > > While re-reading the KIP, I've got few small questions/comments: > > 1. When auto topic creation is enabled, brokers will send a > CreateTopicRequest > to the controller instead of writing

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hi Boyang, While re-reading the KIP, I've got few small questions/comments: 1. When auto topic creation is enabled, brokers will send a CreateTopicRequest to the controller instead of writing to ZK directly. It means that creation of these topics are subject to be rejected with an error if a

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Ah, I see, thanks for the clarification! Shouldn't be an issue I think. My understanding of KIPs was always that they are mostly intended as a place to discuss and agree changes up front, whereas tracking the actual releases that things go into should be handled in Jira. So maybe we just create

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Sure thing Sonke, what I suggest is that usual KIPs get accepted to go into next release. It could span for a couple of releases because of engineering time, but no change has to be shipped in specific future releases, like the backward incompatible change for KafkaPrincipal. But I guess it's not

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hi Boyang, thanks for the update, sounds reasonable to me. Making it a breaking change is definitely the safer route to go. Just one quick question regarding your mail, I didn't fully understand what you mean by "I think this is the first time we need to introduce a KIP without having it fully

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hey Tom, thanks for the suggestion. As long as we could correctly serialize the principal and embed in the Envelope, I think we could still leverage the controller to do the client request authentication. Although this pays an extra round trip if the authorization is doomed to fail on the

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hi Boyang, Thanks for the update. In the EnvelopeRequest handling section of the KIP it might be worth saying explicitly that authorization of the request will happen as normal. Otherwise what you're proposing makes sense to me. Thanks again, Tom On Wed, Apr 29, 2020 at 5:27 PM Boyang Chen

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Thanks for the proposed idea Sonke. I reviewed it and had some offline discussion with Colin, Rajini and Mathew. We do need to add serializability to the PrincipalBuilder interface, but we should not make any default implementation which could go wrong and messy up with the security in a

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

After thinking on this a little bit, maybe this would be an option: add default methods serialize and deserialize to the KafkaPrincipalBuilder interface, these could be very short: default String serialize(KafkaPrincipal principal) { return principal.toString(); } default KafkaPrincipal

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Thanks all, IIUC, the necessity of doing the audit log on the controller side is because we need to make sure the authorized resource modifications eventually arrive on the target broker side, but is that really necessary? I'm thinking the possibility of doing the audit log on the forwarding

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

I agree that this would be useful to have and shouldn't create issues in 99% of all cases. But it would be a breaking change to a public API. I had a quick look at the two large projects that come to mind which might be affected: Ranger and Sentry - both seem to operate directly with

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hmm... Maybe we need to add some way to serialize and deserialize KafkaPrincipal subclasses to/from string. We could add a method to KafkaPrincipalBuilder#deserialize and a method KafkaPrincipal#serialize, I suppose. best, Colin On Thu, Apr 23, 2020, at 02:14, Tom Bentley wrote: > Hi folks,

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hi folks, Colin wrote: > The final broker knows it can trust the principal name in the envelope > (since EnvelopeRequest requires CLUSTERACTION on CLUSTER). So it can use > that principal name for authorization (given who you are, what can you > do?) The forwarded principal name will also be

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hi Colin, thanks for your summary! Just one question - and I may be missing an obvious point here.. You write: "The initial broker should do authentication (who are you?) and come up with a principal name. Then it creates an envelope request, which will contain that principal name, and sends it

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Colin, Boyang: thanks for the updates, I agree that an EnvelopeRequest would be a less vulnerable approach than optional fields, and I'm just wondering if we would keep the EnvelopeRequest for a long time. I was thinking that, potentially if we require clients to be on newer version when talking

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Thanks Colin for the summary! And Guozhang, regarding the future use cases, consider a scenario where there are temporary connectivity issue between controller to a fellow broker A, the controller could then leverage another healthy broker B to do a forwarding request to A in order to maintain a

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hi all, I guess the way I see this working is that the request gets sent from the client, to the initial broker, and then forwarded to the final broker. The initial broker should do authentication (who are you?) and come up with a principal name. Then it creates an envelope request, which

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hello Gwen, The purpose here is for maintaining compatibility old clients, who do not have functionality to do re-routing admin requests themselves. New clients can of course do this themselves by detecting who's the controller. Hello Colin / Boyang, Regarding the usage of the envelope, I'm

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hey Boyang, Sorry if this was already discussed, but I didn't see this as rejected alternative: Until now, we always did client side routing - the client itself found the controller via metadata and directed requests accordingly. Brokers that were not the controller, rejected those requests.

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

We do use custom principals that rely on information not contained in the serialized principal and hence authorization based on serialized principals can break existing production systems. Apart from the information contained in the session principal, ACLs can also be based on host I, but I guess

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hi Boyang and Sönke, Regarding custom Principals, I don't think too many people do this in > practice, but in theory you can provide you own PrincipalBuilder and use > your own Principal objects that contain as much additional information as > you wish. And since these can basically be any Java

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hi Boyang, I think what Tom is referring to is that it is very hard to forward enough information to the controller to put it into a position to properly authenticate any request. While the Default KafkaPrincipal can easily be serialized and sent to the controller, as previously seen, those are

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hey Tom, I agree with the claim here. All the brokers should have the same authentication power, which means getting the forwarding broker verify the client request first is more favorable. This approach avoids sending one unnecessary forwarding request if it couldn't pass the authorization in

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hi Boyang, The answer to my original question about the request principal was that the forwarding broker would authorize the request and the controller would trust the request since it was from another broker. AFAIU you added the principal purely for logging purposes. In the "EnvelopeRequest

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

On Fri, Apr 17, 2020, at 13:11, Ismael Juma wrote: > Hi Colin, > > The read/modify/write is protected by the zk version, right? > > Ismael No, we don't use the ZK version when doing the write to the config znodes. We do for ACLs, I think. This is something that we could fix just by using the

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Thanks Colin for the suggestions! I have added metrics to track the number of total messages being forwarded as a more generic monitoring. Right now the list of metrics are: - num-client-forwarding-to-controller-rate - num-client-fowarding-to-controller-count -

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

On Thu, Apr 16, 2020, at 12:33, Jose Garcia Sancio wrote: > Hi Boyang, > > Thanks for the KIP. The KIP looks good. I have a few questions and comments. > > > As part of the KIP-500 >

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hi Colin, The read/modify/write is protected by the zk version, right? Ismael On Fri, Apr 17, 2020 at 12:53 PM Colin McCabe wrote: > On Thu, Apr 16, 2020, at 08:51, Ismael Juma wrote: > > I don't think these requests are necessarily infrequent under multi > tenant > > environments though.

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

On Thu, Apr 16, 2020, at 08:51, Ismael Juma wrote: > I don't think these requests are necessarily infrequent under multi tenant > environments though. I've seen Controller availability being an issue for > describe topics for example (before it was changed to go to any broker). Hi Ismael, I

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hi David & Boyang, I thought the intention of the "old-client-connections-count" metric was to give some information about how many redirections were going on in the cluster. This is different from the "unknown" software version metric. After all, many versions that support KIP-511 will

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Thanks Jose for the questions! On Thu, Apr 16, 2020 at 12:33 PM Jose Garcia Sancio wrote: > Hi Boyang, > > Thanks for the KIP. The KIP looks good. I have a few questions and > comments. > > > As part of the KIP-500 > < >

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hi Boyang, Thanks for the KIP. The KIP looks good. I have a few questions and comments. > As part of the KIP-500 initiative, we need to build a bridge release version of Kafka

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hey Ismael, my understanding is that we are inevitably making the controller single point, even when the metadata quorum work is done. Redirecting won't make things easier as the controller will still be the role to perform alterations. Just for the sake of argument, DescribeTopic is not

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

I don't think these requests are necessarily infrequent under multi tenant environments though. I've seen Controller availability being an issue for describe topics for example (before it was changed to go to any broker). Would it be better to redirect once the controller quorum is there? Note

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Thanks David for the review! Great suggestion, addressed in the KIP. On Thu, Apr 16, 2020 at 7:40 AM David Jacot wrote: > Hi Boyang, > > Thanks for the KIP. Overall, it looks good to me. I really like the > envelope RPC! > > One minor comment regarding the `old-client-connections-count` metric.

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hi Boyang, Thanks for the KIP. Overall, it looks good to me. I really like the envelope RPC! One minor comment regarding the `old-client-connections-count` metric. Is it really necessary? The number of connected clients whose version is not known (prior to KIP-511) is already reported but with

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Thanks Raymond and Colin for the detailed discussions! I totally agree with the rational here. The new `Envelope` RPC has been added to the KIP and the forwarding section logic has been revised, feel free to take another look. On Wed, Apr 15, 2020 at 5:19 PM Colin McCabe wrote: > Hi Boyang, > >

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hi Boyang, I agree that we need a version bump on the request types we are going to forward. The new versions will be able to return the NOT_CONTROLLER error, and let the client do the retrying, which is what we typically prefer. The existing versions can't ever return NOT_CONTROLLER.

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hi Ismael, I agree that sending these requests through the controller will not work during the periods when there is no controller. However, those periods should be short-- otherwise we have bigger problems in the cluster. These requests are very infrequent because they are administrative

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hi Boyang, Thanks for the KIP. Have we considered that this reduces availability for these operations since we have a single Controller instead of the ZK quorum? Ismael On Fri, Apr 3, 2020 at 4:45 PM Boyang Chen wrote: > Hey all, > > I would like to start off the discussion for KIP-590, a

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Thanks Raymond for the proposal! Yea, adding a unified forwarding envelope request is a good idea, but it doesn't really solve our problem in this KIP. On Mon, Apr 13, 2020 at 11:14 PM Raymond Ng wrote: > Hi Boyang, > > Thanks for the KIP. Overall looks great. > > One suggestion: instead of

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hi Boyang, Thanks for the KIP. Overall looks great. One suggestion: instead of bumping the version and adding an optional field (PrincipalName) for a number of requests, can we consider adding a general ProxyRequest that acts as an "envelope" for the forwarded requests? A few advantages to this

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Thanks for the info Agam! Will add to the KIP. On Wed, Apr 8, 2020 at 4:26 PM Agam Brahma wrote: > Hi Boyang, > > The KIP already talks about incorporating changes for FindCoordinator > request routing, wanted to point out one additional case where internal > topics are created "as a side

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hi Boyang, The KIP already talks about incorporating changes for FindCoordinator request routing, wanted to point out one additional case where internal topics are created "as a side effect": As part of handling metadata requests, if we are looking for metadata for an internal topic and

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Thanks for the various inputs everyone! I think Sonke and Colin's suggestions make sense. The tagged field also avoids the unnecessary protocol changes for affected requests. Will add it to the header. As for the verification, I'm not sure whether it's necessary to require a higher permission

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Thanks for the KIP Boyang, this looks good to me. Some minor comments: 1) I think in order to implement the forwarding mechanism the brokers needs some purgatory to keep the forwarded requests; if that's true, should we add some broker-side metrics for those purgatories for debugging purposes?

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hi Sönke, Yeah, that was my thought too. The request has already been validated on the forwarding broker, so we don't need to validate it again. However, you make a good point that it's unfortunate that the audit log would lose the principal information if we didn't forward it as well.

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hi Boyang, thanks for the KIP. Sounds good overall. @Tom: I thought about your remark a little and think that in principle we can get away without forwarding the principal at all. Brokers currently authenticate and authorize requests before performing writes to Zookeeper - as long as we don't

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Thanks Tom for the question! I'm not super familiar with the Principal stuff, could you elaborate more on the two points you proposed here? I looked up Admin client and just take `createDelegationToken` API for an example, the request data encodes the principal information already, so broker

Re: [DISCUSS] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

Hi Boyang, Thanks for the KIP! When a broker proxies a request to the controller how does the authenticated principal get propagated? I think a couple of things might complicate this: 1. A PrincipalBuilder might be in use, 2. A Principal does not have to be serializable. Kind regards, Tom