Hi all, I wasn't able to send email to our thread (it says we exceeded message size limit :)). So I'm starting the new one.
Jun, Thanks again for the review. Answering your comments: 201. I'm not sure I understand how can we evolve Cluster in backward compatible way. In my understanding topic configs are not returned currently - in TMR_V0. Thus we need to add new property in Cluster - smth like private final Map<String, List<ConfigEntry>> topicConfigs; Which affects Cluster constructor, which is used in MetadataResponse.java - not sure whether we can change Cluster this way so it's backward compatible, I suppose - no. Let me know if I'm missing something... 300. Hm, so you propose to give up ReassignPartition as a separate command? That's interesting, let's discuss it today in detail. Two small points here: 1) afaik currently replica-assignment argument in alter-topic (from TopicCommand) doesn't reassign partitions, it lets users specify replica assignment for newly added partition (AddPartitionsListener) 2) ReassignPartitions command involves a little bit more than just changing replica assignment in zk. People are struggling with partition reassignment so I think it's good to have explicit request for it so we can handle it independently, also as mentioned earlier we'll probably add in future some better status check procedure for this long-running request. 301. Good point. We also agreed to use clientId as an identifier for the requester - whether it's a producer client or admin. I think we can go with -1/-1 approach. 302. Again, as said above replica-assignment in alter-topic doesn't change replica assignment of the existing partitions. But we can think about it in general - how can we change topic replication factor? The easy way - we don't need it, we can use reassign partitions. Not sure whether we want to add special logic to treat this case... 303.1. Okay, sure, I'll generalize topicExists(). 303.2. I think, yes, we need separate verify methods as a status check procedure, because respective requests are long running, and CLI user potentially will asynchronously call reassign-partitions, do other stuff (e.g. create topics) periodically checking status of the partition reassignment. Anyway we'll have to implement this logic because it's a criterion of the completed Future of the reassign partitions async call, we'll to have make those methods just public. 303.3. If preferredReplica returns Future<Map<String, Errors>> than what is an error in terms of preferred replica leader election? As I understand we can only check whether it has succeeded (leader == AR.head) or not _yet_. 304.1. Sure, let's add timeout to reassign/preferred replica. 304.2. This can be finalized after we discuss 300. 305. Misprints - thanks, fixed. Thanks, Andrii Biletskyi
