You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@gobblin.apache.org by GitBox <gi...@apache.org> on 2020/01/13 01:18:15 UTC

[GitHub] [incubator-gobblin] sv2000 opened a new pull request #2864: GOBBLIN-1018: Report GC counts and durations from Gobblin containers …

sv2000 opened a new pull request #2864: GOBBLIN-1018: Report GC counts and durations from Gobblin containers …
URL: https://github.com/apache/incubator-gobblin/pull/2864
 
 
   …metrics service
   
   Dear Gobblin maintainers,
   
   Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!
   
   
   ### JIRA
   - [x] My PR addresses the following [Gobblin JIRA](https://issues.apache.org/jira/browse/GOBBLIN/) issues and references them in the PR title. For example, "[GOBBLIN-XXX] My Gobblin PR"
       - https://issues.apache.org/jira/browse/GOBBLIN-1018
   
   
   ### Description
   - [x] Here are some details about my PR, including screenshots (if applicable):
   The container health metrics service should report minor/major GC counts and durations.
   
   ### Tests
   - [x] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason:
   Enhanced tests in ContainerHealthMetricsServiceTest class.
   
   ### Commits
   - [x] My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
       1. Subject is separated from body by a blank line
       2. Subject is limited to 50 characters
       3. Subject does not end with a period
       4. Subject uses the imperative mood ("add", not "adding")
       5. Body wraps at 72 characters
       6. Body explains "what" and "why", not "how"
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] asfgit closed pull request #2864: GOBBLIN-1018: Report GC counts and durations from Gobblin containers …

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #2864: GOBBLIN-1018: Report GC counts and durations from Gobblin containers …
URL: https://github.com/apache/incubator-gobblin/pull/2864
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] codecov-io edited a comment on issue #2864: GOBBLIN-1018: Report GC counts and durations from Gobblin containers …

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #2864: GOBBLIN-1018: Report GC counts and durations from Gobblin containers …
URL: https://github.com/apache/incubator-gobblin/pull/2864#issuecomment-573483935
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2864?src=pr&el=h1) Report
   > Merging [#2864](https://codecov.io/gh/apache/incubator-gobblin/pull/2864?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/d18783df7ab29b161cca59ca907fb47e1366b358?src=pr&el=desc) will **decrease** coverage by `40.69%`.
   > The diff coverage is `0%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/graphs/tree.svg?width=650&token=4MgURJ0bGc&height=150&src=pr)](https://codecov.io/gh/apache/incubator-gobblin/pull/2864?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff             @@
   ##             master   #2864      +/-   ##
   ===========================================
   - Coverage      44.8%    4.1%   -40.7%     
   + Complexity     8933     747    -8186     
   ===========================================
     Files          1917    1917              
     Lines         72131   72172      +41     
     Branches       7956    7961       +5     
   ===========================================
   - Hits          32316    2961   -29355     
   - Misses        36832   68892   +32060     
   + Partials       2983     319    -2664
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2864?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...apache/gobblin/cluster/ContainerHealthMetrics.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvQ29udGFpbmVySGVhbHRoTWV0cmljcy5qYXZh) | `0% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | [...gobblin/cluster/ContainerHealthMetricsService.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvQ29udGFpbmVySGVhbHRoTWV0cmljc1NlcnZpY2UuamF2YQ==) | `0% <0%> (-78.58%)` | `0 <0> (-5)` | |
   | [...n/converter/AvroStringFieldDecryptorConverter.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tY3J5cHRvLXByb3ZpZGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NvbnZlcnRlci9BdnJvU3RyaW5nRmllbGREZWNyeXB0b3JDb252ZXJ0ZXIuamF2YQ==) | `0% <0%> (-100%)` | `0% <0%> (-2%)` | |
   | [...he/gobblin/cluster/TaskRunnerSuiteThreadModel.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvVGFza1J1bm5lclN1aXRlVGhyZWFkTW9kZWwuamF2YQ==) | `0% <0%> (-100%)` | `0% <0%> (-5%)` | |
   | [...n/mapreduce/avro/AvroKeyCompactorOutputFormat.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1jb21wYWN0aW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NvbXBhY3Rpb24vbWFwcmVkdWNlL2F2cm8vQXZyb0tleUNvbXBhY3Rvck91dHB1dEZvcm1hdC5qYXZh) | `0% <0%> (-100%)` | `0% <0%> (-3%)` | |
   | [...apache/gobblin/fork/CopyNotSupportedException.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZm9yay9Db3B5Tm90U3VwcG9ydGVkRXhjZXB0aW9uLmphdmE=) | `0% <0%> (-100%)` | `0% <0%> (-1%)` | |
   | [.../gobblin/kafka/writer/KafkaWriterCommonConfig.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4ta2Fma2EtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2thZmthL3dyaXRlci9LYWZrYVdyaXRlckNvbW1vbkNvbmZpZy5qYXZh) | `0% <0%> (-100%)` | `0% <0%> (-7%)` | |
   | [...ker/task/TaskLevelPolicyCheckerBuilderFactory.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3F1YWxpdHljaGVja2VyL3Rhc2svVGFza0xldmVsUG9saWN5Q2hlY2tlckJ1aWxkZXJGYWN0b3J5LmphdmE=) | `0% <0%> (-100%)` | `0% <0%> (-2%)` | |
   | [...bblin/data/management/copy/AllEqualComparator.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvQWxsRXF1YWxDb21wYXJhdG9yLmphdmE=) | `0% <0%> (-100%)` | `0% <0%> (-2%)` | |
   | [...blin/converter/string/ObjectToStringConverter.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NvbnZlcnRlci9zdHJpbmcvT2JqZWN0VG9TdHJpbmdDb252ZXJ0ZXIuamF2YQ==) | `0% <0%> (-100%)` | `0% <0%> (-3%)` | |
   | ... and [1140 more](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2864?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2864?src=pr&el=footer). Last update [d18783d...926a86c](https://codecov.io/gh/apache/incubator-gobblin/pull/2864?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] codecov-io commented on issue #2864: GOBBLIN-1018: Report GC counts and durations from Gobblin containers …

Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #2864: GOBBLIN-1018: Report GC counts and durations from Gobblin containers …
URL: https://github.com/apache/incubator-gobblin/pull/2864#issuecomment-573483935
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2864?src=pr&el=h1) Report
   > Merging [#2864](https://codecov.io/gh/apache/incubator-gobblin/pull/2864?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/d18783df7ab29b161cca59ca907fb47e1366b358?src=pr&el=desc) will **increase** coverage by `0.99%`.
   > The diff coverage is `89.55%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/graphs/tree.svg?width=650&token=4MgURJ0bGc&height=150&src=pr)](https://codecov.io/gh/apache/incubator-gobblin/pull/2864?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #2864      +/-   ##
   ============================================
   + Coverage      44.8%   45.79%   +0.99%     
   - Complexity     8933     9110     +177     
   ============================================
     Files          1917     1917              
     Lines         72131    72176      +45     
     Branches       7956     7962       +6     
   ============================================
   + Hits          32316    33052     +736     
   + Misses        36832    36099     -733     
   - Partials       2983     3025      +42
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2864?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...apache/gobblin/cluster/ContainerHealthMetrics.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvQ29udGFpbmVySGVhbHRoTWV0cmljcy5qYXZh) | `0% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | [...gobblin/cluster/ContainerHealthMetricsService.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvQ29udGFpbmVySGVhbHRoTWV0cmljc1NlcnZpY2UuamF2YQ==) | `92.07% <89.55%> (+13.5%)` | `10 <5> (+5)` | :arrow_up: |
   | [...in/java/org/apache/gobblin/cluster/HelixUtils.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvSGVsaXhVdGlscy5qYXZh) | `32.71% <0%> (-2.81%)` | `11% <0%> (-1%)` | |
   | [...lin/restli/throttling/ZookeeperLeaderElection.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi10aHJvdHRsaW5nLXNlcnZpY2UvZ29iYmxpbi10aHJvdHRsaW5nLXNlcnZpY2Utc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3Jlc3RsaS90aHJvdHRsaW5nL1pvb2tlZXBlckxlYWRlckVsZWN0aW9uLmphdmE=) | `70% <0%> (-2.23%)` | `13% <0%> (ø)` | |
   | [...lin/elasticsearch/writer/FutureCallbackHolder.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tZWxhc3RpY3NlYXJjaC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9lbGFzdGljc2VhcmNoL3dyaXRlci9GdXR1cmVDYWxsYmFja0hvbGRlci5qYXZh) | `61.42% <0%> (-1.43%)` | `4% <0%> (ø)` | |
   | [.../org/apache/gobblin/cluster/GobblinTaskRunner.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvR29iYmxpblRhc2tSdW5uZXIuamF2YQ==) | `63.88% <0%> (-0.93%)` | `27% <0%> (-1%)` | |
   | [...src/main/java/org/apache/gobblin/runtime/Task.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvVGFzay5qYXZh) | `67.05% <0%> (+0.23%)` | `82% <0%> (+1%)` | :arrow_up: |
   | [.../org/apache/gobblin/runtime/SafeDatasetCommit.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvU2FmZURhdGFzZXRDb21taXQuamF2YQ==) | `47.31% <0%> (+0.48%)` | `29% <0%> (ø)` | :arrow_down: |
   | [...main/java/org/apache/gobblin/util/HadoopUtils.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvSGFkb29wVXRpbHMuamF2YQ==) | `30.87% <0%> (+0.67%)` | `25% <0%> (+1%)` | :arrow_up: |
   | [...rg/apache/gobblin/runtime/AbstractJobLauncher.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvQWJzdHJhY3RKb2JMYXVuY2hlci5qYXZh) | `59.78% <0%> (+0.79%)` | `36% <0%> (+2%)` | :arrow_up: |
   | ... and [43 more](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2864?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2864?src=pr&el=footer). Last update [d18783d...608737f](https://codecov.io/gh/apache/incubator-gobblin/pull/2864?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] codecov-io edited a comment on issue #2864: GOBBLIN-1018: Report GC counts and durations from Gobblin containers …

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #2864: GOBBLIN-1018: Report GC counts and durations from Gobblin containers …
URL: https://github.com/apache/incubator-gobblin/pull/2864#issuecomment-573483935
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2864?src=pr&el=h1) Report
   > Merging [#2864](https://codecov.io/gh/apache/incubator-gobblin/pull/2864?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/d18783df7ab29b161cca59ca907fb47e1366b358?src=pr&el=desc) will **increase** coverage by `1%`.
   > The diff coverage is `92.95%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/graphs/tree.svg?width=650&token=4MgURJ0bGc&height=150&src=pr)](https://codecov.io/gh/apache/incubator-gobblin/pull/2864?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff            @@
   ##             master   #2864    +/-   ##
   =========================================
   + Coverage      44.8%   45.8%    +1%     
   - Complexity     8933    9108   +175     
   =========================================
     Files          1917    1917            
     Lines         72131   72172    +41     
     Branches       7956    7961     +5     
   =========================================
   + Hits          32316   33057   +741     
   + Misses        36832   36088   -744     
   - Partials       2983    3027    +44
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2864?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...apache/gobblin/cluster/ContainerHealthMetrics.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvQ29udGFpbmVySGVhbHRoTWV0cmljcy5qYXZh) | `0% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | [...gobblin/cluster/ContainerHealthMetricsService.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvQ29udGFpbmVySGVhbHRoTWV0cmljc1NlcnZpY2UuamF2YQ==) | `93.81% <92.95%> (+15.24%)` | `8 <3> (+3)` | :arrow_up: |
   | [...src/main/java/org/apache/gobblin/runtime/Task.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvVGFzay5qYXZh) | `67.05% <0%> (+0.23%)` | `82% <0%> (+1%)` | :arrow_up: |
   | [.../org/apache/gobblin/runtime/SafeDatasetCommit.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvU2FmZURhdGFzZXRDb21taXQuamF2YQ==) | `47.31% <0%> (+0.48%)` | `29% <0%> (ø)` | :arrow_down: |
   | [...rg/apache/gobblin/runtime/AbstractJobLauncher.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvQWJzdHJhY3RKb2JMYXVuY2hlci5qYXZh) | `59.78% <0%> (+0.79%)` | `36% <0%> (+2%)` | :arrow_up: |
   | [...ain/java/org/apache/gobblin/runtime/fork/Fork.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvZm9yay9Gb3JrLmphdmE=) | `75% <0%> (+0.83%)` | `68% <0%> (ø)` | :arrow_down: |
   | [.../apache/gobblin/runtime/api/JobExecutionState.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvYXBpL0pvYkV4ZWN1dGlvblN0YXRlLmphdmE=) | `80.37% <0%> (+0.93%)` | `24% <0%> (ø)` | :arrow_down: |
   | [...in/java/org/apache/gobblin/runtime/JobContext.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvSm9iQ29udGV4dC5qYXZh) | `76.37% <0%> (+2.19%)` | `50% <0%> (+1%)` | :arrow_up: |
   | [...va/org/apache/gobblin/runtime/util/JobMetrics.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvdXRpbC9Kb2JNZXRyaWNzLmphdmE=) | `73.68% <0%> (+2.63%)` | `13% <0%> (+1%)` | :arrow_up: |
   | [.../gobblin/runtime/job/JobInterruptionPredicate.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvam9iL0pvYkludGVycnVwdGlvblByZWRpY2F0ZS5qYXZh) | `59.45% <0%> (+2.7%)` | `8% <0%> (+1%)` | :arrow_up: |
   | ... and [37 more](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2864?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2864?src=pr&el=footer). Last update [d18783d...926a86c](https://codecov.io/gh/apache/incubator-gobblin/pull/2864?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] autumnust commented on a change in pull request #2864: GOBBLIN-1018: Report GC counts and durations from Gobblin containers …

Posted by GitBox <gi...@apache.org>.
autumnust commented on a change in pull request #2864: GOBBLIN-1018: Report GC counts and durations from Gobblin containers …
URL: https://github.com/apache/incubator-gobblin/pull/2864#discussion_r366539877
 
 

 ##########
 File path: gobblin-cluster/src/main/java/org/apache/gobblin/cluster/ContainerHealthMetricsService.java
 ##########
 @@ -94,35 +133,75 @@ protected void runOneIteration() throws Exception {
     this.totalSwapSpaceSize.set(this.operatingSystemMXBean.getTotalSwapSpaceSize());
     this.freePhysicalMemSize.set(this.operatingSystemMXBean.getFreePhysicalMemorySize());
     this.processHeapUsedSize.set(this.memoryMXBean.getHeapMemoryUsage().getUsed());
+
+    GcStats gcStats = collectGcStats();
+    //Since GC Beans report accumulated counts/durations, we need to subtract the previous values to obtain the counts/durations
+    // since the last measurement time.
+    this.minorGcCount.set(gcStats.getMinorCount() - this.minorGcCount.get());
+    this.minorGcDuration.set(gcStats.getMinorDuration() - this.minorGcDuration.get());
+    this.majorGcCount.set(gcStats.getMajorCount() - this.majorGcCount.get());
+    this.majorGcDuration.set(gcStats.getMajorDuration() - this.majorGcDuration.get());
+    this.unknownGcCount.set(gcStats.getUnknownCount() - this.unknownGcCount.get());
+    this.unknownGcDuration.set(gcStats.getUnknownDuration() - this.unknownGcDuration.get());
   }
 
   protected List<ContextAwareGauge<Double>> buildGaugeList() {
     List<ContextAwareGauge<Double>> gaugeList = new ArrayList<>();
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.PROCESS_CPU_LOAD,
-        () -> this.processCpuLoad.get()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.SYSTEM_CPU_LOAD,
-        () -> this.systemCpuLoad.get()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.SYSTEM_LOAD_AVG,
-        () -> this.systemLoadAvg.get()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.COMMITTED_VMEM_SIZE,
-        () -> Long.valueOf(this.committedVmemSize.get()).doubleValue()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.PROCESS_CPU_TIME,
-        () -> Long.valueOf(this.processCpuTime.get()).doubleValue()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.FREE_SWAP_SPACE_SIZE,
-        () -> Long.valueOf(this.freeSwapSpaceSize.get()).doubleValue()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.NUM_AVAILABLE_PROCESSORS,
-        () -> Long.valueOf(this.numAvailableProcessors.get()).doubleValue()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.TOTAL_PHYSICAL_MEM_SIZE,
-        () -> Long.valueOf(this.totalPhysicalMemSize.get()).doubleValue()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.TOTAL_SWAP_SPACE_SIZE,
-        () -> Long.valueOf(this.totalSwapSpaceSize.get()).doubleValue()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.FREE_PHYSICAL_MEM_SIZE,
-        () -> Long.valueOf(this.freePhysicalMemSize.get()).doubleValue()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.PROCESS_HEAP_USED_SIZE,
-        () -> Long.valueOf(this.processHeapUsedSize.get()).doubleValue()));
+
+    gaugeList.add(getGauge(ContainerHealthMetrics.PROCESS_CPU_LOAD, this.processCpuLoad));
+    gaugeList.add(getGauge(ContainerHealthMetrics.SYSTEM_CPU_LOAD, this.systemCpuLoad));
+    gaugeList.add(getGauge(ContainerHealthMetrics.SYSTEM_LOAD_AVG, this.systemLoadAvg));
+    gaugeList.add(getGauge(ContainerHealthMetrics.COMMITTED_VMEM_SIZE, this.committedVmemSize));
+    gaugeList.add(getGauge(ContainerHealthMetrics.PROCESS_CPU_TIME, this.processCpuTime));
+    gaugeList.add(getGauge(ContainerHealthMetrics.FREE_SWAP_SPACE_SIZE, this.freeSwapSpaceSize));
+    gaugeList.add(getGauge(ContainerHealthMetrics.NUM_AVAILABLE_PROCESSORS, this.numAvailableProcessors));
+    gaugeList.add(getGauge(ContainerHealthMetrics.TOTAL_PHYSICAL_MEM_SIZE, this.totalPhysicalMemSize));
+    gaugeList.add(getGauge(ContainerHealthMetrics.TOTAL_SWAP_SPACE_SIZE, this.totalSwapSpaceSize));
+    gaugeList.add(getGauge(ContainerHealthMetrics.FREE_PHYSICAL_MEM_SIZE, this.freePhysicalMemSize));
+    gaugeList.add(getGauge(ContainerHealthMetrics.PROCESS_HEAP_USED_SIZE, this.processHeapUsedSize));
+    gaugeList.add(getGauge(ContainerHealthMetrics.MINOR_GC_COUNT, this.minorGcCount));
+    gaugeList.add(getGauge(ContainerHealthMetrics.MINOR_GC_DURATION, this.minorGcDuration));
+    gaugeList.add(getGauge(ContainerHealthMetrics.MAJOR_GC_COUNT, this.majorGcCount));
+    gaugeList.add(getGauge(ContainerHealthMetrics.MAJOR_GC_DURATION, this.majorGcDuration));
+    gaugeList.add(getGauge(ContainerHealthMetrics.UNKNOWN_GC_COUNT, this.unknownGcCount));
+    gaugeList.add(getGauge(ContainerHealthMetrics.UNKNOWN_GC_DURATION, this.unknownGcDuration));
     return gaugeList;
   }
 
+  private ContextAwareGauge<Double> getGauge(String name, Object metric) {
 
 Review comment:
   nit-pick: Is it more like a `createGauge`? 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] autumnust commented on a change in pull request #2864: GOBBLIN-1018: Report GC counts and durations from Gobblin containers …

Posted by GitBox <gi...@apache.org>.
autumnust commented on a change in pull request #2864: GOBBLIN-1018: Report GC counts and durations from Gobblin containers …
URL: https://github.com/apache/incubator-gobblin/pull/2864#discussion_r366541361
 
 

 ##########
 File path: gobblin-cluster/src/main/java/org/apache/gobblin/cluster/ContainerHealthMetricsService.java
 ##########
 @@ -94,35 +133,75 @@ protected void runOneIteration() throws Exception {
     this.totalSwapSpaceSize.set(this.operatingSystemMXBean.getTotalSwapSpaceSize());
     this.freePhysicalMemSize.set(this.operatingSystemMXBean.getFreePhysicalMemorySize());
     this.processHeapUsedSize.set(this.memoryMXBean.getHeapMemoryUsage().getUsed());
+
+    GcStats gcStats = collectGcStats();
+    //Since GC Beans report accumulated counts/durations, we need to subtract the previous values to obtain the counts/durations
+    // since the last measurement time.
+    this.minorGcCount.set(gcStats.getMinorCount() - this.minorGcCount.get());
+    this.minorGcDuration.set(gcStats.getMinorDuration() - this.minorGcDuration.get());
+    this.majorGcCount.set(gcStats.getMajorCount() - this.majorGcCount.get());
+    this.majorGcDuration.set(gcStats.getMajorDuration() - this.majorGcDuration.get());
+    this.unknownGcCount.set(gcStats.getUnknownCount() - this.unknownGcCount.get());
+    this.unknownGcDuration.set(gcStats.getUnknownDuration() - this.unknownGcDuration.get());
   }
 
   protected List<ContextAwareGauge<Double>> buildGaugeList() {
     List<ContextAwareGauge<Double>> gaugeList = new ArrayList<>();
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.PROCESS_CPU_LOAD,
-        () -> this.processCpuLoad.get()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.SYSTEM_CPU_LOAD,
-        () -> this.systemCpuLoad.get()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.SYSTEM_LOAD_AVG,
-        () -> this.systemLoadAvg.get()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.COMMITTED_VMEM_SIZE,
-        () -> Long.valueOf(this.committedVmemSize.get()).doubleValue()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.PROCESS_CPU_TIME,
-        () -> Long.valueOf(this.processCpuTime.get()).doubleValue()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.FREE_SWAP_SPACE_SIZE,
-        () -> Long.valueOf(this.freeSwapSpaceSize.get()).doubleValue()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.NUM_AVAILABLE_PROCESSORS,
-        () -> Long.valueOf(this.numAvailableProcessors.get()).doubleValue()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.TOTAL_PHYSICAL_MEM_SIZE,
-        () -> Long.valueOf(this.totalPhysicalMemSize.get()).doubleValue()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.TOTAL_SWAP_SPACE_SIZE,
-        () -> Long.valueOf(this.totalSwapSpaceSize.get()).doubleValue()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.FREE_PHYSICAL_MEM_SIZE,
-        () -> Long.valueOf(this.freePhysicalMemSize.get()).doubleValue()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.PROCESS_HEAP_USED_SIZE,
-        () -> Long.valueOf(this.processHeapUsedSize.get()).doubleValue()));
+
+    gaugeList.add(getGauge(ContainerHealthMetrics.PROCESS_CPU_LOAD, this.processCpuLoad));
+    gaugeList.add(getGauge(ContainerHealthMetrics.SYSTEM_CPU_LOAD, this.systemCpuLoad));
+    gaugeList.add(getGauge(ContainerHealthMetrics.SYSTEM_LOAD_AVG, this.systemLoadAvg));
+    gaugeList.add(getGauge(ContainerHealthMetrics.COMMITTED_VMEM_SIZE, this.committedVmemSize));
+    gaugeList.add(getGauge(ContainerHealthMetrics.PROCESS_CPU_TIME, this.processCpuTime));
+    gaugeList.add(getGauge(ContainerHealthMetrics.FREE_SWAP_SPACE_SIZE, this.freeSwapSpaceSize));
+    gaugeList.add(getGauge(ContainerHealthMetrics.NUM_AVAILABLE_PROCESSORS, this.numAvailableProcessors));
+    gaugeList.add(getGauge(ContainerHealthMetrics.TOTAL_PHYSICAL_MEM_SIZE, this.totalPhysicalMemSize));
+    gaugeList.add(getGauge(ContainerHealthMetrics.TOTAL_SWAP_SPACE_SIZE, this.totalSwapSpaceSize));
+    gaugeList.add(getGauge(ContainerHealthMetrics.FREE_PHYSICAL_MEM_SIZE, this.freePhysicalMemSize));
+    gaugeList.add(getGauge(ContainerHealthMetrics.PROCESS_HEAP_USED_SIZE, this.processHeapUsedSize));
+    gaugeList.add(getGauge(ContainerHealthMetrics.MINOR_GC_COUNT, this.minorGcCount));
+    gaugeList.add(getGauge(ContainerHealthMetrics.MINOR_GC_DURATION, this.minorGcDuration));
+    gaugeList.add(getGauge(ContainerHealthMetrics.MAJOR_GC_COUNT, this.majorGcCount));
+    gaugeList.add(getGauge(ContainerHealthMetrics.MAJOR_GC_DURATION, this.majorGcDuration));
+    gaugeList.add(getGauge(ContainerHealthMetrics.UNKNOWN_GC_COUNT, this.unknownGcCount));
+    gaugeList.add(getGauge(ContainerHealthMetrics.UNKNOWN_GC_DURATION, this.unknownGcDuration));
     return gaugeList;
   }
 
