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 > >>> > > >>> > >> >
