Hi David, Thanks for the KIP!
In the rejecting the alternative of having an RPC for log dir failures you say: It was also rejected to prevent "leaking" the notion of a log dir to the > public API. > I'm not quite sure I follow that argument, since we already have RPCs for changing replica log dirs. So in a general sense log dirs already exist in the API. I suspect you were using public API to mean something more specific; could you elaborate? It's also not completely clear that the cost of having to enumerate all the partitions on a log dir was weighed against the perceived benefit of a more flexible RPC. (I'm sure it was, but it would be good to say so). Many thanks, Tom On Wed, Apr 29, 2020 at 12:04 AM Colin McCabe <cmcc...@apache.org> wrote: > Hi David, > > Thanks for the KIP! > > I think the ReplicaStateEventResponse should have a separate error code > for each partition. > Currently it just has one error code for the whole request/response, if > I'm reading this right. I think Jose made a similar point as well. We > should plan for scenarios where some replica states can be changed and some > can't. > > Does EventType need to be an Int32? For enums, we usually use the > smallest reasonable type, which would be Int8 here. We can always change > the schema later if needed. UNKNOWN_REPLICA_EVENT_TYPE seems unnecessary > since INVALID_REQUEST covers this case. > > I'd also suggest "AlterReplicaStates[Request,Response]" as a slightly > better name for this RPC. > > cheers, > Colin > > > On Tue, Apr 7, 2020, at 12:43, David Arthur wrote: > > Hey everyone, > > > > I'd like to start the discussion for KIP-589, part of the KIP-500 effort > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-589+Add+API+to+update+Replica+state+in+Controller > > > > This is a proposal to use a new RPC instead of ZooKeeper for notifying > the > > controller of an offline replica. Please give a read and let me know your > > thoughts. > > > > Thanks! > > David > > > >