[GitHub] flink pull request #4869: [FLINK-7843][docs] Improve documentation for metri...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/flink/pull/4869


---

[GitHub] flink pull request #4869: [FLINK-7843][docs] Improve documentation for metri...

[yew1eb]

Github user yew1eb commented on a diff in the pull request:

https://github.com/apache/flink/pull/4869#discussion_r147100695
  
--- Diff: docs/monitoring/metrics.md ---
@@ -856,66 +918,80 @@ Thus, in order to infer the metric identifier:
 
   
 
-  Scope
-  Metrics
-  Description
+  Scope
+  Metrics
+  Description
+  Type
 
   
   
 
   Task
   currentLowWatermark
-  The lowest watermark this task has received.
+  The lowest watermark this task has received (in 
milliseconds).
+  Gauge
 
 
   numBytesInLocal
   The total number of bytes this task has read from a local 
source.
+  Counter
--- End diff --

Yes.
But, eg. 
**numBytesInRemote** The total number of bytes this task has read from a 
remote source.
**numBytesInRemotePerSecond**  The number of bytes this task reads from a 
remote source per second.

Users can easily know that these metrics units through this description, 
will not be confused.

So, I think we do not need to add additional unit descriptions to these 
metrics (except latency metric).


---

[GitHub] flink pull request #4869: [FLINK-7843][docs] Improve documentation for metri...

[yew1eb]

Github user yew1eb commented on a diff in the pull request:

https://github.com/apache/flink/pull/4869#discussion_r147092709
  
--- Diff: docs/monitoring/metrics.md ---
@@ -856,66 +918,80 @@ Thus, in order to infer the metric identifier:
 
   
 
-  Scope
-  Metrics
-  Description
+  Scope
+  Metrics
+  Description
+  Type
 
   
   
 
   Task
   currentLowWatermark
-  The lowest watermark this task has received.
+  The lowest watermark this task has received (in 
milliseconds).
--- End diff --

I see that `currentLowWatermark` is derived from the 
`Watermark.getTimestamp ()` method,

```
/**
 * Returns the timestamp associated with this {@link Watermark} in 
milliseconds.
 */
public long getTimestamp() {
return timestamp;
}
```
so  `currentLowWatermark` must be milliseconds ?


---

[GitHub] flink pull request #4869: [FLINK-7843][docs] Improve documentation for metri...

[yew1eb]

Github user yew1eb commented on a diff in the pull request:

https://github.com/apache/flink/pull/4869#discussion_r147091360
  
--- Diff: docs/monitoring/metrics.md ---
@@ -801,53 +849,67 @@ Thus, in order to infer the metric identifier:
 
   
 
-  Scope
-  Metrics
-  Description
+  Scope
+  Metrics
+  Description
+  Type
 
   
   
 
   Job (only available on 
JobManager)
   lastCheckpointDuration
-  The time it took to complete the last checkpoint.
+  The time it took to complete the last checkpoint (in 
milliseconds).
+  Gauge
 
 
   lastCheckpointSize
-  The total size of the last checkpoint.
+  The total size of the last checkpoint (in bytes).
+  Gauge
 
 
   lastCheckpointExternalPath
   The path where the last external checkpoint was stored.
+  Gauge
 
 
   lastCheckpointRestoreTimestamp
-  Timestamp when the last checkpoint was restored at the 
coordinator.
+  Timestamp when the last checkpoint was restored at the 
coordinator (in milliseconds).
--- End diff --

Yes.
but the "milliseconds" not cause confusion to the users. 


---

[GitHub] flink pull request #4869: [FLINK-7843][docs] Improve documentation for metri...

[yew1eb]

Github user yew1eb commented on a diff in the pull request:

https://github.com/apache/flink/pull/4869#discussion_r147090946
  
--- Diff: docs/monitoring/metrics.md ---
@@ -801,53 +849,67 @@ Thus, in order to infer the metric identifier:
 
   
 
-  Scope
-  Metrics
-  Description
+  Scope
+  Metrics
+  Description
+  Type
 
   
   
 
   Job (only available on 
JobManager)
   lastCheckpointDuration
-  The time it took to complete the last checkpoint.
+  The time it took to complete the last checkpoint (in 
milliseconds).
+  Gauge
 
 
   lastCheckpointSize
