Sorry that I was not clear. Yes, I was suggesting the state-specific counts were in addition to the simple task count you originally proposed. Thanks for taking my suggestion into account -- the updated KIP looks great.
Thanks for contributing this improvement, Cyrus! Best regards, Randall On Tue, Jun 4, 2019 at 6:35 PM Cyrus Vafadari <cy...@confluent.io> wrote: > Randall, > > I've updated the KIP to include all of your recommendations! > > Cyrus > > On Tue, Jun 4, 2019 at 2:55 PM Cyrus Vafadari <cy...@confluent.io> wrote: > > > Randall, > > > > I plan to update the public details section and the performance impact as > > you recommended. > > > > Regarding state-specific counts, I do agree this is a useful addition. > > Before I make the change, I'd like to agree that these state-specific > > counts should be in addition to the already-proposed total tasks count > > (even though might be redundant, it is robust against new/missed > connector > > states, and is a useful metric in its own right), yes? > > > > Cyrus > > > > On Tue, Jun 4, 2019 at 12:24 PM Randall Hauch <rha...@gmail.com> wrote: > > > >> Thanks, Cyrus -- this will be quite useful. I do have a few > >> comments/requests. > >> > >> Can you please be more specific about the public details about the > metric? > >> What is the MBean name on which the metric will appear? For example, the > >> AK > >> documentation ( > https://kafka.apache.org/documentation/#connect_monitoring > >> ) > >> defines all of the metrics an where they will appear, as does > >> > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-196%3A+Add+metrics+to+Kafka+Connect+framework > >> . > >> > >> Secondly, while a metric showing the total number of tasks is very > useful, > >> might it be worth considering also adding metrics for the number of > >> running > >> tasks, the number of paused tasks, and the number of failed tasks for a > >> connector. It might require using the herder's `connectorStatus(String > >> connectorName)` method instead, but that appears to be just as effective > >> at > >> using the local snapshot of the status store cache. > >> > >> Thirdly, it might be useful for the KIP to address the potential > >> performance impact of computing these methods. Again, IIUC, the herder > >> methods that the proposal mentions use the status and config stores > caches > >> only, so the impact should be negligible. > >> > >> Best regards, > >> > >> Randall > >> > >> On Sun, Jun 2, 2019 at 10:05 PM Ryanne Dolan <ryannedo...@gmail.com> > >> wrote: > >> > >> > Cyrus, I agree this would be useful. > >> > > >> > Ryanne > >> > > >> > On Fri, May 31, 2019, 7:10 PM Oleksandr Diachenko < > >> odiache...@apache.org> > >> > wrote: > >> > > >> > > > >> > > > >> > > On 2019/05/30 06:06:12, Cyrus Vafadari <cy...@confluent.io> wrote: > >> > > > Hello Dev, > >> > > > > >> > > > I'd like to start the discussion of KIP-475: New Metric to Measure > >> > Number > >> > > > of Tasks on a Connector. > >> > > > > >> > > > >> > > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-475%3A+New+Metric+to+Measure+Number+of+Tasks+on+a+Connector > >> > > > > >> > > > The proposal is pretty straightforward -- to add a new metric to > >> > Connect > >> > > to > >> > > > measure the number of tasks on a Connector. Currently, we support > >> this > >> > on > >> > > > Worker level, so this KIP just adds another metric to support this > >> > > > per-connector. > >> > > > > >> > > > There is also a PR: > >> > > > https://github.com/apache/kafka/pull/6843 > >> > > > > >> > > > Thanks, > >> > > > > >> > > > Cyrus > >> > > > > >> > > > >> > > Hi Cyrus, > >> > > > >> > > That sounds like a useful addition. > >> > > > >> > > Regards, Alex. > >> > > > >> > > >> > > >
