Re: RFR: 8203359: Container level resources events [v11]
On Wed, 19 May 2021 10:00:04 GMT, Severin Gehwolf wrote:
>> Jaroslav Bachorik has updated the pull request with a new target base due to
>> a merge or a rebase. The pull request now contains 10 commits:
>>
>> - Small fixes
>> - Remove trailing spaces
>> - Doh
>> - Report container type and register events conditionally
>> - Remove unused test files
>> - Initial test support for JFR container events
>> - Update the JFR control files
>> - Split off the CPU throttling metrics
>> - Formatting spaces
>> - 8203359: Container level resources events
>
> src/jdk.jfr/share/classes/jdk/jfr/events/ContainerMemoryUsageEvent.java line
> 48:
>
>> 46: @Description("(attempts per second * 1000), if enabled, that the
>> operating system tries to satisfy a memory request for any " +
>> 47: "process in the current container when no free memory
>> is readily available.")
>> 48: public double memoryPressure;
>
> Should this `memoryPressure` field go from `ContainerMemoryUsageEvent` class?
> It's not set anywhere is it? would be cgroup v1 only api so I'm not sure it
> should be there for a generic event like this.
Yes. Removing.
-
PR: https://git.openjdk.java.net/jdk/pull/3126
Re: RFR: 8203359: Container level resources events [v11]
On Mon, 3 May 2021 13:16:06 GMT, Jaroslav Bachorik
wrote:
>> With this change it becomes possible to surface various cgroup level metrics
>> (available via `jdk.internal.platform.Metrics`) as JFR events.
>>
>> Only a subset of the metrics exposed by `jdk.internal.platform.Metrics` is
>> turned into JFR events to start with.
>> * CPU related metrics
>> * Memory related metrics
>> * I/O related metrics
>>
>> For each of those subsystems a configuration data will be emitted as well.
>> The initial proposal is to emit the configuration data events at least once
>> per chunk and the metrics values at 30 seconds interval.
>> By using these values the emitted events seem to contain useful information
>> without increasing overhead (the metrics values are read from `/proc`
>> filesystem so that should not be done too frequently).
>
> Jaroslav Bachorik has updated the pull request with a new target base due to
> a merge or a rebase. The pull request now contains 10 commits:
>
> - Small fixes
> - Remove trailing spaces
> - Doh
> - Report container type and register events conditionally
> - Remove unused test files
> - Initial test support for JFR container events
> - Update the JFR control files
> - Split off the CPU throttling metrics
> - Formatting spaces
> - 8203359: Container level resources events
Changes requested by sgehwolf (Reviewer).
src/jdk.jfr/share/classes/jdk/jfr/events/ContainerMemoryUsageEvent.java line 48:
> 46: @Description("(attempts per second * 1000), if enabled, that the
> operating system tries to satisfy a memory request for any " +
> 47: "process in the current container when no free memory is
> readily available.")
> 48: public double memoryPressure;
Should this `memoryPressure` field go from `ContainerMemoryUsageEvent` class?
It's not set anywhere is it? would be cgroup v1 only api so I'm not sure it
should be there for a generic event like this.
-
PR: https://git.openjdk.java.net/jdk/pull/3126
Re: RFR: 8203359: Container level resources events [v11]
On Mon, 3 May 2021 13:16:06 GMT, Jaroslav Bachorik wrote: >> With this change it becomes possible to surface various cgroup level metrics >> (available via `jdk.internal.platform.Metrics`) as JFR events. >> >> Only a subset of the metrics exposed by `jdk.internal.platform.Metrics` is >> turned into JFR events to start with. >> * CPU related metrics >> * Memory related metrics >> * I/O related metrics >> >> For each of those subsystems a configuration data will be emitted as well. >> The initial proposal is to emit the configuration data events at least once >> per chunk and the metrics values at 30 seconds interval. >> By using these values the emitted events seem to contain useful information >> without increasing overhead (the metrics values are read from `/proc` >> filesystem so that should not be done too frequently). > > Jaroslav Bachorik has updated the pull request with a new target base due to > a merge or a rebase. The pull request now contains 10 commits: > > - Small fixes > - Remove trailing spaces > - Doh > - Report container type and register events conditionally > - Remove unused test files > - Initial test support for JFR container events > - Update the JFR control files > - Split off the CPU throttling metrics > - Formatting spaces > - 8203359: Container level resources events Looks good, but if there are test issues they should be fixed. - Marked as reviewed by egahlin (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3126
Re: RFR: 8203359: Container level resources events [v11]
> With this change it becomes possible to surface various cgroup level metrics > (available via `jdk.internal.platform.Metrics`) as JFR events. > > Only a subset of the metrics exposed by `jdk.internal.platform.Metrics` is > turned into JFR events to start with. > * CPU related metrics > * Memory related metrics > * I/O related metrics > > For each of those subsystems a configuration data will be emitted as well. > The initial proposal is to emit the configuration data events at least once > per chunk and the metrics values at 30 seconds interval. > By using these values the emitted events seem to contain useful information > without increasing overhead (the metrics values are read from `/proc` > filesystem so that should not be done too frequently). Jaroslav Bachorik has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 10 additional commits since the last revision: - Small fixes - Remove trailing spaces - Doh - Report container type and register events conditionally - Remove unused test files - Initial test support for JFR container events - Update the JFR control files - Split off the CPU throttling metrics - Formatting spaces - 8203359: Container level resources events - Changes: - all: https://git.openjdk.java.net/jdk/pull/3126/files - new: https://git.openjdk.java.net/jdk/pull/3126/files/04c3f092..fddd1727 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3126&range=10 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3126&range=09-10 Stats: 93348 lines in 2177 files changed: 37265 ins; 49988 del; 6095 mod Patch: https://git.openjdk.java.net/jdk/pull/3126.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3126/head:pull/3126 PR: https://git.openjdk.java.net/jdk/pull/3126