-  The total size of the last checkpoint.
+  The total size of the last checkpoint (in bytes).
+  Gauge
 
 
   lastCheckpointExternalPath
   The path where the last external checkpoint was stored.
+  Gauge
 
 
   lastCheckpointRestoreTimestamp
-  Timestamp when the last checkpoint was restored at the 
coordinator.
+  Timestamp when the last checkpoint was restored at the 
coordinator (in milliseconds).
--- End diff --

Yes. 
but the "milliseconds" not cause confusion to user


---

[GitHub] flink pull request #4869: [FLINK-7843][docs] Improve documentation for metri...

[yew1eb]

Github user yew1eb commented on a diff in the pull request:

https://github.com/apache/flink/pull/4869#discussion_r147090107
  
--- Diff: docs/monitoring/metrics.md ---
@@ -543,62 +546,75 @@ Thus, in order to infer the metric identifier:

 

-  Scope
-  Infix  
-  Metrics
   
-  Description
   
+  Scope
+  Infix  
+  Metrics
   
+  Description
+  Type
   
   

 

   Job-/TaskManager
   Status.JVM.Memory
   Heap.Used
-  The amount of heap memory currently used.
+  The amount of heap memory currently used (in bytes).
+  Gauge
 
 
   Heap.Committed
-  The amount of heap memory guaranteed to be available to the 
JVM.
+  The amount of heap memory guaranteed to be available to the JVM 
(in bytes).
+  Gauge
 
 
   Heap.Max
-  The maximum amount of heap memory that can be used for memory 
management.
+  The maximum amount of heap memory that can be used for memory 
management (in bytes).
+  Gauge
 
 
   NonHeap.Used
-  The amount of non-heap memory currently used.
+  The amount of non-heap memory currently used (in bytes).
+  Gauge
 
 
   NonHeap.Committed
-  The amount of non-heap memory guaranteed to be available to the 
JVM.
+  The amount of non-heap memory guaranteed to be available to the 
JVM (in bytes).
+  Gauge
 
 
   NonHeap.Max
-  The maximum amount of non-heap memory that can be used for 
memory management.
+  The maximum amount of non-heap memory that can be used for 
memory management (in bytes).
+  Gauge
 
 
   Direct.Count
-  The number of buffers in the direct buffer pool.
+  The number of buffers in the direct buffer pool (in bytes).
--- End diff --

will change


---

[GitHub] flink pull request #4869: [FLINK-7843][docs] Improve documentation for metri...

[yew1eb]

Github user yew1eb commented on a diff in the pull request:

https://github.com/apache/flink/pull/4869#discussion_r147090125
  
--- Diff: docs/monitoring/metrics.md ---
@@ -543,62 +546,75 @@ Thus, in order to infer the metric identifier:

 

-  Scope
-  Infix  
-  Metrics
   
-  Description
   
+  Scope
+  Infix  
+  Metrics
   
+  Description
+  Type
   
   

 

   Job-/TaskManager
   Status.JVM.Memory
   Heap.Used
-  The amount of heap memory currently used.
+  The amount of heap memory currently used (in bytes).
+  Gauge
 
 
   Heap.Committed
-  The amount of heap memory guaranteed to be available to the 
JVM.
+  The amount of heap memory guaranteed to be available to the JVM 
(in bytes).
+  Gauge
 
 
   Heap.Max
-  The maximum amount of heap memory that can be used for memory 
management.
+  The maximum amount of heap memory that can be used for memory 
management (in bytes).
+  Gauge
 
 
   NonHeap.Used
-  The amount of non-heap memory currently used.
+  The amount of non-heap memory currently used (in bytes).
+  Gauge
 
 
   NonHeap.Committed
-  The amount of non-heap memory guaranteed to be available to the 
JVM.
+  The amount of non-heap memory guaranteed to be available to the 
JVM (in bytes).
+  Gauge
 
 
   NonHeap.Max
-  The maximum amount of non-heap memory that can be used for 
memory management.
+  The maximum amount of non-heap memory that can be used for 
memory management (in bytes).
+  Gauge
 
 
   Direct.Count
-  The number of buffers in the direct buffer pool.
+  The number of buffers in the direct buffer pool (in bytes).
+  Gauge
 
 
   Direct.MemoryUsed
