Hi, Tom, 2. Thanks for the explanation. That makes sense and we can leave this as it is.
Jun On Tue, Sep 12, 2017 at 10:46 AM, Tom Bentley <t.j.bent...@gmail.com> wrote: > 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 >
