Tom: bq. create a znode /admin/reassignments/$topic-$partition Looks like the tree structure above should be:
/admin/reassignments/$topic/$partition bq. The controller removes /admin/reassignment/$topic/$partition Note the lack of 's' for reassignment. It would be good to make zookeeper paths consistent. Thanks On Wed, Dec 13, 2017 at 9:49 AM, Tom Bentley <t.j.bent...@gmail.com> wrote: > Hi Jun and Ted, > > Jun, you're right that needing one watcher per reassigned partition > presents a scalability problem, and using a separate notification path > solves that. I also agree that it makes sense to prevent users from using > both methods on the same reassignment. > > Ted, naming the reassignments like mytopic-42 was simpler while I was > proposing a watcher-per-reassignment (I'd have needed a child watcher on > /admin/reassignments and also on /admin/reassignments/mytopic). Using the > separate notification path means I don't need any watchers in the > /admin/reassignments subtree, so switching to /admin/reassignments/mytopic/ > 42 > would work, and avoid /admin/reassignments having a very large number of > child nodes. On the other hand it also means I have to create and delete > the topic nodes (e.g. /admin/reassignments/mytopic), which incurs the cost > of extra round trips to zookeeper. I suppose that since reassignment is > generally a slow process it makes little difference if we increase the > latency of the interactions with zookeeper. > > I have updated the KIP with these improvements, and a more detailed > description of exactly how we would manage these znodes. > > Reading the algorithm in KafkaController.onPartitionReassignment(), it > seems that it would be suboptimal for changing reassignments in-flight. > Consider an initial assignment of [1,2], reassigned to [2,3] and then > changed to [2,4]. Broker 3 will remain in the assigned replicas until > broker 4 is in sync, even though 3 wasn't actually one of the original > assigned replicas and is no longer a new assigned replica. I think this > also affects the case where the reassignment is cancelled > ([1,2]->[2,3]->[1,2]): We again have to wait for 3 to catch up, even though > its replica will then be deleted. > > Should we seek to improve this algorithm in this KIP, or leave that as a > later optimisation? > > Cheers, > > Tom > > On 11 December 2017 at 21:31, Jun Rao <j...@confluent.io> wrote: > > > Another question is on the compatibility. Since now there are 2 ways of > > specifying a partition reassignment, one under /admin/reassign_partitions > > and the other under /admin/reassignments, we probably want to prevent the > > same topic being reassigned under both paths at the same time? > > Thanks, > > > > Jun > > > > > > > > On Fri, Dec 8, 2017 at 5:41 PM, Jun Rao <j...@confluent.io> wrote: > > > > > Hi, Tom, > > > > > > Thanks for the KIP. It definitely addresses one of the pain points in > > > partition reassignment. Another issue that it also addresses is the ZK > > node > > > size limit when writing the reassignment JSON. > > > > > > My only concern is that the KIP needs to create one watcher per > > reassigned > > > partition. This could add overhead in ZK and complexity for debugging > > when > > > lots of partitions are being reassigned simultaneously. We could > > > potentially improve this by introducing a separate ZK path for change > > > notification as we do for configs. For example, every time we change > the > > > assignment for a set of partitions, we could further write a sequential > > > node /admin/reassignment_changes/[change_x]. That way, the controller > > > only needs to watch the change path. Once a change is triggered, the > > > controller can read everything under /admin/reassignments/. > > > > > > Jun > > > > > > > > > On Wed, Dec 6, 2017 at 1:19 PM, Tom Bentley <t.j.bent...@gmail.com> > > wrote: > > > > > >> Hi, > > >> > > >> This is still very new, but I wanted some quick feedback on a > > preliminary > > >> KIP which could, I think, help with providing an AdminClient API for > > >> partition reassignment. > > >> > > >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-236% > > >> 3A+Interruptible+Partition+Reassignment > > >> > > >> I wasn't sure whether to start fleshing out a whole AdminClient API in > > >> this > > >> KIP (which would make it very big, and difficult to read), or whether > to > > >> break it down into smaller KIPs (which makes it easier to read and > > >> implement in pieces, but harder to get a high-level picture of the > > >> ultimate > > >> destination). For now I've gone for a very small initial KIP, but I'm > > >> happy > > >> to sketch the bigger picture here if people are interested. > > >> > > >> Cheers, > > >> > > >> Tom > > >> > > > > > > > > > > > On 11 December 2017 at 21:31, Jun Rao <j...@confluent.io> wrote: > > > Another question is on the compatibility. Since now there are 2 ways of > > specifying a partition reassignment, one under /admin/reassign_partitions > > and the other under /admin/reassignments, we probably want to prevent the > > same topic being reassigned under both paths at the same time? > > Thanks, > > > > Jun > > > > > > > > On Fri, Dec 8, 2017 at 5:41 PM, Jun Rao <j...@confluent.io> wrote: > > > > > Hi, Tom, > > > > > > Thanks for the KIP. It definitely addresses one of the pain points in > > > partition reassignment. Another issue that it also addresses is the ZK > > node > > > size limit when writing the reassignment JSON. > > > > > > My only concern is that the KIP needs to create one watcher per > > reassigned > > > partition. This could add overhead in ZK and complexity for debugging > > when > > > lots of partitions are being reassigned simultaneously. We could > > > potentially improve this by introducing a separate ZK path for change > > > notification as we do for configs. For example, every time we change > the > > > assignment for a set of partitions, we could further write a sequential > > > node /admin/reassignment_changes/[change_x]. That way, the controller > > > only needs to watch the change path. Once a change is triggered, the > > > controller can read everything under /admin/reassignments/. > > > > > > Jun > > > > > > > > > On Wed, Dec 6, 2017 at 1:19 PM, Tom Bentley <t.j.bent...@gmail.com> > > wrote: > > > > > >> Hi, > > >> > > >> This is still very new, but I wanted some quick feedback on a > > preliminary > > >> KIP which could, I think, help with providing an AdminClient API for > > >> partition reassignment. > > >> > > >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-236% > > >> 3A+Interruptible+Partition+Reassignment > > >> > > >> I wasn't sure whether to start fleshing out a whole AdminClient API in > > >> this > > >> KIP (which would make it very big, and difficult to read), or whether > to > > >> break it down into smaller KIPs (which makes it easier to read and > > >> implement in pieces, but harder to get a high-level picture of the > > >> ultimate > > >> destination). For now I've gone for a very small initial KIP, but I'm > > >> happy > > >> to sketch the bigger picture here if people are interested. > > >> > > >> Cheers, > > >> > > >> Tom > > >> > > > > > > > > >