-  The amount of memory used by the JVM for the direct buffer 
pool.
+  The amount of memory used by the JVM for the direct buffer pool 
(in bytes).
+  Gauge
 
 
   Direct.TotalCapacity
-  The total capacity of all buffers in the direct buffer pool.
+  The total capacity of all buffers in the direct buffer pool (in 
bytes).
+  Gauge
 
 
   Mapped.Count
-  The number of buffers in the mapped buffer pool.
+  The number of buffers in the mapped buffer pool (in bytes).
--- End diff --

will change


---

[GitHub] flink pull request #4869: [FLINK-7843][docs] Improve documentation for metri...

[zentol]

Github user zentol commented on a diff in the pull request:

https://github.com/apache/flink/pull/4869#discussion_r146798931
  
--- Diff: docs/monitoring/metrics.md ---
@@ -543,62 +546,75 @@ Thus, in order to infer the metric identifier:

 

-  Scope
-  Infix  
-  Metrics
   
-  Description
   
+  Scope
+  Infix  
+  Metrics
   
+  Description
+  Type
   
   

 

   Job-/TaskManager
   Status.JVM.Memory
   Heap.Used
-  The amount of heap memory currently used.
+  The amount of heap memory currently used (in bytes).
+  Gauge
 
 
   Heap.Committed
-  The amount of heap memory guaranteed to be available to the 
JVM.
+  The amount of heap memory guaranteed to be available to the JVM 
(in bytes).
+  Gauge
 
 
   Heap.Max
-  The maximum amount of heap memory that can be used for memory 
management.
+  The maximum amount of heap memory that can be used for memory 
management (in bytes).
+  Gauge
 
 
   NonHeap.Used
-  The amount of non-heap memory currently used.
+  The amount of non-heap memory currently used (in bytes).
+  Gauge
 
 
   NonHeap.Committed
-  The amount of non-heap memory guaranteed to be available to the 
JVM.
+  The amount of non-heap memory guaranteed to be available to the 
JVM (in bytes).
+  Gauge
 
 
   NonHeap.Max
-  The maximum amount of non-heap memory that can be used for 
memory management.
+  The maximum amount of non-heap memory that can be used for 
memory management (in bytes).
+  Gauge
 
 
   Direct.Count
-  The number of buffers in the direct buffer pool.
+  The number of buffers in the direct buffer pool (in bytes).
--- End diff --

the unit is not bytes, but just a simple count.


---

[GitHub] flink pull request #4869: [FLINK-7843][docs] Improve documentation for metri...

[zentol]

Github user zentol commented on a diff in the pull request:

https://github.com/apache/flink/pull/4869#discussion_r146799826
  
--- Diff: docs/monitoring/metrics.md ---
@@ -856,66 +918,80 @@ Thus, in order to infer the metric identifier:
 
   
 
-  Scope
-  Metrics
-  Description
+  Scope
+  Metrics
+  Description
+  Type
 
   
   
 
   Task
   currentLowWatermark
-  The lowest watermark this task has received.
+  The lowest watermark this task has received (in 
milliseconds).
--- End diff --

AFAIK the timeunit for watermarks depends on the watermark assigned used, 
so it may not be milliseconds. @aljoscha 


---

[GitHub] flink pull request #4869: [FLINK-7843][docs] Improve documentation for metri...

[zentol]

Github user zentol commented on a diff in the pull request:

https://github.com/apache/flink/pull/4869#discussion_r146798990
  
--- Diff: docs/monitoring/metrics.md ---
@@ -543,62 +546,75 @@ Thus, in order to infer the metric identifier:

 

-  Scope
-  Infix  
-  Metrics
   
-  Description
   
+  Scope
+  Infix  
+  Metrics
   
+  Description
+  Type
   
   

 

   Job-/TaskManager
   Status.JVM.Memory
   Heap.Used
-  The amount of heap memory currently used.
+  The amount of heap memory currently used (in bytes).
+  Gauge
 
 
   Heap.Committed
-  The amount of heap memory guaranteed to be available to the 
JVM.
+  The amount of heap memory guaranteed to be available to the JVM 
(in bytes).
+  Gauge
 
 
   Heap.Max
-  The maximum amount of heap memory that can be used for memory 
management.
+  The maximum amount of heap memory that can be used for memory 
management (in bytes).
+  Gauge
 
 
   NonHeap.Used
