Hi all,
A summary of our discussion:
201. Q: Cluster updates in backward compatible way.
A: Add topicConfigs map property and change constructor, this
shouldn't break Consumer/Producer since TMR is used in NetworkClient,
not directly by Consumer/Producer.
300. Q: Can we merge AlterTopic and ReassignPartitions requests?
A: It looks like in terms of Wire Protocol partition reassignment
can
be just an application of AlterTopicRequest. On the AdminClient side we can
split this into two separate methods, if needed.
Some additional items that were added today:
400. Q: Do we need ListTopicsRequest, we can use TMR for this purpose.
A: The answer depends on whether we can leverage ListTopics in
consumer/producer, because the only benefit of the ListTopics is performance
optimization, otherwise it doesn't worth it.
401. Q: AdminClient.topicExists - do we need it?
A: AdminClient.listTopics should be sufficient.
402. Review AdminClient API and use separate objects instead of collections
for methods arguments / return results (e.g. preferredReplica accepts
Map<String, List<Int>>
might be better to add separate java object)
403. Error number in KIP-4 (100x). Currently there are no dedicated ranges
for errors, we will probably continue doing it this way.
404. There were some concerns again about the asynchronous semantics
of the admin requests. Jun and Jay to agree separately how we want
to handle it.
Please add / correct me if I missed something.
Thanks,
Andrii Biletskyi
On Tue, Apr 7, 2015 at 4:11 PM, Andrii Biletskyi <
andrii.bilets...@stealth.ly> wrote:
> 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
>