+  private ContextAwareGauge<Double> getGauge(String name, Object metric) {
+    if (metric instanceof AtomicLong) {
+      return RootMetricContext.get().newContextAwareGauge(name, () -> Long.valueOf(((AtomicLong) metric).get()).doubleValue());
 
 Review comment:
   I have no strong opinion here but using `doubleValue()` for a `Long` value can save us from using `instanceof` which I believe should be avoided as much as possible in OOP. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] htran1 commented on issue #2864: GOBBLIN-1018: Report GC counts and durations from Gobblin containers …

Posted by GitBox <gi...@apache.org>.
htran1 commented on issue #2864: GOBBLIN-1018: Report GC counts and durations from Gobblin containers …
URL: https://github.com/apache/incubator-gobblin/pull/2864#issuecomment-574405420
 
 
   +1

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] autumnust commented on a change in pull request #2864: GOBBLIN-1018: Report GC counts and durations from Gobblin containers …

Posted by GitBox <gi...@apache.org>.
autumnust commented on a change in pull request #2864: GOBBLIN-1018: Report GC counts and durations from Gobblin containers …
URL: https://github.com/apache/incubator-gobblin/pull/2864#discussion_r366539273
 
 

 ##########
 File path: gobblin-cluster/src/main/java/org/apache/gobblin/cluster/ContainerHealthMetricsService.java
 ##########
 @@ -94,35 +133,75 @@ protected void runOneIteration() throws Exception {
     this.totalSwapSpaceSize.set(this.operatingSystemMXBean.getTotalSwapSpaceSize());
     this.freePhysicalMemSize.set(this.operatingSystemMXBean.getFreePhysicalMemorySize());
     this.processHeapUsedSize.set(this.memoryMXBean.getHeapMemoryUsage().getUsed());
+
+    GcStats gcStats = collectGcStats();
+    //Since GC Beans report accumulated counts/durations, we need to subtract the previous values to obtain the counts/durations
+    // since the last measurement time.
+    this.minorGcCount.set(gcStats.getMinorCount() - this.minorGcCount.get());
+    this.minorGcDuration.set(gcStats.getMinorDuration() - this.minorGcDuration.get());
+    this.majorGcCount.set(gcStats.getMajorCount() - this.majorGcCount.get());
+    this.majorGcDuration.set(gcStats.getMajorDuration() - this.majorGcDuration.get());
+    this.unknownGcCount.set(gcStats.getUnknownCount() - this.unknownGcCount.get());
+    this.unknownGcDuration.set(gcStats.getUnknownDuration() - this.unknownGcDuration.get());
   }
 
   protected List<ContextAwareGauge<Double>> buildGaugeList() {
     List<ContextAwareGauge<Double>> gaugeList = new ArrayList<>();
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.PROCESS_CPU_LOAD,
-        () -> this.processCpuLoad.get()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.SYSTEM_CPU_LOAD,
-        () -> this.systemCpuLoad.get()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.SYSTEM_LOAD_AVG,
-        () -> this.systemLoadAvg.get()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.COMMITTED_VMEM_SIZE,
-        () -> Long.valueOf(this.committedVmemSize.get()).doubleValue()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.PROCESS_CPU_TIME,
-        () -> Long.valueOf(this.processCpuTime.get()).doubleValue()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.FREE_SWAP_SPACE_SIZE,
-        () -> Long.valueOf(this.freeSwapSpaceSize.get()).doubleValue()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.NUM_AVAILABLE_PROCESSORS,
-        () -> Long.valueOf(this.numAvailableProcessors.get()).doubleValue()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.TOTAL_PHYSICAL_MEM_SIZE,
-        () -> Long.valueOf(this.totalPhysicalMemSize.get()).doubleValue()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.TOTAL_SWAP_SPACE_SIZE,
-        () -> Long.valueOf(this.totalSwapSpaceSize.get()).doubleValue()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.FREE_PHYSICAL_MEM_SIZE,
-        () -> Long.valueOf(this.freePhysicalMemSize.get()).doubleValue()));
-    gaugeList.add(RootMetricContext.get().newContextAwareGauge(ContainerHealthMetrics.PROCESS_HEAP_USED_SIZE,
-        () -> Long.valueOf(this.processHeapUsedSize.get()).doubleValue()));
+
+    gaugeList.add(getGauge(ContainerHealthMetrics.PROCESS_CPU_LOAD, this.processCpuLoad));
+    gaugeList.add(getGauge(ContainerHealthMetrics.SYSTEM_CPU_LOAD, this.systemCpuLoad));
+    gaugeList.add(getGauge(ContainerHealthMetrics.SYSTEM_LOAD_AVG, this.systemLoadAvg));
+    gaugeList.add(getGauge(ContainerHealthMetrics.COMMITTED_VMEM_SIZE, this.committedVmemSize));
+    gaugeList.add(getGauge(ContainerHealthMetrics.PROCESS_CPU_TIME, this.processCpuTime));
+    gaugeList.add(getGauge(ContainerHealthMetrics.FREE_SWAP_SPACE_SIZE, this.freeSwapSpaceSize));
+    gaugeList.add(getGauge(ContainerHealthMetrics.NUM_AVAILABLE_PROCESSORS, this.numAvailableProcessors));
+    gaugeList.add(getGauge(ContainerHealthMetrics.TOTAL_PHYSICAL_MEM_SIZE, this.totalPhysicalMemSize));
+    gaugeList.add(getGauge(ContainerHealthMetrics.TOTAL_SWAP_SPACE_SIZE, this.totalSwapSpaceSize));
+    gaugeList.add(getGauge(ContainerHealthMetrics.FREE_PHYSICAL_MEM_SIZE, this.freePhysicalMemSize));
+    gaugeList.add(getGauge(ContainerHealthMetrics.PROCESS_HEAP_USED_SIZE, this.processHeapUsedSize));
+    gaugeList.add(getGauge(ContainerHealthMetrics.MINOR_GC_COUNT, this.minorGcCount));
+    gaugeList.add(getGauge(ContainerHealthMetrics.MINOR_GC_DURATION, this.minorGcDuration));
+    gaugeList.add(getGauge(ContainerHealthMetrics.MAJOR_GC_COUNT, this.majorGcCount));
+    gaugeList.add(getGauge(ContainerHealthMetrics.MAJOR_GC_DURATION, this.majorGcDuration));
+    gaugeList.add(getGauge(ContainerHealthMetrics.UNKNOWN_GC_COUNT, this.unknownGcCount));
+    gaugeList.add(getGauge(ContainerHealthMetrics.UNKNOWN_GC_DURATION, this.unknownGcDuration));
     return gaugeList;
   }
 
