Skip to content
This repository was archived by the owner on Sep 28, 2021. It is now read-only.

Conversation

@gimaker
Copy link
Collaborator

@gimaker gimaker commented Nov 8, 2017

FileDescriptorGaugeSet is broken at least in OpenJDK 9, throwing an exception
like this:

java.lang.reflect.InaccessibleObjectException: Unable to make public long com.sun.management.internal.OperatingSystemImpl.getOpenFileDescriptorCount() accessible: module jdk.management does not "opens com.sun.management.internal" to unnamed module @1a451d4d
	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:337)
	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:281)
	at java.base/java.lang.reflect.Method.checkCanSetAccessible(Method.java:198)
	at java.base/java.lang.reflect.Method.setAccessible(Method.java:192)
	at com.codahale.metrics.jvm.FileDescriptorRatioGauge.invoke(FileDescriptorRatioGauge.java:48)
	at com.codahale.metrics.jvm.FileDescriptorRatioGauge.getRatio(FileDescriptorRatioGauge.java:35)
	at com.codahale.metrics.RatioGauge.getValue(RatioGauge.java:64)
	at com.spotify.metrics.jvm.FileDescriptorGaugeSet$1.getValue(FileDescriptorGaugeSet.java:54)
	at com.spotify.metrics.ffwd.FastForwardReporter.reportGauge(FastForwardReporter.java:231)
	at com.spotify.metrics.ffwd.FastForwardReporter.report(FastForwardReporter.java:193)
	at com.spotify.metrics.ffwd.FastForwardReporter.report(FastForwardReporter.java:181)
	at com.spotify.metrics.ffwd.FastForwardReporter$2.run(FastForwardReporter.java:348)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:514)
	at java.base/java.util.concurrent.FutureTask.runAndReset(FutureTask.java:305)
	at java.base/java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:300)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
	at java.base/java.lang.Thread.run(Thread.java:844)

Any metrics that would have been reported AFTER this metric will not be reported
at all, which is pretty bad - no rates, etc would be reported when this throws.

Until upstream (ideally codahale metrics, possibly semantic-metrics) is patched
to handle this properly we reimplement the FileDescriptorSetGauge from
semantic-metrics in a way that handles this safely.

@gimaker
Copy link
Collaborator Author

gimaker commented Nov 8, 2017

@jhaals @tommyulfsparre

FileDescriptorGaugeSet is broken at least in OpenJDK 9, throwing an exception
like this:

```
java.lang.reflect.InaccessibleObjectException: Unable to make public long com.sun.management.internal.OperatingSystemImpl.getOpenFileDescriptorCount() accessible: module jdk.management does not "opens com.sun.management.internal" to unnamed module @1a451d4d
	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:337)
	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:281)
	at java.base/java.lang.reflect.Method.checkCanSetAccessible(Method.java:198)
	at java.base/java.lang.reflect.Method.setAccessible(Method.java:192)
	at com.codahale.metrics.jvm.FileDescriptorRatioGauge.invoke(FileDescriptorRatioGauge.java:48)
	at com.codahale.metrics.jvm.FileDescriptorRatioGauge.getRatio(FileDescriptorRatioGauge.java:35)
	at com.codahale.metrics.RatioGauge.getValue(RatioGauge.java:64)
	at com.spotify.metrics.jvm.FileDescriptorGaugeSet$1.getValue(FileDescriptorGaugeSet.java:54)
	at com.spotify.metrics.ffwd.FastForwardReporter.reportGauge(FastForwardReporter.java:231)
	at com.spotify.metrics.ffwd.FastForwardReporter.report(FastForwardReporter.java:193)
	at com.spotify.metrics.ffwd.FastForwardReporter.report(FastForwardReporter.java:181)
	at com.spotify.metrics.ffwd.FastForwardReporter$2.run(FastForwardReporter.java:348)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:514)
	at java.base/java.util.concurrent.FutureTask.runAndReset(FutureTask.java:305)
	at java.base/java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:300)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
	at java.base/java.lang.Thread.run(Thread.java:844)
```

Any metrics that would have been reported AFTER this metric will not be reported
at all, which is pretty bad - no rates, etc would be reported when this throws.

Until upstream (ideally codahale metrics, possibly semantic-metrics) is patched
to handle this properly we reimplement the FileDescriptorSetGauge from
semantic-metrics in a way that handles this safely.
@gimaker gimaker force-pushed the staffan/file-desc-metrics-java9 branch from c37f1f5 to ed660f8 Compare November 8, 2017 14:42
@codecov-io
Copy link

Codecov Report

Merging #231 into 1.x will decrease coverage by 0.03%.
The diff coverage is 63.63%.

Impacted file tree graph

@@             Coverage Diff              @@
##                1.x     #231      +/-   ##
============================================
- Coverage     69.66%   69.63%   -0.04%     
  Complexity      653      653              
============================================
  Files           139      139              
  Lines          2924     2934      +10     
  Branches        172      172              
============================================
+ Hits           2037     2043       +6     
- Misses          844      848       +4     
  Partials         43       43
Impacted Files Coverage Δ Complexity Δ
...java/com/spotify/apollo/metrics/MetricsModule.java 55.81% <63.63%> (+1.26%) 5 <0> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 88a0a2e...ed660f8. Read the comment docs.

@jhaals
Copy link
Member

jhaals commented Nov 8, 2017

👍

@gimaker gimaker merged commit f49a9ed into 1.x Nov 8, 2017
@gimaker gimaker deleted the staffan/file-desc-metrics-java9 branch November 8, 2017 14:50
@mattnworb
Copy link
Member

Can you report this in https://github.com/dropwizard/metrics/issues too? I searched and didn't see anything.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants