On Mon, Aug 3, 2020, at 15:51, Jose Garcia Sancio wrote: > Thanks for the KIP Colin, > > Here is a partial review: > > > 1. Even when a broker and a controller are co-located in the same JVM, they > > must > > have different node IDs > > Why? What problem are you trying to solve? >
Hi Jose, Thanks for the comments. We did talk about this a bit earlier in the thread. The controller is always on its own port, even when it is running in the same JVM as a broker. So it would not make sense to share the same ID here-- your messages would not get through to the controller if you sent them to the broker port instead. And clearly if the controller is on a separate host from any broker, it can't share a broker id. > > > 2. Node IDs must be set in the configuration file for brokers and > > controllers. > > I understand that controller IDs must be static and in the > configuration file to be able to generate consensus in KIP-595. Why > are the broker nodes which are observers in KIP-595 cannot discover > their ID on first boot and persist their ID for consistency in future > restarts? > This is discussed in the rejected alternatives section. Basically, node ID auto-assignment is complicated and antiquated in the age of Kubernetes, Chef, Puppet, Ansible, etc. > > > 3. Controller processes will listen on a separate endpoint from brokers > > Why is this? Kafka supports multi endpoints. For example, one broker > can have one endpoint for listening to connections by other brokers > and another endpoint for connections from admin, producer and consumer > clients. > The reason for having separate ports is discussed in KIP-590. Basically, it is so that control plane traffic can be isolated from data plane traffic, as much as possible. This is the existing situation with ZooKeeper. Since ZK is on a separate port, the client cannot disrupt the cluster by flooding it with traffic (unless the admin has unwisely exposed all internal ports, but this would cause bigger security issues). We want to preserve this property. > > 4. In the case of controller RPCs like AlterIsr, the controller handles > > this by not sending > > back a response until the designated change has been persisted. > > Should we enumerate these RPCs? For example, we also have > `ListPartitionReassignments` which is a read operation and goes > directly to the controller. The naive solution would be to return the > uncommitted state in the controller. > Hmm. The KIP says that the "active controller must not make [...] future state "visible" to the rest of the cluster until it has been made persistent." So we don't return uncommitted state for read operations. > > 5. > This KIP mentions a topic named __kafka_metadata. KIP-595 and KIP-630 > mention a partition named __cluster_metadata. We should reconcile this > difference. > Jason did write that the name of the controller topic is "not a formal part of [the KIP-595] proposal." I think he wanted to avoid having the discussion about the topic name in two different KIPs. :) Let's discuss the metadata topic name here in KIP-631, and update KIP-595 as required once this one is accepted. best, Colin > > -- > -Jose >
