Hi Jason, Yea I think it could make sense. In this case I would rename the DeadFetcherThreadCount to DeadReplicaFetcherThreadCount and introduce the metric you're referring to as DeadLogDirFetcherThreadCount. I'll update the KIP to reflect this.
Viktor On Thu, Apr 25, 2019 at 8:07 PM Jason Gustafson <ja...@confluent.io> wrote: > Hi Viktor, > > This looks good. Just one question I had is whether we may as well cover > the log dir fetchers as well. > > Thanks, > Jason > > > On Thu, Apr 25, 2019 at 7:46 AM Viktor Somogyi-Vass < > viktorsomo...@gmail.com> > wrote: > > > Hi Folks, > > > > This thread sunk a bit but I'd like to bump it hoping to get some > feedback > > and/or votes. > > > > Thanks, > > Viktor > > > > On Thu, Mar 28, 2019 at 8:47 PM Viktor Somogyi-Vass < > > viktorsomo...@gmail.com> > > wrote: > > > > > Sorry, the end of the message cut off. > > > > > > So I tried to be consistent with the convention in LogManager, hence > the > > > hyphens and in AbstractFetcherManager, hence the camel case. It would > be > > > nice though to decide with one convention across the whole project, > > however > > > it requires a major refactor (especially for the components that > leverage > > > metrics for monitoring). > > > > > > Thanks, > > > Viktor > > > > > > On Thu, Mar 28, 2019 at 8:44 PM Viktor Somogyi-Vass < > > > viktorsomo...@gmail.com> wrote: > > > > > >> Hi Dhruvil, > > >> > > >> Thanks for the feedback and the vote. I fixed the typo in the KIP. > > >> The naming is interesting though. Unfortunately kafka overall is not > > >> consistent in metric naming but at least I tried to be consistent > among > > the > > >> other metrics used in LogManager > > >> > > >> On Thu, Mar 28, 2019 at 7:32 PM Dhruvil Shah <dhru...@confluent.io> > > >> wrote: > > >> > > >>> Thanks for the KIP, Viktor! This is a useful addition. +1 overall. > > >>> > > >>> Minor nits: > > >>> > I propose to add three gauge: DeadFetcherThreadCount for the > fetcher > > >>> threads, log-cleaner-dead-thread-count for the log cleaner. > > >>> I think you meant two instead of three. > > >>> > > >>> Also, would it make sense to name these metrics consistency, > something > > >>> like > > >>> `log-cleaner-dead-thread-count` and > > `replica-fetcher-dead-thread-count`? > > >>> > > >>> Thanks, > > >>> Dhruvil > > >>> > > >>> On Thu, Mar 28, 2019 at 11:27 AM Viktor Somogyi-Vass < > > >>> viktorsomo...@gmail.com> wrote: > > >>> > > >>> > Hi All, > > >>> > > > >>> > I'd like to start a vote on KIP-434. > > >>> > This basically would add a metrics to count dead threads in > > >>> > ReplicaFetcherManager and LogCleaner to allow monitoring systems to > > >>> alert > > >>> > based on this. > > >>> > > > >>> > The KIP link: > > >>> > > > >>> > > > >>> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-434%3A+Add+Replica+Fetcher+and+Log+Cleaner+Count+Metrics > > >>> > The > > >>> > PR: https://github.com/apache/kafka/pull/6514 > > >>> > > > >>> > I'd be happy to receive any votes or additional feedback/reviews > too. > > >>> > > > >>> > Thanks, > > >>> > Viktor > > >>> > > > >>> > > >> > > >
