flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From zentol <...@git.apache.org>
Subject [GitHub] flink pull request #2517: [FLINK-4564] [metrics] Delimiter should be configu...
Date Wed, 28 Sep 2016 10:33:47 GMT
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/2517#discussion_r80886088
  
    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/metrics/MetricRegistry.java
---
    @@ -216,17 +244,20 @@ public ScopeFormats getScopeFormats() {
     	 * @param metricName  the name of the metric
     	 * @param group       the group that contains the metric
     	 */
    -	public void register(Metric metric, String metricName, MetricGroup group) {
    +	public void register(Metric metric, String metricName, AbstractMetricGroup group) {
     		try {
     			if (reporters != null) {
    -				for (MetricReporter reporter : reporters) {
    +				for (int i = 0; i < reporters.size(); i++) {
    +					MetricReporter reporter = reporters.get(i);
     					if (reporter != null) {
    -						reporter.notifyOfAddedMetric(metric, metricName, group);
    +						FrontMetricGroup front = new FrontMetricGroup(i);
    --- End diff --
    
    we will now create a new object every time we add a new metric. Instead, keep a single
`FrontMetricGroup` instance in the registry, and add a `setIndex()` method to it that you
call here along with `setReference()`.
    
    but, i just realized that without this you would end up with concurrency issues since
multiple register calls can be active at the same time...I'll have to think about this for
a bit.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message