<!here>

we can move the metrics plugin discussion here

I think Xiaoman has some question about the listener implementation

<!here> I think the major problem here is that the old plugins that implemented

MetricsRegistryRegistrationListener

makes the server start process hang, without any clue in our log to trace it

I agree that the interface change is a good direction in design; but I am a bit concerned about the other Pinot users that did the same thing will have trouble debugging

It took me quite a few hours digging until I figure out

Even

jstack

does not help

I don't have a better solution to this though. Maybe a proper communication is the only way

Originally I thought it was another interface change, but here we are dealing with dependencies too, so it is hard to find a good solution

After reimplementing my plugin the pinot server now starts properly with my metrics plugin

Hey Xiaoman, thanks for reaching out! I understand your concern of removing the methods in

PinotMetricsRegistryListener

. While

PinotMetricsRegistryListener

is just a wrapper. The wrapper’s method won’t be registered to the actual registry; instead, it’s the actual yammer’s listener which methods will be invoked. That’s why I think you want to add the method like

void onMetricsRegistryRegistered(MetricsRegistry metricsRegistry);

. While that’ll make the repo unclean, because we still have to pull in the actual yammer dependencies to pinot’s code. One thing I’d suggest is to initialize an actual Yammer listener and pass it as the param to the constructor.

this is the sample code on how to handle listener in LinkedIn (not open source), hope that will give you some idea:

@Override
  public void onMetricsRegistryRegistered(final PinotMetricsRegistry metricsRegistry) {
    MetricsRegistryListener metricsRegistryListener = new MetricsRegistryListener() {
      @Override
      public void onMetricAdded(MetricName metricName, Metric metric) {
        // do sth
        }
      }

      @Override
      public void onMetricRemoved(MetricName metricName) {
        // do sth
      }
    };
    metricsRegistry.addListener(new YammerMetricsRegistryListener(metricsRegistryListener));
  }

Thanks. I have got mine working after recompiling. Mostly it is because of Plugins are loaded in runtime by reflection, and then Plugins built agains old Pinot got loaded without any check.

I see. Glad that was resolved. Usually the build will fail at compile time, then we should do some changes on addressing the new code

@User has left the channel