-  The amount of non-heap memory currently used.
+  The amount of non-heap memory currently used (in bytes).
+  Gauge
 
 
   NonHeap.Committed
-  The amount of non-heap memory guaranteed to be available to the 
JVM.
+  The amount of non-heap memory guaranteed to be available to the 
JVM (in bytes).
+  Gauge
 
 
   NonHeap.Max
-  The maximum amount of non-heap memory that can be used for 
memory management.
+  The maximum amount of non-heap memory that can be used for 
memory management (in bytes).
+  Gauge
 
 
   Direct.Count
-  The number of buffers in the direct buffer pool.
+  The number of buffers in the direct buffer pool (in bytes).
+  Gauge
 
 
   Direct.MemoryUsed
-  The amount of memory used by the JVM for the direct buffer 
pool.
+  The amount of memory used by the JVM for the direct buffer pool 
(in bytes).
+  Gauge
 
 
   Direct.TotalCapacity
-  The total capacity of all buffers in the direct buffer pool.
+  The total capacity of all buffers in the direct buffer pool (in 
bytes).
+  Gauge
 
 
   Mapped.Count
-  The number of buffers in the mapped buffer pool.
+  The number of buffers in the mapped buffer pool (in bytes).
--- End diff --

the unit is not bytes, but just a simple count.


---

[GitHub] flink pull request #4869: [FLINK-7843][docs] Improve documentation for metri...

[zentol]

Github user zentol commented on a diff in the pull request:

https://github.com/apache/flink/pull/4869#discussion_r146799279
  
--- Diff: docs/monitoring/metrics.md ---
@@ -801,53 +849,67 @@ Thus, in order to infer the metric identifier:
 
   
 
-  Scope
-  Metrics
-  Description
+  Scope
+  Metrics
+  Description
+  Type
 
   
   
 
   Job (only available on 
JobManager)
   lastCheckpointDuration
-  The time it took to complete the last checkpoint.
+  The time it took to complete the last checkpoint (in 
milliseconds).
+  Gauge
 
 
   lastCheckpointSize
-  The total size of the last checkpoint.
+  The total size of the last checkpoint (in bytes).
+  Gauge
 
 
   lastCheckpointExternalPath
   The path where the last external checkpoint was stored.
+  Gauge
 
 
   lastCheckpointRestoreTimestamp
-  Timestamp when the last checkpoint was restored at the 
coordinator.
+  Timestamp when the last checkpoint was restored at the 
coordinator (in milliseconds).
--- End diff --

"milliseconds" as a unit is not sufficient; it's missing a reference port 
(like the UNIX epoch).


---

[GitHub] flink pull request #4869: [FLINK-7843][docs] Improve documentation for metri...

[zentol]

Github user zentol commented on a diff in the pull request:

https://github.com/apache/flink/pull/4869#discussion_r146800048
  
--- Diff: docs/monitoring/metrics.md ---
@@ -856,66 +918,80 @@ Thus, in order to infer the metric identifier:
 
   
 
-  Scope
-  Metrics
-  Description
+  Scope
+  Metrics
+  Description
+  Type
 
   
   
 
   Task
   currentLowWatermark
-  The lowest watermark this task has received.
+  The lowest watermark this task has received (in 
milliseconds).
+  Gauge
 
 
   numBytesInLocal
   The total number of bytes this task has read from a local 
source.
+  Counter
--- End diff --

these metrics are all missing units. (bytes, records, and in particular 
latency (milliseconds)


---

[GitHub] flink pull request #4869: [FLINK-7843][docs] Improve documentation for metri...

[yew1eb]

GitHub user yew1eb opened a pull request:

https://github.com/apache/flink/pull/4869

[FLINK-7843][docs] Improve documentation for metrics


* Add a column that the **Type** of metric. eg. Counters, Gauges, 
Histograms and Meters
* Modify the **Description** of the metric，Add unit description. eg. in 
bytes, in megabytes, in nanoseconds, in milliseconds

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/yew1eb/flink FLINK-7843

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/flink/pull/4869.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #4869


commit 99ab8feb8536a8f22f9abd8a83b1d854d80a513f
Author: yew1eb 
Date:   2017-10-20T07:55:21Z

add a column that the type of metrics




---