Hi Gwen, Thanks for taking a look. Yes, I do think unregisterBroker would be a better name than decommissionBroker. Let's do that, and also rename the associated RPC.
cheers, Colin On Sat, Feb 6, 2021, at 21:13, Gwen Shapira wrote: > I realize that I'm very very late to a pretty big party, but I was > just looking at the new code (super readable, btw), and noticed that > the inverse API for registerBroker() is called decomissionBroker(). > It would be way clearer that we are undoing a registration is we > rename it to unregisterBroker() (and the AdminClient API that matches, > to avoid more confusion). > > Any objections? > > On Fri, Dec 18, 2020 at 10:08 AM Jun Rao <j...@confluent.io> wrote: > > > > Hi, Colin, > > > > Thanks for the reply. The KIP looks good to me now. Thanks for your > > diligence. > > > > Jun > > > > On Thu, Dec 17, 2020 at 5:15 PM Colin McCabe <cmcc...@apache.org> wrote: > > > > > On Thu, Dec 17, 2020, at 17:02, Jun Rao wrote: > > > > Hi, Colin, > > > > > > > > Thanks for the reply. Sounds good. One more comment. > > > > > > > > 231. Currently, we have sasl.mechanism.inter.broker.protocol for inter > > > > broker communication. It seems that we need a similar config for > > > specifying > > > > the sasl mechanism for the communication between the broker and the > > > > controller. > > > > > > > > > > Hi Jun, > > > > > > Yeah... sounds like we could use a new configuration key for this. I > > > added sasl.mechanism.controller.protocol for this. > > > > > > regards, > > > Colin > > > > > > > Jun > > > > > > > > On Thu, Dec 17, 2020 at 2:29 PM Colin McCabe <cmcc...@apache.org> wrote: > > > > > > > > > On Thu, Dec 17, 2020, at 10:19, Jun Rao wrote: > > > > > > Hi, Colin, > > > > > > > > > > > > Thanks for the reply. > > > > > > > > > > > > 211. Hmm, I still don't see a clear benefit of registering a broker > > > > > before > > > > > > recovery. It's possible for the recovery to take time. However, > > > during > > > > > the > > > > > > recovery mode, it seems the broker will still be in the fenced mode > > > and > > > > > > won't be able to do work for the clients. So, registering and > > > > > heartbeating > > > > > > early seems to only add unnecessary overhead. For your point on > > > > > > topic > > > > > > creation, I thought we now allow replicas to be created on > > > > > > unregistered brokers. > > > > > > > > > > > > > > > > Hi Jun, > > > > > > > > > > Thanks again for the reviews. > > > > > > > > > > Topics cannot be created on unregistered brokers. They can be created > > > on > > > > > registered but fenced brokers. So for that reason I think it makes > > > sense > > > > > to register as early as possible. > > > > > > > > > > > 230. Currently, we do have a ControllerId field in MetadataResponse. > > > In > > > > > the > > > > > > early discussion, I thought that we want to expose the controller > > > > > > for > > > > > > debugging purposes, but not used by the client library. > > > > > > > > > > > > > > > > The current plan is that we will expose the controller node ID, but > > > > > the > > > > > controller will not be included in the list of nodes in the metadata > > > > > response. > > > > > > > > > > It's not really possible to include the controller in that list of > > > nodes > > > > > because the controller may not share the same set of listeners as the > > > > > broker. So, for example, maybe the controller endpoint is using a > > > > > different type of security than the broker. So while we could pass > > > back a > > > > > hostname and port, the client would have no way to connect since it > > > doesn't > > > > > know what security settings to use. > > > > > > > > > > regards, > > > > > Colin > > > > > > > > > > > Jun > > > > > > > > > > > > On Wed, Dec 16, 2020 at 9:13 PM Colin McCabe <cmcc...@apache.org> > > > wrote: > > > > > > > > > > > > > On Wed, Dec 16, 2020, at 18:13, Jun Rao wrote: > > > > > > > > Hi, Colin, > > > > > > > > > > > > > > > > Thanks for the reply. Just a couple of more comments. > > > > > > > > > > > > > > > > 211. Currently, the broker only registers itself in ZK after log > > > > > > > recovery. > > > > > > > > Is there any benefit to change that? As you mentioned, the > > > > > > > > broker > > > > > can't > > > > > > > do > > > > > > > > much before completing log recovery. > > > > > > > > > > > > > > > > > > > > > > Hi Jun, > > > > > > > > > > > > > > Previously, it wasn't possible to register in ZK without > > > immediately > > > > > > > getting added to the MetadataResponse. So I think that's the main > > > > > reason > > > > > > > why registration was delayed until after log recovery. Since that > > > > > > > constraint doesn't exist any more, there seems to be no reason to > > > delay > > > > > > > registration. > > > > > > > > > > > > > > I think delaying registration would have some major downsides. If > > > log > > > > > > > recovery takes a while, that's a longer window during which > > > > > > > someone > > > > > else > > > > > > > could register a broker with the same ID. Having parts of the > > > cluster > > > > > > > missing for a while gives up some of the benefit of separating > > > > > registration > > > > > > > from fencing. For example, if a broker somehow gets unregistered > > > and > > > > > we > > > > > > > want to re-register it, but we have to wait for a 10 minute log > > > > > recovery to > > > > > > > do that, that could be a window during which topics can't be > > > created > > > > > that > > > > > > > need to be on that broker, and so forth. > > > > > > > > > > > > > > > 230. Regarding MetadataResponse, there is a slight awkwardness. > > > We > > > > > return > > > > > > > > rack for each node. However, if that node is for the controller, > > > the > > > > > rack > > > > > > > > field is not really relevant. Should we clean it up here or in > > > > > another > > > > > > > KIP > > > > > > > > like KIP-700? > > > > > > > > > > > > > > Oh, controllers don't appear in the MetadataResponses returned to > > > > > clients, > > > > > > > since clients can't access them. I should have been more clear > > > about > > > > > that > > > > > > > in the KIP-- I added a sentence to "Networking" describing this. > > > > > > > > > > > > > > best, > > > > > > > Colin > > > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > > > Jun > > > > > > > > > > > > > > > > On Wed, Dec 16, 2020 at 4:23 PM Colin McCabe <cmcc...@apache.org > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > On Wed, Dec 16, 2020, at 13:40, Jun Rao wrote: > > > > > > > > > > Hi, Colin, > > > > > > > > > > > > > > > > > > > > Thanks for the reply. A few follow up comments. > > > > > > > > > > > > > > > > > > > > 211. When does the broker send the BrokerRegistration > > > request to > > > > > the > > > > > > > > > > controller? Is it after the recovery phase? If so, at that > > > > > point, the > > > > > > > > > > broker has already caught up on the metadata (in order to > > > clean > > > > > up > > > > > > > > > deleted > > > > > > > > > > partitions). Then, it seems we don't need the ShouldFence > > > field > > > > > > > > > > in BrokerHeartbeatRequest? > > > > > > > > > > > > > > > > > > Hi Jun, > > > > > > > > > > > > > > > > > > Thanks again for the reviews. > > > > > > > > > > > > > > > > > > The broker sends the registration request as soon as it starts > > > > > up. It > > > > > > > > > cannot wait until the recovery phase is over since sometimes > > > log > > > > > > > recovery > > > > > > > > > can take quite a long time. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 213. kafka-cluster.sh > > > > > > > > > > 213.1 For the decommision example, should the command take a > > > > > broker > > > > > > > id? > > > > > > > > > > > > > > > > > > Yes, the example should have a broker id. Fixed. > > > > > > > > > > > > > > > > > > > 213.2 Existing tools use the "--" command line option (e.g. > > > > > > > kafka-topics > > > > > > > > > > --list --topic test). Should we follow the same convention > > > > > > > > > > for kafka-cluster.sh (and kafka-storage.sh)? > > > > > > > > > > > > > > > > > > Hmm. I don't think argparse4j supports using double dashes to > > > > > identify > > > > > > > > > subcommands. I think it might be confusing as well, since the > > > > > > > subcommand > > > > > > > > > must come first, unlike a plain old argument which can be > > > anywhere > > > > > on > > > > > > > the > > > > > > > > > command line. > > > > > > > > > > > > > > > > > > > 213.3 Should we add an admin api for broker decommission so > > > that > > > > > > > this can > > > > > > > > > > be done programmatically? > > > > > > > > > > > > > > > > > > > > > > > > > > > > Yes, there is an admin client API for decommissioning as well. > > > > > > > > > > > > > > > > > > > 220. DecommissionBroker: "NOT_CONTROLLER if the node that > > > > > > > > > > the > > > > > > > request was > > > > > > > > > > sent to is not the controller". I thought the clients never > > > send > > > > > a > > > > > > > > > request > > > > > > > > > > to the controller directly now and the broker will forward > > > it to > > > > > the > > > > > > > > > > controller? > > > > > > > > > > > > > > > > > > > > > > > > > > > > If the controller moved recently, it's possible that the > > > > > > > > > broker > > > > > could > > > > > > > send > > > > > > > > > to a controller that has just recently become inactive. In > > > that > > > > > case > > > > > > > > > NOT_CONTROLLER would be returned. (A standby controller > > > returns > > > > > > > > > NOT_CONTROLLER for most APIs). > > > > > > > > > > > > > > > > > > > 221. Could we add the required ACL for the new requests? > > > > > > > > > > > > > > > > > > > > > > > > > > > > Good point. I added the required ACL for each new RPC. > > > > > > > > > > > > > > > > > > best, > > > > > > > > > Colin > > > > > > > > > > > > > > > > > > > > > > > > > > > > Jun > > > > > > > > > > > > > > > > > > > > On Wed, Dec 16, 2020 at 11:38 AM Colin McCabe < > > > > > cmcc...@apache.org> > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > On Wed, Dec 16, 2020, at 09:59, Jun Rao wrote: > > > > > > > > > > > > Hi, Colin, > > > > > > > > > > > > > > > > > > > > > > > > Thanks for the reply. A few more comments below. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Jun, > > > > > > > > > > > > > > > > > > > > > > Thanks for the comments. > > > > > > > > > > > > > > > > > > > > > > > 206. "RemoveTopic is the last step, that scrubs all > > > metadata > > > > > > > about > > > > > > > > > the > > > > > > > > > > > > topic. In order to get to that last step, the topic > > > > > > > > > > > > data > > > > > needs > > > > > > > to > > > > > > > > > > > removed > > > > > > > > > > > > from all brokers (after each broker notices that the > > > topic is > > > > > > > being > > > > > > > > > > > > deleted)." Currently, this is done based on the response > > > of > > > > > > > > > > > > StopReplicaRequest. Since the controller no longer sends > > > this > > > > > > > > > request, > > > > > > > > > > > how > > > > > > > > > > > > does the controller know that the data for the deleted > > > topic > > > > > has > > > > > > > > > > > > been removed in the brokers? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > That's a good point... definitely an oversight. > > > > > > > > > > > > > > > > > > > > > > It seems complex to force the controller to track when log > > > > > > > directories > > > > > > > > > > > have been deleted. Let's just assume that KIP-516 has > > > > > > > > > > > been > > > > > > > > > implemented, > > > > > > > > > > > and track them by UUID. Then we can just have a single > > > topic > > > > > > > deletion > > > > > > > > > > > record. > > > > > > > > > > > > > > > > > > > > > > I added a section on "topic identifiers" describing this. > > > > > > > > > > > > > > > > > > > > > > > 210. Thanks for the explanation. Sounds good to me. > > > > > > > > > > > > > > > > > > > > > > > > 211. I still don't see an example when ShouldFence is > > > set to > > > > > > > true in > > > > > > > > > > > > BrokerHeartbeatReques. Could we add one? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > It is sent to true when the broker is first starting up > > > > > > > > > > > and > > > > > > > doesn't yet > > > > > > > > > > > want to be unfenced. I added a longer explanation of this > > > in > > > > > the > > > > > > > > > "Broker > > > > > > > > > > > Leases" section. > > > > > > > > > > > > > > > > > > > > > > > 213. The KIP now allows replicas to be assigned on > > > brokers > > > > > that > > > > > > > are > > > > > > > > > > > fenced, > > > > > > > > > > > > which is an improvement. How do we permanently remove a > > > > > broker > > > > > > > (e.g. > > > > > > > > > > > > cluster shrinking) to prevent it from being used for > > > future > > > > > > > replica > > > > > > > > > > > > assignments? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > This is a fair point. I will create a kafka-cluster.sh > > > script > > > > > > > which > > > > > > > > > can > > > > > > > > > > > do this, plus a DecommissionBrokerRequest. > > > > > > > > > > > > > > > > > > > > > > As a bonus the kafka-cluster.sh script can help people > > > find the > > > > > > > > > cluster ID > > > > > > > > > > > of brokers, something that people have wanted a tool for > > > in the > > > > > > > past. > > > > > > > > > > > > > > > > > > > > > > > 214. "Currently, when a node is down, all of its ZK > > > > > registration > > > > > > > > > > > > information is gone. But we need this information in > > > order > > > > > to > > > > > > > > > > > understand > > > > > > > > > > > > things like whether the replicas of a particular > > > partition > > > > > are > > > > > > > > > > > > well-balanced across racks." This is not quite true. > > > > > Currently, > > > > > > > even > > > > > > > > > when > > > > > > > > > > > > ZK registration is gone, the existing replica assignment > > > is > > > > > still > > > > > > > > > > > available > > > > > > > > > > > > in the metadata response. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I agree that the existing replica assignment is still > > > > > available. > > > > > > > But > > > > > > > > > that > > > > > > > > > > > just tells you partition X is on nodes A, B, and C. If > > > > > > > > > > > you > > > > > don't > > > > > > > have > > > > > > > > > the > > > > > > > > > > > ZK registration for one or more of A, B, or C then you > > > don't > > > > > know > > > > > > > > > whether > > > > > > > > > > > we are following the policy of "two replicas on one rack, > > > one > > > > > > > replica > > > > > > > > > on > > > > > > > > > > > another." Or any other more complex rack placement policy > > > > > that you > > > > > > > > > might > > > > > > > > > > > have. > > > > > > > > > > > > > > > > > > > > > > best, > > > > > > > > > > > Colin > > > > > > > > > > > > > > > > > > > > > > > Jun > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Dec 16, 2020 at 8:48 AM Colin McCabe < > > > > > cmcc...@apache.org > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, Dec 15, 2020, at 13:08, Jun Rao wrote: > > > > > > > > > > > > > > Hi, Colin, > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks for the reply. A few more follow up comments. > > > > > > > > > > > > > > > > > > > > > > > > > > > > 210. initial.broker.registration.timeout.ms: The > > > default > > > > > > > value > > > > > > > > > is > > > > > > > > > > > 90sec, > > > > > > > > > > > > > > which seems long. If a broker fails the registration > > > > > because > > > > > > > of > > > > > > > > > > > incorrect > > > > > > > > > > > > > > configs, we want to fail faster. In comparison, the > > > > > defaults > > > > > > > for > > > > > > > > > > > > > > zookeeper.connection.timeout.ms is 18 secs. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Jun, > > > > > > > > > > > > > > > > > > > > > > > > > > I agree that the initial connection timeout here is > > > longer > > > > > than > > > > > > > > > what we > > > > > > > > > > > > > had with ZK. The main reason I selected a slightly > > > longer > > > > > > > timeout > > > > > > > > > > > here is > > > > > > > > > > > > > to handle the case where the controllers and the > > > brokers > > > > > are > > > > > > > > > > > co-located. > > > > > > > > > > > > > For example, if you have a 3 node cluster and all > > > > > > > > > > > > > three > > > > > nodes > > > > > > > are > > > > > > > > > > > > > controllers+brokers, when you first bring up the > > > cluster, > > > > > we > > > > > > > will > > > > > > > > > have > > > > > > > > > > > to > > > > > > > > > > > > > stand up the controller quorum and then handle broker > > > > > > > > > registrations. > > > > > > > > > > > > > Although we believe Raft will start up relatively > > > quickly, > > > > > it's > > > > > > > > > good to > > > > > > > > > > > > > leave some extra margin here. > > > > > > > > > > > > > > > > > > > > > > > > > > I don't think there's a big disadvantage to having a > > > > > slightly > > > > > > > > > longer > > > > > > > > > > > > > timeout here. After all, starting up the brokers with > > > no > > > > > > > > > controllers > > > > > > > > > > > is an > > > > > > > > > > > > > admin mistake, which we don't expect to see very > > > > > > > > > > > > > often. > > > > > Maybe > > > > > > > > > let's > > > > > > > > > > > set it > > > > > > > > > > > > > to 60 seconds for now and maybe see if we can tweak it > > > in > > > > > the > > > > > > > > > future if > > > > > > > > > > > > > that turns out to be too long or short? > > > > > > > > > > > > > > > > > > > > > > > > > > > 211. "Once they are all moved, the controller > > > responds > > > > > to the > > > > > > > > > > > heartbeat > > > > > > > > > > > > > > with a nextState of SHUTDOWN." It seems that > > > nextState > > > > > is no > > > > > > > > > longer > > > > > > > > > > > > > > relevant. Also, could you add a bit more explanation > > > on > > > > > when > > > > > > > > > > > ShouldFence > > > > > > > > > > > > > is > > > > > > > > > > > > > > set to true in BrokerHeartbeatRequest? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Good catch. I removed the obsolete section referring > > > to > > > > > > > nextState > > > > > > > > > and > > > > > > > > > > > > > added a reference to the new boolean. I also added > > > some > > > > > more > > > > > > > > > > > information > > > > > > > > > > > > > about ShouldFence and about the rationale for > > > separating > > > > > > > fencing > > > > > > > > > from > > > > > > > > > > > > > registration. > > > > > > > > > > > > > > > > > > > > > > > > > > > 212. Related to the above, what does the broker do > > > > > > > > > > > > > > if > > > > > > > IsFenced is > > > > > > > > > > > true in > > > > > > > > > > > > > > the BrokerHeartbeatResponse? Will the broker do the > > > same > > > > > > > thing on > > > > > > > > > > > > > receiving > > > > > > > > > > > > > > a FenceBrokerRecord from the metadata log? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The broker only checks this boolean during the startup > > > > > process. > > > > > > > > > After > > > > > > > > > > > the > > > > > > > > > > > > > startup process is finished, it ignores this boolean. > > > > > > > > > > > > > > > > > > > > > > > > > > The broker uses fence / unfence records in the > > > > > > > > > > > > > metadata > > > > > log to > > > > > > > > > > > determine > > > > > > > > > > > > > which brokers should appear in its MetadataResponse. > > > > > > > > > > > > > > > > > > > > > > > > > > best, > > > > > > > > > > > > > Colin > > > > > > > > > > > > > > > > > > > > > > > > > > > Jun > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, Dec 15, 2020 at 8:51 AM Colin McCabe < > > > > > > > cmcc...@apache.org > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, Dec 15, 2020, at 04:13, Tom Bentley wrote: > > > > > > > > > > > > > > > > Hi Colin, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The KIP says that "brokers which are fenced will > > > not > > > > > > > appear > > > > > > > > > in > > > > > > > > > > > > > > > > MetadataResponses. So clients that have > > > up-to-date > > > > > > > metadata > > > > > > > > > will > > > > > > > > > > > > > not try > > > > > > > > > > > > > > > > to contact fenced brokers.", which is fine, but > > > it > > > > > > > doesn't > > > > > > > > > seem > > > > > > > > > > > to > > > > > > > > > > > > > > > > elaborate on what happens for clients which try > > > to > > > > > > > contact a > > > > > > > > > > > broker > > > > > > > > > > > > > > > (using > > > > > > > > > > > > > > > > stale metadata, for example) and find it in the > > > > > > > > > > > registered-but-fenced > > > > > > > > > > > > > > > > state. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Tom, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I have dropped the broker-side fencing from this > > > > > > > proposal. So > > > > > > > > > now > > > > > > > > > > > the > > > > > > > > > > > > > > > fencing is basically the same as today's fencing: > > > it's > > > > > > > > > > > controller-side > > > > > > > > > > > > > > > only. That means that clients using stale > > > metadata can > > > > > > > contact > > > > > > > > > > > fenced > > > > > > > > > > > > > > > brokers and communicate with them. The only case > > > > > where the > > > > > > > > > broker > > > > > > > > > > > > > will be > > > > > > > > > > > > > > > inaccessible is when it hasn't finished starting > > > yet > > > > > > > (i.e., has > > > > > > > > > > > not yet > > > > > > > > > > > > > > > attained RUNNING state.) > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Just like today, we expect the safeguards built > > > into > > > > > the > > > > > > > > > > > replication > > > > > > > > > > > > > > > protocol to prevent the worst corner cases that > > > could > > > > > > > result > > > > > > > > > from > > > > > > > > > > > > > this. I > > > > > > > > > > > > > > > do think we will probably take up this issue later > > > and > > > > > > > find a > > > > > > > > > > > better > > > > > > > > > > > > > > > solution for client-side metadata, but this KIP is > > > big > > > > > > > enough > > > > > > > > > > > as-is. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > best, > > > > > > > > > > > > > > > Colin > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > You said previously that the broker will respond > > > > > with a > > > > > > > > > retriable > > > > > > > > > > > > > > > > error, which is again fine, but you didn't > > > answer my > > > > > > > question > > > > > > > > > > > about > > > > > > > > > > > > > which > > > > > > > > > > > > > > > > error code you've chosen for this. I realise > > > > > > > > > > > > > > > > that > > > > > this > > > > > > > > > doesn't > > > > > > > > > > > really > > > > > > > > > > > > > > > > affect the correctness of the behaviour, but I'm > > > not > > > > > > > aware > > > > > > > > > of any > > > > > > > > > > > > > > > existing > > > > > > > > > > > > > > > > error code which is a good fit. So it would be > > > good > > > > > to > > > > > > > > > understand > > > > > > > > > > > > > about > > > > > > > > > > > > > > > how > > > > > > > > > > > > > > > > you're making this backward compatible for > > > clients. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Many thanks, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Tom > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, Dec 15, 2020 at 1:42 AM Colin McCabe < > > > > > > > > > cmcc...@apache.org > > > > > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Fri, Dec 11, 2020, at 17:07, Jun Rao wrote: > > > > > > > > > > > > > > > > > > Hi, Colin, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks for the reply. Just a couple of more > > > > > comments > > > > > > > > > below. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 210. Since we are deprecating > > > > > > > > > > > zookeeper.connection.timeout.ms, > > > > > > > > > > > > > > > should we > > > > > > > > > > > > > > > > > > add a new config to bound the time for a > > > broker > > > > > to > > > > > > > > > connect > > > > > > > > > > > to the > > > > > > > > > > > > > > > > > > controller during starting up? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Good idea. I added > > > > > > > initial.broker.registration.timeout.ms > > > > > > > > > for > > > > > > > > > > > > > this. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 211. BrokerHeartbeat no longer has the state > > > > > field > > > > > > > in the > > > > > > > > > > > > > > > > > request/response. > > > > > > > > > > > > > > > > > > However, (a) the controller shutdown section > > > > > still > > > > > > > has > > > > > > > > > "In > > > > > > > > > > > its > > > > > > > > > > > > > > > periodic > > > > > > > > > > > > > > > > > > heartbeats, the broker asks the controller > > > if it > > > > > can > > > > > > > > > > > transition > > > > > > > > > > > > > into > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > SHUTDOWN state. This motivates the > > > controller to > > > > > > > move > > > > > > > > > all > > > > > > > > > > > of the > > > > > > > > > > > > > > > leaders > > > > > > > > > > > > > > > > > > off of that broker. Once they are all > > > moved, the > > > > > > > > > controller > > > > > > > > > > > > > > > responds to > > > > > > > > > > > > > > > > > > the heartbeat with a nextState of > > > > > > > > > > > > > > > > > > SHUTDOWN."; > > > > > (2) the > > > > > > > > > > > > > description of > > > > > > > > > > > > > > > > > > BrokerHeartbeat still references > > > currentState and > > > > > > > > > > > targetState. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks. I've made these sections clearer and > > > > > removed > > > > > > > the > > > > > > > > > > > obsolete > > > > > > > > > > > > > > > > > references to sending states. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > best, > > > > > > > > > > > > > > > > > Colin > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Jun > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Fri, Dec 11, 2020 at 1:33 PM Colin McCabe > > > < > > > > > > > > > > > cmcc...@apache.org > > > > > > > > > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Dec 9, 2020, at 10:10, Jun Rao > > > wrote: > > > > > > > > > > > > > > > > > > > > Hi, Colin, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks for the update. A few more follow > > > up > > > > > > > comments. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Jun, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks again for the review. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 100. FailedReplicaRecord: Since this is > > > > > reported > > > > > > > by > > > > > > > > > each > > > > > > > > > > > > > broker > > > > > > > > > > > > > > > > > > > > independently, perhaps we could use a > > > more > > > > > > > concise > > > > > > > > > > > > > representation > > > > > > > > > > > > > > > > > that > > > > > > > > > > > > > > > > > > > has > > > > > > > > > > > > > > > > > > > > a top level broker field, an array of > > > topics, > > > > > > > which > > > > > > > > > has > > > > > > > > > > > an > > > > > > > > > > > > > array > > > > > > > > > > > > > > > of > > > > > > > > > > > > > > > > > > > > partitions. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The issue is that there is a size limit on > > > the > > > > > each > > > > > > > > > record. > > > > > > > > > > > > > > > Putting > > > > > > > > > > > > > > > > > all > > > > > > > > > > > > > > > > > > > of the partitions of a log directory into > > > > > > > > > > > > > > > > > > > a > > > > > single > > > > > > > > > record > > > > > > > > > > > would > > > > > > > > > > > > > > > > > probably > > > > > > > > > > > > > > > > > > > break that in many cases. Still, we can > > > > > optimize a > > > > > > > > > bit by > > > > > > > > > > > > > having > > > > > > > > > > > > > > > an > > > > > > > > > > > > > > > > > array > > > > > > > > > > > > > > > > > > > of partition IDs, since nearly all the > > > time, we > > > > > > > have > > > > > > > > > more > > > > > > > > > > > than > > > > > > > > > > > > > one > > > > > > > > > > > > > > > > > from the > > > > > > > > > > > > > > > > > > > same topic. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 200. Sounds good. If we remove the > > > > > broker-side > > > > > > > > > fencing > > > > > > > > > > > > > logic, do > > > > > > > > > > > > > > > we > > > > > > > > > > > > > > > > > plan > > > > > > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > > > > still keep FENCED in broker state? Do we > > > > > plan to > > > > > > > > > expose > > > > > > > > > > > the > > > > > > > > > > > > > new > > > > > > > > > > > > > > > > > states > > > > > > > > > > > > > > > > > > > > through the existing BrokerState metric > > > and > > > > > if > > > > > > > so, > > > > > > > > > what > > > > > > > > > > > are > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > values > > > > > > > > > > > > > > > > > > > for > > > > > > > > > > > > > > > > > > > > the new states? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > No, we don't need FENCED any more. I have > > > > > removed > > > > > > > it > > > > > > > > > from > > > > > > > > > > > the > > > > > > > > > > > > > KIP. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The new states are very similar to the > > > current > > > > > > > ones, > > > > > > > > > > > actually. > > > > > > > > > > > > > > > There > > > > > > > > > > > > > > > > > are > > > > > > > > > > > > > > > > > > > no new states or removed ones. The main > > > > > change in > > > > > > > the > > > > > > > > > > > broker > > > > > > > > > > > > > state > > > > > > > > > > > > > > > > > machine > > > > > > > > > > > > > > > > > > > is that the > > > RECOVERING_FROM_UNCLEAN_SHUTDOWN > > > > > state > > > > > > > has > > > > > > > > > been > > > > > > > > > > > > > > > renamed to > > > > > > > > > > > > > > > > > > > RECOVERY. Also, unlike previously, the > > > broker > > > > > will > > > > > > > > > always > > > > > > > > > > > pass > > > > > > > > > > > > > > > through > > > > > > > > > > > > > > > > > > > RECOVERY (although it may only stay in > > > > > > > > > > > > > > > > > > > this > > > > > state > > > > > > > for a > > > > > > > > > > > very > > > > > > > > > > > > > short > > > > > > > > > > > > > > > > > amount > > > > > > > > > > > > > > > > > > > of time). > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 201. This may be fine too. Could we > > > document > > > > > what > > > > > > > > > happens > > > > > > > > > > > > > when > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > > broker.id/controller.id in > > > > > metadata.properties > > > > > > > don't > > > > > > > > > > > match > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > broker > > > > > > > > > > > > > > > > > > > > config when the broker starts up? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I added some documentation about this. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 204. There is still "The highest > > > > > > > > > > > > > > > > > > > > metadata > > > > > offset > > > > > > > > > which > > > > > > > > > > > the > > > > > > > > > > > > > broker > > > > > > > > > > > > > > > > > has not > > > > > > > > > > > > > > > > > > > > reached" referenced under > > > BrokerRegistration. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > It should be CurrentMetadataOffset. > > > > > > > > > > > > > > > > > > > Fixed. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 206. Is that separate step needed given > > > > > KIP-516? > > > > > > > With > > > > > > > > > > > > > KIP-516 ( > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-516%3A+Topic+Identifiers#KIP516:TopicIdentifiers-LeaderAndIsr > > > > > > > > > > > > > > > > > > > ), > > > > > > > > > > > > > > > > > > > > we don't need to wait for the topic data > > > to > > > > > be > > > > > > > > > removed > > > > > > > > > > > from > > > > > > > > > > > > > all > > > > > > > > > > > > > > > > > brokers > > > > > > > > > > > > > > > > > > > > before removing the topic metadata. The > > > > > > > combination > > > > > > > > > of > > > > > > > > > > > > > unmatching > > > > > > > > > > > > > > > > > > > > topicId > > > > > > > > > > > > > > > > > > > > or the missing topicId from the metadata > > > is > > > > > > > enough > > > > > > > > > for > > > > > > > > > > > the > > > > > > > > > > > > > > > broker to > > > > > > > > > > > > > > > > > > > > clean > > > > > > > > > > > > > > > > > > > > up deleted topics asynchronously. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > It won't be needed once KIP-516 is > > > adopted, but > > > > > > > this > > > > > > > > > hasn't > > > > > > > > > > > > > been > > > > > > > > > > > > > > > > > > > implemented yet. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > best, > > > > > > > > > > > > > > > > > > > Colin > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Jun > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, Dec 8, 2020 at 5:27 PM Colin > > > McCabe < > > > > > > > > > > > > > cmcc...@apache.org> > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Thu, Dec 3, 2020, at 16:37, Jun Rao > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > Hi, Colin, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks for the updated KIP. A few > > > more > > > > > > > comments > > > > > > > > > > > below. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Jun, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks again for the reviews. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 80.2 For deprecated configs, we need > > > to > > > > > > > include > > > > > > > > > > > > > zookeeper.* > > > > > > > > > > > > > > > and > > > > > > > > > > > > > > > > > > > > > > broker.id.generation.enable. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Added. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 83.1 If a broker is down, does the > > > > > controller > > > > > > > > > keep > > > > > > > > > > > the > > > > > > > > > > > > > > > previously > > > > > > > > > > > > > > > > > > > > > > registered broker epoch forever? If > > > not, > > > > > how > > > > > > > long > > > > > > > > > > > does > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > controller > > > > > > > > > > > > > > > > > > > > > keep > > > > > > > > > > > > > > > > > > > > > > it? What does the controller do when > > > > > > > receiving a > > > > > > > > > > > broker > > > > > > > > > > > > > > > heartbeat > > > > > > > > > > > > > > > > > > > request > > > > > > > > > > > > > > > > > > > > > > with an unfound broker epoch? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Yes, the controller keeps the previous > > > > > > > registration > > > > > > > > > > > > > forever. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Broker heartbeat requests with an > > > incorrect > > > > > > > broker > > > > > > > > > > > epoch > > > > > > > > > > > > > will > > > > > > > > > > > > > > > be > > > > > > > > > > > > > > > > > > > rejected > > > > > > > > > > > > > > > > > > > > > with STALE_BROKER_EPOCH. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 100. Have you figured out if we need > > > to > > > > > add > > > > > > > a new > > > > > > > > > > > record > > > > > > > > > > > > > > > type for > > > > > > > > > > > > > > > > > > > > > reporting > > > > > > > > > > > > > > > > > > > > > > partitions on failed disks? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I added FailedReplicaRecord to reflect > > > the > > > > > case > > > > > > > > > where a > > > > > > > > > > > > > JBOD > > > > > > > > > > > > > > > > > directory > > > > > > > > > > > > > > > > > > > has > > > > > > > > > > > > > > > > > > > > > failed, leading to failed replicas. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 102. For debugging purposes, > > > sometimes > > > > > it's > > > > > > > > > useful to > > > > > > > > > > > > > read > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > metadata > > > > > > > > > > > > > > > > > > > > > > topic using tools like > > > console-consumer. > > > > > > > Should > > > > > > > > > we > > > > > > > > > > > > > support > > > > > > > > > > > > > > > that > > > > > > > > > > > > > > > > > and > > > > > > > > > > > > > > > > > > > if > > > > > > > > > > > > > > > > > > > > > so, > > > > > > > > > > > > > > > > > > > > > > how? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > For now, we have the ability to read > > > the > > > > > > > metadata > > > > > > > > > logs > > > > > > > > > > > > > with the > > > > > > > > > > > > > > > > > > > dump-logs > > > > > > > > > > > > > > > > > > > > > tool. I think we will come up with > > > some > > > > > other > > > > > > > > > tools > > > > > > > > > > > in the > > > > > > > > > > > > > > > future > > > > > > > > > > > > > > > > > as > > > > > > > > > > > > > > > > > > > we > > > > > > > > > > > > > > > > > > > > > get experience. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 200. "brokers which are fenced will > > > not > > > > > > > appear in > > > > > > > > > > > > > > > > > MetadataResponses. > > > > > > > > > > > > > > > > > > > The > > > > > > > > > > > > > > > > > > > > > > broker will not respond to these > > > > > requests-- > > > > > > > > > instead, > > > > > > > > > > > it > > > > > > > > > > > > > will > > > > > > > > > > > > > > > > > simply > > > > > > > > > > > > > > > > > > > > > > disconnect." If the controller is > > > > > > > partitioned off > > > > > > > > > > > from > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > brokers, > > > > > > > > > > > > > > > > > > > this > > > > > > > > > > > > > > > > > > > > > > design will cause every broker to > > > stop > > > > > > > accepting > > > > > > > > > new > > > > > > > > > > > > > client > > > > > > > > > > > > > > > > > > > requests. In > > > > > > > > > > > > > > > > > > > > > > contrast, if ZK is partitioned off, > > > the > > > > > > > existing > > > > > > > > > > > > > behavior is > > > > > > > > > > > > > > > > > that the > > > > > > > > > > > > > > > > > > > > > > brokers can continue to work based > > > on the > > > > > > > last > > > > > > > > > known > > > > > > > > > > > > > > > metadata. > > > > > > > > > > > > > > > > > So, I > > > > > > > > > > > > > > > > > > > am > > > > > > > > > > > > > > > > > > > > > not > > > > > > > > > > > > > > > > > > > > > > sure if we should change the > > > > > > > > > > > > > > > > > > > > > > existing > > > > > > > behavior > > > > > > > > > > > because > > > > > > > > > > > > > of the > > > > > > > > > > > > > > > > > bigger > > > > > > > > > > > > > > > > > > > > > impact > > > > > > > > > > > > > > > > > > > > > > in the new one. Another option is to > > > > > keep the > > > > > > > > > > > existing > > > > > > > > > > > > > > > behavior > > > > > > > > > > > > > > > > > and > > > > > > > > > > > > > > > > > > > > > expose > > > > > > > > > > > > > > > > > > > > > > a metric for fenced brokers so that > > > the > > > > > > > operator > > > > > > > > > > > could be > > > > > > > > > > > > > > > > > alerted. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I'm skeptical about how well running > > > > > without ZK > > > > > > > > > > > currently > > > > > > > > > > > > > > > works. > > > > > > > > > > > > > > > > > > > However, > > > > > > > > > > > > > > > > > > > > > I will move the broker-side fencing > > > into a > > > > > > > > > follow-up > > > > > > > > > > > KIP. > > > > > > > > > > > > > This > > > > > > > > > > > > > > > > > KIP is > > > > > > > > > > > > > > > > > > > > > already pretty large and there is no > > > hard > > > > > > > > > dependency on > > > > > > > > > > > > > this. > > > > > > > > > > > > > > > > > There > > > > > > > > > > > > > > > > > > > may > > > > > > > > > > > > > > > > > > > > > also be other ways of accomplishing > > > > > > > > > > > > > > > > > > > > > the > > > > > > > positive > > > > > > > > > > > effects of > > > > > > > > > > > > > > > what > > > > > > > > > > > > > > > > > > > > > broker-side fencing, so more > > > discussion is > > > > > > > needed. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 201. I read Ron's comment, but I am > > > > > still not > > > > > > > > > sure > > > > > > > > > > > the > > > > > > > > > > > > > > > benefit of > > > > > > > > > > > > > > > > > > > keeping > > > > > > > > > > > > > > > > > > > > > > broker.id and controller.id in > > > > > > > meta.properties. > > > > > > > > > It > > > > > > > > > > > seems > > > > > > > > > > > > > > > that > > > > > > > > > > > > > > > > > we are > > > > > > > > > > > > > > > > > > > > > just > > > > > > > > > > > > > > > > > > > > > > duplicating the same info in two > > > places > > > > > and > > > > > > > have > > > > > > > > > the > > > > > > > > > > > > > > > additional > > > > > > > > > > > > > > > > > > > burden of > > > > > > > > > > > > > > > > > > > > > > making sure the values in the two > > > places > > > > > are > > > > > > > > > > > consistent. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I think the reasoning is that having > > > > > broker.id > > > > > > > > > > > protects us > > > > > > > > > > > > > > > against > > > > > > > > > > > > > > > > > > > > > accidentally bringing up a broker with > > > a > > > > > disk > > > > > > > from > > > > > > > > > a > > > > > > > > > > > > > different > > > > > > > > > > > > > > > > > > > broker. I > > > > > > > > > > > > > > > > > > > > > don't feel strongly about this but it > > > > > seemed > > > > > > > > > simpler to > > > > > > > > > > > > > keep > > > > > > > > > > > > > > > it. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 202. > > > > > controller.connect.security.protocol: Is > > > > > > > > > this > > > > > > > > > > > needed > > > > > > > > > > > > > > > since > > > > > > > > > > > > > > > > > > > > > > controller.listener.names and > > > > > > > > > > > > > listener.security.protocol.map > > > > > > > > > > > > > > > > > imply > > > > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > > > > security protocol already? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > You're right, this isn't needed. I'll > > > > > remove > > > > > > > it. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 203. > > > registration.heartbeat.interval.ms: > > > > > It > > > > > > > > > > > defaults to > > > > > > > > > > > > > 2k. > > > > > > > > > > > > > > > ZK > > > > > > > > > > > > > > > > > uses > > > > > > > > > > > > > > > > > > > 1/3 > > > > > > > > > > > > > > > > > > > > > of > > > > > > > > > > > > > > > > > > > > > > the session timeout for heartbeat. > > > So, > > > > > given > > > > > > > the > > > > > > > > > > > default > > > > > > > > > > > > > 18k > > > > > > > > > > > > > > > for > > > > > > > > > > > > > > > > > > > > > > registration.lease.timeout.ms, > > > should we > > > > > > > default > > > > > > > > > > > > > > > > > > > > > > registration.heartbeat.interval.ms > > > to > > > > > 6k? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 6 seconds seems like a pretty long > > > > > > > > > > > > > > > > > > > > > time > > > > > between > > > > > > > > > > > > > heartbeats. It > > > > > > > > > > > > > > > > > might > > > > > > > > > > > > > > > > > > > be > > > > > > > > > > > > > > > > > > > > > useful to know when a broker is > > > > > > > > > > > > > > > > > > > > > missing > > > > > > > heartbeats, > > > > > > > > > > > with > > > > > > > > > > > > > less > > > > > > > > > > > > > > > time > > > > > > > > > > > > > > > > > than > > > > > > > > > > > > > > > > > > > > > that. I provisionally set it to 3 > > > seconds > > > > > (we > > > > > > > can > > > > > > > > > > > always > > > > > > > > > > > > > > > change > > > > > > > > > > > > > > > > > > > later...) > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I also changed the name of these > > > > > > > configurations to > > > > > > > > > " > > > > > > > > > > > > > > > > > > > > > broker.heartbeat.interval.ms" and " > > > > > > > > > > > > > > > broker.registration.timeout.ms" > > > > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > > > try > > > > > > > > > > > > > > > > > > > > > to clarify them a bit. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 204. "The highest metadata offset > > > which > > > > > the > > > > > > > > > broker > > > > > > > > > > > has > > > > > > > > > > > > > not > > > > > > > > > > > > > > > > > reached." > > > > > > > > > > > > > > > > > > > It > > > > > > > > > > > > > > > > > > > > > > seems this should be "has reached". > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I changed this to "one more than the > > > > > highest > > > > > > > > > metadata > > > > > > > > > > > > > offset > > > > > > > > > > > > > > > which > > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > > > broker has reached." > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 205. UnfenceBrokerRecord and > > > > > > > > > UnregisterBrokerRecord: > > > > > > > > > > > To > > > > > > > > > > > > > me, > > > > > > > > > > > > > > > they > > > > > > > > > > > > > > > > > > > seem to > > > > > > > > > > > > > > > > > > > > > be > > > > > > > > > > > > > > > > > > > > > > the same. Do we need both? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Unregistration means that the broker > > > has > > > > > been > > > > > > > > > removed > > > > > > > > > > > from > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > cluster. > > > > > > > > > > > > > > > > > > > > > That is different than unfencing, > > > > > > > > > > > > > > > > > > > > > which > > > > > marks > > > > > > > the > > > > > > > > > > > broker as > > > > > > > > > > > > > > > active. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 206. TopicRecord: The Deleting field > > > is > > > > > used > > > > > > > to > > > > > > > > > > > indicate > > > > > > > > > > > > > > > that the > > > > > > > > > > > > > > > > > > > topic > > > > > > > > > > > > > > > > > > > > > is > > > > > > > > > > > > > > > > > > > > > > being deleted. I am wondering if > > > this is > > > > > > > really > > > > > > > > > > > needed > > > > > > > > > > > > > since > > > > > > > > > > > > > > > > > > > RemoveTopic > > > > > > > > > > > > > > > > > > > > > > already indicates the same thing. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > RemoveTopic is the last step, that > > > scrubs > > > > > all > > > > > > > > > metadata > > > > > > > > > > > > > about > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > topic. > > > > > > > > > > > > > > > > > > > > > In order to get to that last step, the > > > > > topic > > > > > > > data > > > > > > > > > > > needs to > > > > > > > > > > > > > > > removed > > > > > > > > > > > > > > > > > > > from all > > > > > > > > > > > > > > > > > > > > > brokers (after each broker notices > > > that the > > > > > > > topic > > > > > > > > > is > > > > > > > > > > > being > > > > > > > > > > > > > > > > > deleted). > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > best, > > > > > > > > > > > > > > > > > > > > > Colin > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Jun > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Dec 2, 2020 at 2:50 PM Colin > > > > > McCabe < > > > > > > > > > > > > > > > cmcc...@apache.org> > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Dec 2, 2020, at 14:07, Ron > > > > > > > Dagostino > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > Hi Colin. Thanks for the > > > updates. > > > > > It's > > > > > > > now > > > > > > > > > > > clear > > > > > > > > > > > > > to me > > > > > > > > > > > > > > > that > > > > > > > > > > > > > > > > > > > brokers > > > > > > > > > > > > > > > > > > > > > > > > keep their broker epoch for the > > > life > > > > > of > > > > > > > their > > > > > > > > > > > JVM -- > > > > > > > > > > > > > they > > > > > > > > > > > > > > > > > > > register > > > > > > > > > > > > > > > > > > > > > > > > once, get their broker epoch in > > > the > > > > > > > > > response, and > > > > > > > > > > > > > then > > > > > > > > > > > > > > > never > > > > > > > > > > > > > > > > > > > > > > > > re-register again. Brokers may > > > get > > > > > > > fenced, > > > > > > > > > but > > > > > > > > > > > they > > > > > > > > > > > > > > > keep the > > > > > > > > > > > > > > > > > > > same > > > > > > > > > > > > > > > > > > > > > > > > broker epoch for the life of > > > their > > > > > JVM. > > > > > > > The > > > > > > > > > > > > > incarnation > > > > > > > > > > > > > > > ID > > > > > > > > > > > > > > > > > is > > > > > > > > > > > > > > > > > > > also > > > > > > > > > > > > > > > > > > > > > > > > kept for the life of the JVM but > > > is > > > > > > > > > generated by > > > > > > > > > > > the > > > > > > > > > > > > > > > broker > > > > > > > > > > > > > > > > > > > itself > > > > > > > > > > > > > > > > > > > > > > > > upon startup, and the > > > combination of > > > > > the > > > > > > > two > > > > > > > > > > > allows > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > Controller to > > > > > > > > > > > > > > > > > > > > > > > > act idempotently if any > > > > > previously-sent > > > > > > > > > > > registration > > > > > > > > > > > > > > > response > > > > > > > > > > > > > > > > > > > gets > > > > > > > > > > > > > > > > > > > > > > > > lost. Makes sense. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks, Ron. That's a good > > > summary. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > One thing I wonder about is if > > > > > > > > > > > > > > > > > > > > > > > > it > > > > > might > > > > > > > be > > > > > > > > > > > helpful > > > > > > > > > > > > > for > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > broker to > > > > > > > > > > > > > > > > > > > > > > > > send the Cluster ID as > > > > > > > > > > > > > > > > > > > > > > > > determined > > > > > from > > > > > > > its > > > > > > > > > > > > > > > meta.properties > > > > > > > > > > > > > > > > > file > > > > > > > > > > > > > > > > > > > in > > > > > > > > > > > > > > > > > > > > > its > > > > > > > > > > > > > > > > > > > > > > > > registration request. Does it > > > even > > > > > make > > > > > > > > > sense > > > > > > > > > > > for > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > broker to > > > > > > > > > > > > > > > > > > > > > > > > successfully register and enter > > > the > > > > > > > Fenced > > > > > > > > > state > > > > > > > > > > > if > > > > > > > > > > > > > it > > > > > > > > > > > > > > > has > > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > wrong > > > > > > > > > > > > > > > > > > > > > > > > Cluster ID? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Yeah, that's a good idea. Let's > > > have > > > > > the > > > > > > > > > broker > > > > > > > > > > > pass > > > > > > > > > > > > > its > > > > > > > > > > > > > > > > > cluster > > > > > > > > > > > > > > > > > > > ID in > > > > > > > > > > > > > > > > > > > > > > > the registration RPC, and then > > > > > > > registration can > > > > > > > > > > > fail > > > > > > > > > > > > > if the > > > > > > > > > > > > > > > > > broker > > > > > > > > > > > > > > > > > > > is > > > > > > > > > > > > > > > > > > > > > > > configured for the wrong cluster. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The nextMetadatOffset value > > > that the > > > > > > > broker > > > > > > > > > > > > > communicates > > > > > > > > > > > > > > > > > > > > > > > > in its registration request only > > > has > > > > > > > meaning > > > > > > > > > > > within > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > correct > > > > > > > > > > > > > > > > > > > > > > > > cluster, so it feels to me that > > > the > > > > > > > > > Controller > > > > > > > > > > > should > > > > > > > > > > > > > > > have > > > > > > > > > > > > > > > > > some > > > > > > > > > > > > > > > > > > > way > > > > > > > > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > > > > > > > > perform this sanity check. > > > There is > > > > > > > > > currently > > > > > > > > > > > > > (pre-KIP > > > > > > > > > > > > > > > 500) > > > > > > > > > > > > > > > > > a > > > > > > > > > > > > > > > > > > > check > > > > > > > > > > > > > > > > > > > > > > > > in the broker to make sure its > > > > > configured > > > > > > > > > > > cluster ID > > > > > > > > > > > > > > > matches > > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > one > > > > > > > > > > > > > > > > > > > > > > > > stored in ZooKeeper, and we will > > > > > have to > > > > > > > > > perform > > > > > > > > > > > this > > > > > > > > > > > > > > > > > validation > > > > > > > > > > > > > > > > > > > > > > > > somewhere in the KIP-500 world. > > > If > > > > > the > > > > > > > > > > > Controller > > > > > > > > > > > > > > > doesn't > > > > > > > > > > > > > > > > > do it > > > > > > > > > > > > > > > > > > > > > > > > within the registration request > > > then > > > > > the > > > > > > > > > broker > > > > > > > > > > > will > > > > > > > > > > > > > > > have to > > > > > > > > > > > > > > > > > > > make a > > > > > > > > > > > > > > > > > > > > > > > > metadata request to the > > > Controller, > > > > > > > retrieve > > > > > > > > > the > > > > > > > > > > > > > Cluster > > > > > > > > > > > > > > > ID, > > > > > > > > > > > > > > > > > and > > > > > > > > > > > > > > > > > > > > > > > > perform the check itself. It > > > feels > > > > > to me > > > > > > > > > that it > > > > > > > > > > > > > might > > > > > > > > > > > > > > > be > > > > > > > > > > > > > > > > > > > better for > > > > > > > > > > > > > > > > > > > > > > > > the Controller to just do it, > > > > > > > > > > > > > > > > > > > > > > > > and > > > > > then > > > > > > > the > > > > > > > > > broker > > > > > > > > > > > > > doesn't > > > > > > > > > > > > > > > > > have to > > > > > > > > > > > > > > > > > > > > > > > > worry about it anymore once it > > > > > > > successfully > > > > > > > > > > > > > registers. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I also have a question about the > > > > > > > broker.id > > > > > > > > > > > value and > > > > > > > > > > > > > > > > > > > > > meta.properties. > > > > > > > > > > > > > > > > > > > > > > > > The KIP now says "In version 0 > > > > > > > > > > > > > > > > > > > > > > > > of > > > > > > > > > > > meta.properties, > > > > > > > > > > > > > there > > > > > > > > > > > > > > > is a > > > > > > > > > > > > > > > > > > > > > > > > broker.id field. Version 1 > > > does not > > > > > > > have > > > > > > > > > this > > > > > > > > > > > > > field. > > > > > > > > > > > > > > > It > > > > > > > > > > > > > > > > > is no > > > > > > > > > > > > > > > > > > > > > longer > > > > > > > > > > > > > > > > > > > > > > > > needed because we no longer > > > support > > > > > > > dynamic > > > > > > > > > > > broker id > > > > > > > > > > > > > > > > > > > assignment." > > > > > > > > > > > > > > > > > > > > > > > > But then there is an example > > > version > > > > > 1 > > > > > > > > > > > > > meta.properties > > > > > > > > > > > > > > > file > > > > > > > > > > > > > > > > > that > > > > > > > > > > > > > > > > > > > > > shows > > > > > > > > > > > > > > > > > > > > > > > > the broker.id value. I actually > > > > > wonder > > > > > > > if > > > > > > > > > > > maybe the > > > > > > > > > > > > > > > > > broker.id > > > > > > > > > > > > > > > > > > > value > > > > > > > > > > > > > > > > > > > > > > > > would be good to keep in the > > > version > > > > > 1 > > > > > > > > > > > > > meta.properties > > > > > > > > > > > > > > > file > > > > > > > > > > > > > > > > > > > because > > > > > > > > > > > > > > > > > > > > > it > > > > > > > > > > > > > > > > > > > > > > > > currently (pre-KIP 500, version > > > 0) > > > > > acts > > > > > > > as a > > > > > > > > > > > sanity > > > > > > > > > > > > > > > check to > > > > > > > > > > > > > > > > > make > > > > > > > > > > > > > > > > > > > > > sure > > > > > > > > > > > > > > > > > > > > > > > > the broker is using the correct > > > log > > > > > > > > > directory. > > > > > > > > > > > > > Similarly > > > > > > > > > > > > > > > > > with > > > > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > > > > > > controller.id value on > > > controllers > > > > > -- it > > > > > > > > > would > > > > > > > > > > > > > allow the > > > > > > > > > > > > > > > > > same > > > > > > > > > > > > > > > > > > > type > > > > > > > > > > > > > > > > > > > > > of > > > > > > > > > > > > > > > > > > > > > > > > sanity check for quorum > > > controllers. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > That's a good point. I will add > > > > > broker.id > > > > > > > > > back, > > > > > > > > > > > and > > > > > > > > > > > > > also > > > > > > > > > > > > > > > add > > > > > > > > > > > > > > > > > > > > > > > controller.id as a possibility. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > cheers, > > > > > > > > > > > > > > > > > > > > > > > Colin > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Nov 30, 2020 at 7:41 PM > > > Colin > > > > > > > McCabe > > > > > > > > > < > > > > > > > > > > > > > > > > > cmcc...@apache.org > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Fri, Oct 23, 2020, at > > > > > > > > > > > > > > > > > > > > > > > > > 16:10, > > > > > Jun Rao > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > Hi, Colin, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks for the reply. A few > > > more > > > > > > > > > comments. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Jun, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks again for the reply. > > > Sorry > > > > > for > > > > > > > the > > > > > > > > > long > > > > > > > > > > > > > > > hiatus. I > > > > > > > > > > > > > > > > > was > > > > > > > > > > > > > > > > > > > on > > > > > > > > > > > > > > > > > > > > > > > vacation for a while. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 55. There is still text that > > > > > favors > > > > > > > new > > > > > > > > > > > broker > > > > > > > > > > > > > > > > > registration. > > > > > > > > > > > > > > > > > > > > > "When a > > > > > > > > > > > > > > > > > > > > > > > broker > > > > > > > > > > > > > > > > > > > > > > > > > > first starts up, when it is > > > in > > > > > the > > > > > > > > > INITIAL > > > > > > > > > > > > > state, it > > > > > > > > > > > > > > > will > > > > > > > > > > > > > > > > > > > always > > > > > > > > > > > > > > > > > > > > > > > "win" > > > > > > > > > > > > > > > > > > > > > > > > > > broker ID conflicts. > > > However, > > > > > once > > > > > > > it is > > > > > > > > > > > > > granted a > > > > > > > > > > > > > > > > > lease, it > > > > > > > > > > > > > > > > > > > > > > > transitions > > > > > > > > > > > > > > > > > > > > > > > > > > out of the INITIAL state. > > > > > > > Thereafter, > > > > > > > > > it may > > > > > > > > > > > > > lose > > > > > > > > > > > > > > > > > subsequent > > > > > > > > > > > > > > > > > > > > > > > conflicts if > > > > > > > > > > > > > > > > > > > > > > > > > > its broker epoch is stale. > > > (See > > > > > > > KIP-380 > > > > > > > > > for > > > > > > > > > > > some > > > > > > > > > > > > > > > > > background > > > > > > > > > > > > > > > > > > > on > > > > > > > > > > > > > > > > > > > > > > > broker > > > > > > > > > > > > > > > > > > > > > > > > > > epoch.) The reason for > > > favoring > > > > > new > > > > > > > > > > > processes > > > > > > > > > > > > > is to > > > > > > > > > > > > > > > > > > > accommodate > > > > > > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > > > > > common > > > > > > > > > > > > > > > > > > > > > > > > > > case where a process is > > > killed > > > > > with > > > > > > > kill > > > > > > > > > -9 > > > > > > > > > > > and > > > > > > > > > > > > > then > > > > > > > > > > > > > > > > > > > restarted. > > > > > > > > > > > > > > > > > > > > > We > > > > > > > > > > > > > > > > > > > > > > > want it > > > > > > > > > > > > > > > > > > > > > > > > > > to be able to reclaim its > > > old ID > > > > > > > quickly > > > > > > > > > in > > > > > > > > > > > this > > > > > > > > > > > > > > > case." > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks for the reminder. I > > > have > > > > > > > clarified > > > > > > > > > the > > > > > > > > > > > > > language > > > > > > > > > > > > > > > > > here. > > > > > > > > > > > > > > > > > > > > > > > Hopefully now it is clear that we > > > don't > > > > > > > allow > > > > > > > > > quick > > > > > > > > > > > > > re-use > > > > > > > > > > > > > > > of > > > > > > > > > > > > > > > > > > > broker > > > > > > > > > > > > > > > > > > > > > IDs. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 80.1 Sounds good. Could you > > > > > document > > > > > > > that > > > > > > > > > > > > > listeners > > > > > > > > > > > > > > > is a > > > > > > > > > > > > > > > > > > > required > > > > > > > > > > > > > > > > > > > > > > > config > > > > > > > > > > > > > > > > > > > > > > > > > > now? It would also be useful > > > to > > > > > > > annotate > > > > > > > > > > > other > > > > > > > > > > > > > > > required > > > > > > > > > > > > > > > > > > > configs. > > > > > > > > > > > > > > > > > > > > > For > > > > > > > > > > > > > > > > > > > > > > > > > > example, controller.connect > > > > > should be > > > > > > > > > > > required. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I added a note specifying that > > > > > these > > > > > > > are > > > > > > > > > > > required. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 80.2 Could you list all > > > > > deprecated > > > > > > > > > existing > > > > > > > > > > > > > configs? > > > > > > > > > > > > > > > > > Another > > > > > > > > > > > > > > > > > > > one > > > > > > > > > > > > > > > > > > > > > is > > > > > > > > > > > > > > > > > > > > > > > > > > control.plane.listener.name > > > > > since > > > > > > > the > > > > > > > > > > > > > controller no > > > > > > > > > > > > > > > > > longer > > > > > > > > > > > > > > > > > > > sends > > > > > > > > > > > > > > > > > > > > > > > > > > LeaderAndIsr, UpdateMetadata > > > and > > > > > > > > > StopReplica > > > > > > > > > > > > > > > requests. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I added a section specifying > > > some > > > > > > > > > deprecated > > > > > > > > > > > > > configs. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 83.1 It seems that the > > > broker can > > > > > > > > > transition > > > > > > > > > > > from > > > > > > > > > > > > > > > FENCED > > > > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > > > > > RUNNING > > > > > > > > > > > > > > > > > > > > > > > without > > > > > > > > > > > > > > > > > > > > > > > > > > registering for a new broker > > > > > epoch. > > > > > > > I am > > > > > > > > > not > > > > > > > > > > > > > sure how > > > > > > > > > > > > > > > > > this > > > > > > > > > > > > > > > > > > > works. > > > > > > > > > > > > > > > > > > > > > > > Once the > > > > > > > > > > > > > > > > > > > > > > > > > > controller fences a broker, > > > > > there is > > > > > > > no > > > > > > > > > need > > > > > > > > > > > for > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > controller > > > > > > > > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > > > > > > > keep the > > > > > > > > > > > > > > > > > > > > > > > > > > boker epoch around. So, if > > > the > > > > > fenced > > > > > > > > > > > broker's > > > > > > > > > > > > > > > heartbeat > > > > > > > > > > > > > > > > > > > request > > > > > > > > > > > > > > > > > > > > > > > with the > > > > > > > > > > > > > > > > > > > > > > > > > > existing broker epoch will > > > > > > > > > > > > > > > > > > > > > > > > > > be > > > > > > > rejected, > > > > > > > > > > > leading > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > broker > > > > > > > > > > > > > > > > > > > back > > > > > > > > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > > > > > > > > FENCED state again. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The broker epoch refers to the > > > > > broker > > > > > > > > > > > registration. > > > > > > > > > > > > > > > So we > > > > > > > > > > > > > > > > > DO > > > > > > > > > > > > > > > > > > > keep > > > > > > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > > > > > broker epoch around even while the > > > > > broker > > > > > > > is > > > > > > > > > > > fenced. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The broker epoch changes only > > > when > > > > > > > there > > > > > > > > > is a > > > > > > > > > > > new > > > > > > > > > > > > > > > broker > > > > > > > > > > > > > > > > > > > > > > > registration. Fencing or > > > unfencing the > > > > > > > broker > > > > > > > > > > > doesn't > > > > > > > > > > > > > > > change > > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > > > broker > > > > > > > > > > > > > > > > > > > > > > > epoch. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 83.5 Good point on KIP-590. > > > Then > > > > > > > should > > > > > > > > > we > > > > > > > > > > > > > expose the > > > > > > > > > > > > > > > > > > > controller > > > > > > > > > > > > > > > > > > > > > for > > > > > > > > > > > > > > > > > > > > > > > > > > debugging purposes? If not, > > > we > > > > > should > > > > > > > > > > > deprecate > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > controllerID > > > > > > > > > > > > > > > > > > > > > > > field in > > > > > > > > > > > > > > > > > > > > > > > > > > MetadataResponse? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I think it's OK to expose it > > > for > > > > > now, > > > > > > > with > > > > > > > > > the > > > > > > > > > > > > > proviso > > > > > > > > > > > > > > > > > that it > > > > > > > > > > > > > > > > > > > > > won't > > > > > > > > > > > > > > > > > > > > > > > be reachable by clients. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 90. We rejected the shared > > > > > > > > > > > > > > > > > > > > > > > > > > ID > > > > > with > > > > > > > just > > > > > > > > > one > > > > > > > > > > > > > reason > > > > > > > > > > > > > > > "This > > > > > > > > > > > > > > > > > is > > > > > > > > > > > > > > > > > > > not a > > > > > > > > > > > > > > > > > > > > > > > good idea > > > > > > > > > > > > > > > > > > > > > > > > > > because NetworkClient > > > assumes a > > > > > > > single ID > > > > > > > > > > > > > space. So > > > > > > > > > > > > > > > if > > > > > > > > > > > > > > > > > > > there is > > > > > > > > > > > > > > > > > > > > > > > both a > > > > > > > > > > > > > > > > > > > > > > > > > > controller 1 and a broker 1, > > > we > > > > > don't > > > > > > > > > have a > > > > > > > > > > > way > > > > > > > > > > > > > of > > > > > > > > > > > > > > > > > picking > > > > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > > > > > "right" > > > > > > > > > > > > > > > > > > > > > > > > > > one." This doesn't seem to > > > be a > > > > > > > strong > > > > > > > > > > > reason. > > > > > > > > > > > > > For > > > > > > > > > > > > > > > > > example, > > > > > > > > > > > > > > > > > > > we > > > > > > > > > > > > > > > > > > > > > could > > > > > > > > > > > > > > > > > > > > > > > > > > address the NetworkClient > > > issue > > > > > with > > > > > > > the > > > > > > > > > node > > > > > > > > > > > > > type > > > > > > > > > > > > > > > as you > > > > > > > > > > > > > > > > > > > pointed > > > > > > > > > > > > > > > > > > > > > > > out or > > > > > > > > > > > > > > > > > > > > > > > > > > using the negative value of > > > > > > > > > > > > > > > > > > > > > > > > > > a > > > > > broker > > > > > > > ID > > > > > > > > > as > > > > > > > > > > > the > > > > > > > > > > > > > > > > > controller ID. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > It would require a lot of code > > > > > changes > > > > > > > to > > > > > > > > > > > support > > > > > > > > > > > > > > > multiple > > > > > > > > > > > > > > > > > > > types of > > > > > > > > > > > > > > > > > > > > > > > node IDs. It's not clear to me > > > that > > > > > the > > > > > > > end > > > > > > > > > result > > > > > > > > > > > > > would > > > > > > > > > > > > > > > be > > > > > > > > > > > > > > > > > > > better -- > > > > > > > > > > > > > > > > > > > > > I > > > > > > > > > > > > > > > > > > > > > > > tend to think it would be worse, > > > since > > > > > it > > > > > > > > > would be > > > > > > > > > > > more > > > > > > > > > > > > > > > > > complex. > > > > > > > > > > > > > > > > > > > In a > > > > > > > > > > > > > > > > > > > > > > > similar vein, using negative > > > numbers > > > > > seems > > > > > > > > > > > dangerous, > > > > > > > > > > > > > > > since we > > > > > > > > > > > > > > > > > use > > > > > > > > > > > > > > > > > > > > > > > negatives or -1 as "special > > > values" in > > > > > many > > > > > > > > > places. > > > > > > > > > > > > > For > > > > > > > > > > > > > > > > > example, > > > > > > > > > > > > > > > > > > > -1 > > > > > > > > > > > > > > > > > > > > > often > > > > > > > > > > > > > > > > > > > > > > > represents "no such node." > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > One important thing to keep in > > > > > mind is > > > > > > > > > that we > > > > > > > > > > > > > want to > > > > > > > > > > > > > > > be > > > > > > > > > > > > > > > > > able > > > > > > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > > > > > > > transition from a broker and a > > > > > controller > > > > > > > being > > > > > > > > > > > > > co-located > > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > them > > > > > > > > > > > > > > > > > > > no > > > > > > > > > > > > > > > > > > > > > > > longer being co-located. This is > > > much > > > > > > > easier > > > > > > > > > to do > > > > > > > > > > > > > when > > > > > > > > > > > > > > > they > > > > > > > > > > > > > > > > > have > > > > > > > > > > > > > > > > > > > > > separate > > > > > > > > > > > > > > > > > > > > > > > IDs. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 100. In KIP-589 > > > > > > > > > > > > > > > > > > > > > > > > > > < > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-589+Add+API+to+update+Replica+state+in+Controller > > > > > > > > > > > > > > > > > > > > > > > >, > > > > > > > > > > > > > > > > > > > > > > > > > > the broker reports all > > > offline > > > > > > > replicas > > > > > > > > > due > > > > > > > > > > > to a > > > > > > > > > > > > > disk > > > > > > > > > > > > > > > > > > > failure to > > > > > > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > > > > > > > > controller. It seems this > > > > > information > > > > > > > > > needs > > > > > > > > > > > to be > > > > > > > > > > > > > > > > > persisted > > > > > > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > > > > > > > > metadata > > > > > > > > > > > > > > > > > > > > > > > > > > log. Do we have a > > > corresponding > > > > > > > record > > > > > > > > > for > > > > > > > > > > > that? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hmm, I have to look into this > > > > > > > > > > > > > > > > > > > > > > > > > a > > > > > little > > > > > > > bit > > > > > > > > > > > more. > > > > > > > > > > > > > We > > > > > > > > > > > > > > > may > > > > > > > > > > > > > > > > > need > > > > > > > > > > > > > > > > > > > a new > > > > > > > > > > > > > > > > > > > > > > > record type. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 101. Currently, StopReplica > > > > > request > > > > > > > has 2 > > > > > > > > > > > modes, > > > > > > > > > > > > > > > without > > > > > > > > > > > > > > > > > > > deletion > > > > > > > > > > > > > > > > > > > > > > > and with > > > > > > > > > > > > > > > > > > > > > > > > > > deletion. The former is used > > > for > > > > > > > > > controlled > > > > > > > > > > > > > shutdown > > > > > > > > > > > > > > > and > > > > > > > > > > > > > > > > > > > handling > > > > > > > > > > > > > > > > > > > > > > > disk > > > > > > > > > > > > > > > > > > > > > > > > > > failure, and causes the > > > follower > > > > > to > > > > > > > > > stop. The > > > > > > > > > > > > > latter > > > > > > > > > > > > > > > is > > > > > > > > > > > > > > > > > for > > > > > > > > > > > > > > > > > > > topic > > > > > > > > > > > > > > > > > > > > > > > deletion > > > > > > > > > > > > > > > > > > > > > > > > > > and partition reassignment, > > > and > > > > > > > causes > > > > > > > > > the > > > > > > > > > > > > > replica > > > > > > > > > > > > > > > to be > > > > > > > > > > > > > > > > > > > deleted. > > > > > > > > > > > > > > > > > > > > > > > Since we > > > > > > > > > > > > > > > > > > > > > > > > > > are deprecating StopReplica, > > > > > could we > > > > > > > > > > > document > > > > > > > > > > > > > what > > > > > > > > > > > > > > > > > triggers > > > > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > > > > > stopping > > > > > > > > > > > > > > > > > > > > > > > > > > of a follower and the > > > deleting > > > > > of a > > > > > > > > > replica > > > > > > > > > > > now? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > RemoveTopic triggers > > > deletion. In > > > > > > > general > > > > > > > > > the > > > > > > > > > > > > > > > > > functionality of > > > > > > > > > > > > > > > > > > > > > > > StopReplica is subsumed by the > > > metadata > > > > > > > > > records. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 102. Should we include the > > > > > metadata > > > > > > > > > topic in > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > > > MetadataResponse? > > > > > > > > > > > > > > > > > > > > > > > If so, > > > > > > > > > > > > > > > > > > > > > > > > > > when it will be included and > > > what > > > > > > > will > > > > > > > > > the > > > > > > > > > > > > > metadata > > > > > > > > > > > > > > > > > response > > > > > > > > > > > > > > > > > > > look > > > > > > > > > > > > > > > > > > > > > > > like? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > No, it won't be included in > > > > > > > > > > > > > > > > > > > > > > > > > the > > > > > > > metadata > > > > > > > > > > > response > > > > > > > > > > > > > sent > > > > > > > > > > > > > > > back > > > > > > > > > > > > > > > > > > > from > > > > > > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > > > > > brokers. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 103. "The active controller > > > > > assigns > > > > > > > the > > > > > > > > > > > broker a > > > > > > > > > > > > > new > > > > > > > > > > > > > > > > > broker > > > > > > > > > > > > > > > > > > > > > epoch, > > > > > > > > > > > > > > > > > > > > > > > based on > > > > > > > > > > > > > > > > > > > > > > > > > > the latest committed offset > > > in > > > > > the > > > > > > > log." > > > > > > > > > This > > > > > > > > > > > > > seems > > > > > > > > > > > > > > > > > > > inaccurate > > > > > > > > > > > > > > > > > > > > > since > > > > > > > > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > > > > > > > > latest committed offset > > > doesn't > > > > > > > always > > > > > > > > > > > advance on > > > > > > > > > > > > > > > every > > > > > > > > > > > > > > > > > log > > > > > > > > > > > > > > > > > > > > > append. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Given that the new broker > > > > > > > > > > > > > > > > > > > > > > > > > epoch > > > > > won't > > > > > > > be > > > > > > > > > > > visible > > > > > > > > > > > > > until > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > commit > > > > > > > > > > > > > > > > > > > > > has > > > > > > > > > > > > > > > > > > > > > > > happened, I have changed this to > > > "the > > > > > next > > > > > > > > > > > available > > > > > > > > > > > > > > > offset in > > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > log" > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 104. REGISTERING(1) : It > > > > > > > > > > > > > > > > > > > > > > > > > > says > > > > > > > > > "Otherwise, the > > > > > > > > > > > > > broker > > > > > > > > > > > > > > > > > moves > > > > > > > > > > > > > > > > > > > into > > > > > > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > > > > > FENCED > > > > > > > > > > > > > > > > > > > > > > > > > > state.". It seems this > > > should be > > > > > > > RUNNING? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 105. RUNNING: Should we > > > require > > > > > the > > > > > > > > > broker to > > > > > > > > > > > > > catch > > > > > > > > > > > > > > > up > > > > > > > > > > > > > > > > > to the > > > > > > > > > > > > > > > > > > > > > > > metadata log > > > > > > > > > > > > > > > > > > > > > > > > > > to get into this state? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > For 104 and 105, these > > > > > > > > > > > > > > > > > > > > > > > > > sections > > > > > have > > > > > > > been > > > > > > > > > > > reworked. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > best, > > > > > > > > > > > > > > > > > > > > > > > > > Colin > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Jun > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Fri, Oct 23, 2020 at 1:20 > > > PM > > > > > Colin > > > > > > > > > McCabe > > > > > > > > > > > < > > > > > > > > > > > > > > > > > > > cmcc...@apache.org > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Oct 21, 2020, at > > > > > 05:51, Tom > > > > > > > > > Bentley > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Colin, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Oct 19, 2020, at > > > > > 08:59, > > > > > > > Ron > > > > > > > > > > > Dagostino > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Colin. Thanks > > > for the > > > > > > > hard > > > > > > > > > work > > > > > > > > > > > on > > > > > > > > > > > > > this > > > > > > > > > > > > > > > KIP. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I have some > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > questions > > > > > about > > > > > > > what > > > > > > > > > > > happens > > > > > > > > > > > > > to a > > > > > > > > > > > > > > > > > broker > > > > > > > > > > > > > > > > > > > > > when it > > > > > > > > > > > > > > > > > > > > > > > becomes > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > fenced (e.g. because > > > it > > > > > can't > > > > > > > > > send a > > > > > > > > > > > > > > > heartbeat > > > > > > > > > > > > > > > > > > > request to > > > > > > > > > > > > > > > > > > > > > > > keep its > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > lease). The KIP > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > says > > > > > "When a > > > > > > > > > broker > > > > > > > > > > > is > > > > > > > > > > > > > > > fenced, > > > > > > > > > > > > > > > > > it > > > > > > > > > > > > > > > > > > > cannot > > > > > > > > > > > > > > > > > > > > > > > process any > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > client requests. > > > This > > > > > > > prevents > > > > > > > > > > > brokers > > > > > > > > > > > > > which > > > > > > > > > > > > > > > > > are not > > > > > > > > > > > > > > > > > > > > > > > receiving > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > metadata updates or > > > that > > > > > are > > > > > > > not > > > > > > > > > > > > > receiving > > > > > > > > > > > > > > > and > > > > > > > > > > > > > > > > > > > processing > > > > > > > > > > > > > > > > > > > > > > > them fast > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > enough from causing > > > > > issues to > > > > > > > > > > > clients." > > > > > > > > > > > > > And > > > > > > > > > > > > > > > in > > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > > > > > description of the > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > FENCED(4) state it > > > > > likewise > > > > > > > says > > > > > > > > > > > "While > > > > > > > > > > > > > in > > > > > > > > > > > > > > > this > > > > > > > > > > > > > > > > > > > state, > > > > > > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > > > > > broker > > > > > > > > > > > > > > > > > > > > > > > > > > > does > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > not respond to > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > client > > > > > > > > > requests." It > > > > > > > > > > > > > makes > > > > > > > > > > > > > > > sense > > > > > > > > > > > > > > > > > > > that a > > > > > > > > > > > > > > > > > > > > > > > fenced broker > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > should not accept > > > > > producer > > > > > > > > > requests > > > > > > > > > > > -- I > > > > > > > > > > > > > > > assume > > > > > > > > > > > > > > > > > any > > > > > > > > > > > > > > > > > > > such > > > > > > > > > > > > > > > > > > > > > > > requests > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > would result in > > > > > > > > > > > > > NotLeaderOrFollowerException. > > > > > > > > > > > > > > > > > But > > > > > > > > > > > > > > > > > > > what > > > > > > > > > > > > > > > > > > > > > > > about KIP-392 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > (fetch from > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > follower) > > > > > > > consumer > > > > > > > > > > > > > requests? It > > > > > > > > > > > > > > > is > > > > > > > > > > > > > > > > > > > > > conceivable > > > > > > > > > > > > > > > > > > > > > > > that > > > > > > > > > > > > > > > > > > > > > > > > > > > these > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > could continue. > > > Related > > > > > to > > > > > > > that, > > > > > > > > > > > would a > > > > > > > > > > > > > > > fenced > > > > > > > > > > > > > > > > > > > broker > > > > > > > > > > > > > > > > > > > > > > > continue to > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > fetch data for > > > partitions > > > > > > > where > > > > > > > > > it > > > > > > > > > > > > > thinks it > > > > > > > > > > > > > > > is a > > > > > > > > > > > > > > > > > > > > > follower? > > > > > > > > > > > > > > > > > > > > > > > Even if > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > it rejects consumer > > > > > requests > > > > > > > it > > > > > > > > > might > > > > > > > > > > > > > still > > > > > > > > > > > > > > > > > continue > > > > > > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > > > > > > > fetch as a > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > follower. Might it > > > be > > > > > > > helpful to > > > > > > > > > > > clarify > > > > > > > > > > > > > > > both > > > > > > > > > > > > > > > > > > > decisions > > > > > > > > > > > > > > > > > > > > > > > here? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Ron, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Good question. I > > > think a > > > > > > > fenced > > > > > > > > > broker > > > > > > > > > > > > > should > > > > > > > > > > > > > > > > > > > continue to > > > > > > > > > > > > > > > > > > > > > > > fetch on > > > > > > > > > > > > > > > > > > > > > > > > > > > > > partitions it was > > > already > > > > > > > fetching > > > > > > > > > > > before > > > > > > > > > > > > > it > > > > > > > > > > > > > > > was > > > > > > > > > > > > > > > > > > > fenced, > > > > > > > > > > > > > > > > > > > > > > > unless it > > > > > > > > > > > > > > > > > > > > > > > > > > > hits a > > > > > > > > > > > > > > > > > > > > > > > > > > > > > problem. At that > > > point it > > > > > > > won't be > > > > > > > > > > > able to > > > > > > > > > > > > > > > > > continue, > > > > > > > > > > > > > > > > > > > > > since it > > > > > > > > > > > > > > > > > > > > > > > doesn't > > > > > > > > > > > > > > > > > > > > > > > > > > > have > > > > > > > > > > > > > > > > > > > > > > > > > > > > > the new metadata. For > > > > > > > example, it > > > > > > > > > > > won't > > > > > > > > > > > > > know > > > > > > > > > > > > > > > about > > > > > > > > > > > > > > > > > > > > > leadership > > > > > > > > > > > > > > > > > > > > > > > changes > > > > > > > > > > > > > > > > > > > > > > > > > > > in > > > > > > > > > > > > > > > > > > > > > > > > > > > > > the partitions it's > > > > > fetching. > > > > > > > The > > > > > > > > > > > > > rationale > > > > > > > > > > > > > > > for > > > > > > > > > > > > > > > > > > > > > continuing to > > > > > > > > > > > > > > > > > > > > > > > fetch > > > > > > > > > > > > > > > > > > > > > > > > > > > is to > > > > > > > > > > > > > > > > > > > > > > > > > > > > > try to avoid > > > disruptions as > > > > > > > much as > > > > > > > > > > > > > possible. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I don't think fenced > > > > > brokers > > > > > > > should > > > > > > > > > > > accept > > > > > > > > > > > > > > > client > > > > > > > > > > > > > > > > > > > requests. > > > > > > > > > > > > > > > > > > > > > > > The issue > > > > > > > > > > > > > > > > > > > > > > > > > > > is > > > > > > > > > > > > > > > > > > > > > > > > > > > > > that the fenced broker > > > may > > > > > or > > > > > > > may > > > > > > > > > not > > > > > > > > > > > have > > > > > > > > > > > > > any > > > > > > > > > > > > > > > > > data it > > > > > > > > > > > > > > > > > > > is > > > > > > > > > > > > > > > > > > > > > > > supposed to > > > > > > > > > > > > > > > > > > > > > > > > > > > > > have. It may or may > > > not > > > > > have > > > > > > > > > applied > > > > > > > > > > > any > > > > > > > > > > > > > > > > > configuration > > > > > > > > > > > > > > > > > > > > > > > changes, etc. > > > > > > > > > > > > > > > > > > > > > > > > > > > that > > > > > > > > > > > > > > > > > > > > > > > > > > > > > it is supposed to have > > > > > applied. > > > > > > > > > So it > > > > > > > > > > > > > could > > > > > > > > > > > > > > > get > > > > > > > > > > > > > > > > > pretty > > > > > > > > > > > > > > > > > > > > > > > confusing, and > > > > > > > > > > > > > > > > > > > > > > > > > > > also > > > > > > > > > > > > > > > > > > > > > > > > > > > > > potentially waste the > > > > > client's > > > > > > > > > time. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > When fenced, how would > > > the > > > > > broker > > > > > > > > > reply > > > > > > > > > > > to a > > > > > > > > > > > > > > > client > > > > > > > > > > > > > > > > > > > which did > > > > > > > > > > > > > > > > > > > > > > > make a > > > > > > > > > > > > > > > > > > > > > > > > > > > > request? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Tom, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The broker will respond > > > with a > > > > > > > > > retryable > > > > > > > > > > > error > > > > > > > > > > > > > in > > > > > > > > > > > > > > > that > > > > > > > > > > > > > > > > > > > case. > > > > > > > > > > > > > > > > > > > > > Once > > > > > > > > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > > > > > > > > > client has re-fetched its > > > > > > > metadata, it > > > > > > > > > > > will no > > > > > > > > > > > > > > > longer > > > > > > > > > > > > > > > > > see > > > > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > > > > > fenced broker > > > > > > > > > > > > > > > > > > > > > > > > > > > as part of the cluster. I > > > > > added a > > > > > > > > > note to > > > > > > > > > > > the > > > > > > > > > > > > > KIP. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > best, > > > > > > > > > > > > > > > > > > > > > > > > > > > Colin > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Tom > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > Gwen Shapira > Engineering Manager | Confluent > 650.450.2760 | @gwenshap > Follow us: Twitter | blog >