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 #3348: [FLINK-5090] [network] Add metrics for details abo...
Date Tue, 14 Mar 2017 12:43:32 GMT
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3348#discussion_r105895733
  
    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/taskmanager/Task.java ---
    @@ -389,11 +389,20 @@ public Task(
     			++counter;
     		}
     
    +		invokableHasBeenCanceled = new AtomicBoolean(false);
    +
    +		// finally, create the executing thread, but do not start it
    +		executingThread = new Thread(TASK_THREADS_GROUP, this, taskNameWithSubtask);
    +
    +		// add metrics for buffers
    +		this.metrics.getIOMetricGroup().initializeBufferMetrics(this);
    +
     		// register detailed network metrics, if configured
     		if (tmConfig.getBoolean(TaskManagerOptions.NETWORK_DETAILED_METRICS_KEY)) {
    -			MetricGroup networkGroup = metricGroup.addGroup("Network"); // same as in MetricUtils.instantiateNetworkMetrics()
    -			MetricGroup outputGroup = networkGroup.addGroup("Output"); // this is optional
    -			MetricGroup inputGroup = networkGroup.addGroup("Input"); // this is optional
    +			// similar to MetricUtils.instantiateNetworkMetrics() but inside this IOMetricGroup
    +			MetricGroup networkGroup = this.metrics.getIOMetricGroup().addGroup("Network");
    --- End diff --
    
    The point of the IOMetricGroup is to keep a lot of details out of the TaskMetricGroup
without affecting the actual MetricGroup structure. IO metrics are handled a bit differently
than other metrics in that they are a) also stored in the ExecutionGraph and b) are used from
different parts of the code (like multiple RecordWriters). We preemptively moved this logic
into a separate class so that the TaskMG doesn't blow up over time.
    
    There isn't anything wrong with registering metrics/adding groups on it, they aren't lost
or anything. I'm only mentioning it since you modified existing code with something that is
equivalent.
    
    If we want there to be an actual "IO" group we only have to modify these 2 lines:
    ```
    TaskMetricGroup:
    this.ioMetrics = new TaskIOMetricGroup();
    =>
    this.ioMetrics = new TaskIOMetricGroup(addGroup("IO"));
    ```
    
    ```
    TaskIOMetricGroup:
    public TaskIOMetricGroup(TaskMetricGroup parent) {
    =>
    public TaskIOMetricGroup(MetricGroup parent) {
    ```


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