Hey Kevin, Thanks for the KIP! This sounds like a very useful feature to have and makes excellent use of the new AdminClient introduced in KIP-377 in my opinion. I believe it's perfectly fine to start a vote thread on the fact that KIP-377 was accepted already.
On Mon, Nov 5, 2018 at 3:03 PM Kevin Lu <lu.ke...@berkeley.edu> wrote: > Bumping this to hopefully get a couple more opinions. > > Since KIP-377 has been accepted (it adds the --bootstrap-server option > needed for this KIP), do we think it is okay to proceed to vote on this KIP > without KIP-377 fully merged yet? > > Thanks. > > Regards, > Kevin > > On Mon, Oct 22, 2018 at 6:32 PM Kevin Lu <lu.ke...@berkeley.edu> wrote: > > > Hi Mickael, > > > > Thanks for the suggestion for getting the "computed" configuration from > > AdminClient.describeConfigs(). That's exactly what I was looking for! > > > > I have updated the KIP to use AdminClient.describeConfigs(), and included > > a code snippet. Please take a look. > > > > Since KIP-377 proposes the same bootstrap-server option, I've put a > > dependency for this KIP to have KIP-377 implemented first so we don't > > duplicate work or introduce conflicts. I'll give the other thread a bump, > > but I think we can still continue discussion/voting for this KIP. > > > > Thoughts? > > > > Regards, > > Kevin > > > > On Mon, Oct 22, 2018 at 4:01 AM Mickael Maison <mickael.mai...@gmail.com > > > > wrote: > > > >> It might be worth syncing with KIP-377 which is already planning to > >> make TopicCommand use the AdminClient and add a --bootstrap-server > >> argument. > >> > >> Also in the proposed changes section, you mention the challenge of > >> finding the topic min ISR configuration. Using the > >> AdminClient.describeConfigs() API, > >> you directly get the "computed" configuration for topics. If the topic > >> is using the default config from the broker the configuration source > >> will be set to "DEFAULT_CONFIG". In case, the configuration was > >> specified during creation, the source will be set to > >> "DYNAMIC_TOPIC_CONFIG". So there's no need to query Zookeeper. > >> On Fri, Oct 19, 2018 at 5:02 PM Kevin Lu <lu.ke...@berkeley.edu> wrote: > >> > > >> > Bumping this as I have added some additional details. > >> > > >> > This change will require adding a "--bootstrap-server" flag to > identify > >> the > >> > current broker/cluster configured "min.insync.replicas". > >> > > >> > Regards, > >> > Kevin > >> > > >> > On Fri, Oct 12, 2018 at 4:19 PM Kevin Lu <lu.ke...@berkeley.edu> > wrote: > >> > > >> > > Hi All, > >> > > > >> > > After some feedback, I have reformulated KIP-351 > >> > > < > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-351%3A+Add+--under-min-isr+option+to+describe+topics+command > >> > > >> > > . > >> > > > >> > > This KIP proposes an additional "--under-min-isr" option in > >> TopicCommand > >> > > to show topic partitions which are under the configured > >> > > "min.insync.replicas" to help operators identify which topic > >> partitions > >> > > need immediate fixing. > >> > > > >> > > Please take a look and provide some feedback! > >> > > > >> > > Thanks! > >> > > > >> > > Regards, > >> > > Kevin > >> > > > >> > > > -- Best, Stanislav
