I shouldn't have said "always", sorry. When a replica is deleted, either due to a topic deletion or a reassignment, the controller transitions the replica to the `OfflineReplica` state and then to the `ReplicaDeletionStarted` state. The first transition issues a StopReplicaRequest with DeletePartitions=false and the second transition issues a StopReplicaRequest with DeleatePartitions=true. The two operations are actually doing the same except that the latter one deletes the replica.
At the moment, the `ControllerRequestBatch` partitions the accumulated stop replica requests and sends two requests. https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/controller/ControllerChannelManager.scala#L554 Having a per-partition flag allows us to combine everything into one request here if the requests are sent within the same batch obviously. This won't guarantee that all requests will be combined though because requests are batched optimistically in the controller but it opens the door to improve it in the future. Best, David One side note: The batching of the two requests depends on how the On Wed, Mar 25, 2020 at 6:12 PM Ismael Juma <ism...@juma.me.uk> wrote: > Is it really true that the controller always sends two requests? Aren't the > operations different (stop replica with delete versus stop replica > without)? > > On Wed, Mar 25, 2020, 9:59 AM David Jacot <dja...@confluent.io> wrote: > > > Hi all, > > > > I'd like to inform you that I have slightly changed the schema which was > > proposed > > in the KIP. During the implementation, I have realized that the proposed > > schema > > did not work. The new one reorganises how topics/partitions are stored. > > > > I'd like to amend the current KIP with the following: > > > > At the moment, the StopReplicaRequest has a top level field named > > `DeletePartitions` > > which indicates whether the partitions present in the request must be > > deleted or not. > > The downside of this is that the controller always ends up sending two > > StopReplica > > requests, one with DeletePartitions=true and one with > > DeletePartitions=false. > > > > Instead, I'd like to add a per-partition DeletePartition field to combine > > everything in > > one request. This will reduce the number of requests sent to each broker > > and also > > increase the batching. I've already implemented it. > > > > I've already updated the schema in the KIP if you want to see it. I will > > update the > > KIP itself if you agree with the amendment. > > > > What do you think? Does it sound reasonable? > > > > Best, > > David > > > > On Fri, Mar 6, 2020 at 3:37 PM David Jacot <dja...@confluent.io> wrote: > > > > > Hi all, > > > > > > The vote has passed with +3 binding votes (Jason Gustafson, Gwen > Shapira, > > > Jun Rao). > > > > > > Thanks to everyone! > > > > > > Best, > > > David > > > > > > On Wed, Mar 4, 2020 at 9:02 AM David Jacot <dja...@confluent.io> > wrote: > > > > > >> Hi Jun, > > >> > > >> You're right. I have noticed it while implementing it. I plan to use a > > >> default > > >> value as a sentinel in the protocol (e.g. -2) to cover this case. > > >> > > >> David > > >> > > >> On Wed, Mar 4, 2020 at 3:18 AM Jun Rao <j...@confluent.io> wrote: > > >> > > >>> Hi, David, > > >>> > > >>> Thanks for the KIP. +1 from me too. Just one comment below. > > >>> > > >>> 1. Regarding the sentinel leader epoch to indicate topic deletion, it > > >>> seems > > >>> that we need to use a different sentinel value to indicate that the > > >>> leader > > >>> epoch is not present when the controller is still on the old version > > >>> during > > >>> upgrade. > > >>> > > >>> Jun > > >>> > > >>> On Mon, Mar 2, 2020 at 11:20 AM Gwen Shapira <g...@confluent.io> > > wrote: > > >>> > > >>> > +1 > > >>> > > > >>> > On Mon, Feb 24, 2020, 2:16 AM David Jacot <dja...@confluent.io> > > wrote: > > >>> > > > >>> > > Hi all, > > >>> > > > > >>> > > I would like to start a vote on KIP-570: Add leader epoch in > > >>> > > StopReplicaRequest > > >>> > > > > >>> > > The KIP is here: > > >>> > > > > >>> > > > > >>> > > > >>> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-570%3A+Add+leader+epoch+in+StopReplicaRequest > > >>> > > > > >>> > > Thanks, > > >>> > > David > > >>> > > > > >>> > > > >>> > > >> > > >
