Hi Jun, Thanks for the comments.
On 12 September 2017 at 18:15, Jun Rao <j...@confluent.io> wrote: > Hi, Tom, > > Thanks for the KIP. +1. Just a couple of minor comments below. > > 1. The KIP has "INVALID_PARTITIONS (37) If the partition count was <= the > current partition count for the topic." We probably want to add one more > constraint: # of replicas in each new partition has to be the same as the > existing replication factor for the topic. > This is presently covered by the INVALID_REQUEST error code, but sure I can change it. 2. The KIP has the following. > > - REASSIGNMENT_IN_PROGRESS (new) If a partition reassignment is in > progress. It is necessary to prevent increasing partitions at the same > time so that we can be sure the partition has a meaningful replication > factor. > > > Currently, it's possible to increase the partition count while a > partition reassignment is in progress (adding partitions is much > cheaper than partition reassignment). On the other hand, if a topic is > being deleted, we will prevent adding new partitions. So, we probably > want to do the same in the KIP. > Increasing the partition count at the same time as a reassignment was covered in the [DISCUSS] thread. The problem is that during a reassignment there isn't a stable notion of replication factor. The brokers don't really know about replication factor at all, AFAICS. Currently when the partitions are increased we just use the replication factor inferred from partition 0. Ismael suggested to add this restriction to prevent this edge case. I don't see an error code specifically pertaining to topic actions on topics being deleted, so I guess INVALID_TOPIC_EXCEPTION would suffice for the deletion case. Thanks again, Tom