+  private ContextAwareGauge<Double> getGauge(String name, Object metric) {
+    if (metric instanceof AtomicLong) {
+      return RootMetricContext.get().newContextAwareGauge(name, () -> Long.valueOf(((AtomicLong) metric).get()).doubleValue());
+    } else if (metric instanceof AtomicDouble) {
+      return RootMetricContext.get().newContextAwareGauge(name, () -> ((AtomicDouble) metric).get());
+    } else {
+      throw new RuntimeException(String.format("Unexpected metric type: %s for metric %s", metric.getClass().getName(), name));
 
 Review comment:
   Does it need to be a vital exception ? 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] codecov-io edited a comment on issue #2864: GOBBLIN-1018: Report GC counts and durations from Gobblin containers …

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #2864: GOBBLIN-1018: Report GC counts and durations from Gobblin containers …
URL: https://github.com/apache/incubator-gobblin/pull/2864#issuecomment-573483935
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2864?src=pr&el=h1) Report
   > Merging [#2864](https://codecov.io/gh/apache/incubator-gobblin/pull/2864?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/64b074182a7bea017a249204c5448ee79a0a28ac?src=pr&el=desc) will **increase** coverage by `41.68%`.
   > The diff coverage is `89.55%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/graphs/tree.svg?width=650&token=4MgURJ0bGc&height=150&src=pr)](https://codecov.io/gh/apache/incubator-gobblin/pull/2864?src=pr&el=tree)
   
   ```diff
   @@              Coverage Diff              @@
   ##             master    #2864       +/-   ##
   =============================================
   + Coverage       4.1%   45.79%   +41.68%     
   - Complexity      747     9110     +8363     
   =============================================
     Files          1917     1917               
     Lines         72131    72176       +45     
     Branches       7956     7962        +6     
   =============================================
   + Hits           2961    33052    +30091     
   + Misses        68851    36099    -32752     
   - Partials        319     3025     +2706
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2864?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...apache/gobblin/cluster/ContainerHealthMetrics.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvQ29udGFpbmVySGVhbHRoTWV0cmljcy5qYXZh) | `0% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | [...gobblin/cluster/ContainerHealthMetricsService.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvQ29udGFpbmVySGVhbHRoTWV0cmljc1NlcnZpY2UuamF2YQ==) | `92.07% <89.55%> (+92.07%)` | `10 <5> (+10)` | :arrow_up: |
   | [...gobblin/service/monitoring/JobStatusRetriever.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Kb2JTdGF0dXNSZXRyaWV2ZXIuamF2YQ==) | `0% <0%> (ø)` | `2% <0%> (+2%)` | :arrow_up: |
   | [...ata/management/copy/hive/HivePartitionFileSet.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlUGFydGl0aW9uRmlsZVNldC5qYXZh) | `0.95% <0%> (+0.95%)` | `1% <0%> (+1%)` | :arrow_up: |
   | [...ain/java/org/apache/gobblin/runtime/TaskState.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvVGFza1N0YXRlLmphdmE=) | `81.97% <0%> (+1.16%)` | `32% <0%> (ø)` | :arrow_down: |
   | [...pache/gobblin/runtime/GobblinMultiTaskAttempt.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvR29iYmxpbk11bHRpVGFza0F0dGVtcHQuamF2YQ==) | `56.3% <0%> (+1.35%)` | `27% <0%> (+2%)` | :arrow_up: |
   | [...ava/org/apache/gobblin/runtime/MultiConverter.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvTXVsdGlDb252ZXJ0ZXIuamF2YQ==) | `83.6% <0%> (+1.63%)` | `9% <0%> (+1%)` | :arrow_up: |
   | [...rg/apache/gobblin/runtime/FsDatasetStateStore.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvRnNEYXRhc2V0U3RhdGVTdG9yZS5qYXZh) | `73.8% <0%> (+1.78%)` | `35% <0%> (+1%)` | :arrow_up: |
   | [...a/org/apache/gobblin/cluster/SingleTaskRunner.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvU2luZ2xlVGFza1J1bm5lci5qYXZh) | `1.85% <0%> (+1.85%)` | `1% <0%> (+1%)` | :arrow_up: |
   | [.../java/org/apache/gobblin/runtime/TaskExecutor.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvVGFza0V4ZWN1dG9yLmphdmE=) | `45.35% <0%> (+2.73%)` | `9% <0%> (+1%)` | :arrow_up: |
   | ... and [1112 more](https://codecov.io/gh/apache/incubator-gobblin/pull/2864/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2864?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2864?src=pr&el=footer). Last update [64b0741...608737f](https://codecov.io/gh/apache/incubator-gobblin/pull/2864?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] autumnust commented on a change in pull request #2864: GOBBLIN-1018: Report GC counts and durations from Gobblin containers …

Posted by GitBox <gi...@apache.org>.
autumnust commented on a change in pull request #2864: GOBBLIN-1018: Report GC counts and durations from Gobblin containers …
URL: https://github.com/apache/incubator-gobblin/pull/2864#discussion_r366523530
 
 

 ##########
 File path: gobblin-cluster/src/main/java/org/apache/gobblin/cluster/ContainerHealthMetrics.java
 ##########
 @@ -30,5 +30,10 @@
   public static final String NUM_AVAILABLE_PROCESSORS = CONTAINER_METRICS_PREFIX + "numAvailableProcessors";
   public static final String TOTAL_PHYSICAL_MEM_SIZE = CONTAINER_METRICS_PREFIX + "totalPhysicalMemSize";
   public static final String FREE_PHYSICAL_MEM_SIZE = CONTAINER_METRICS_PREFIX + "freePhysicalMemSize";
-
+  public static final String MINOR_GC_COUNT = CONTAINER_METRICS_PREFIX + "minorGcCount";
 
 Review comment:
   Do you need full_gc_count ( if available )

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] autumnust commented on a change in pull request #2864: GOBBLIN-1018: Report GC counts and durations from Gobblin containers …

Posted by GitBox <gi...@apache.org>.
autumnust commented on a change in pull request #2864: GOBBLIN-1018: Report GC counts and durations from Gobblin containers …
URL: https://github.com/apache/incubator-gobblin/pull/2864#discussion_r366541788
 
 

 ##########
 File path: gobblin-cluster/src/main/java/org/apache/gobblin/cluster/ContainerHealthMetricsService.java
 ##########
 @@ -94,35 +133,75 @@ protected void runOneIteration() throws Exception {
     this.totalSwapSpaceSize.set(this.operatingSystemMXBean.getTotalSwapSpaceSize());
     this.freePhysicalMemSize.set(this.operatingSystemMXBean.getFreePhysicalMemorySize());
     this.processHeapUsedSize.set(this.memoryMXBean.getHeapMemoryUsage().getUsed());
+
+    GcStats gcStats = collectGcStats();
+    //Since GC Beans report accumulated counts/durations, we need to subtract the previous values to obtain the counts/durations
+    // since the last measurement time.
+    this.minorGcCount.set(gcStats.getMinorCount() - this.minorGcCount.get());
 
 Review comment:
   Just curious, when printed, do we have any mention of this duration ? 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] sv2000 commented on issue #2864: GOBBLIN-1018: Report GC counts and durations from Gobblin containers …

Posted by GitBox <gi...@apache.org>.
sv2000 commented on issue #2864: GOBBLIN-1018: Report GC counts and durations from Gobblin containers …
URL: https://github.com/apache/incubator-gobblin/pull/2864#issuecomment-573480716
 
 
   @autumnust please review.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services