jdk icon indicating copy to clipboard operation
jdk copied to clipboard

8289711: Add container configuration data to mbeans

Open xpbob opened this issue 3 years ago • 23 comments
trafficstars

Container configuration information is useful for troubleshooting problems,Exposing information in MBeans is ideal for monitoring, jConsole, and other scenarios. Results the following 图片


Progress

  • [ ] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
  • [x] Change must not contain extraneous whitespace
  • [x] Commit message must refer to an issue
  • [ ] Change requires a CSR request to be approved

Issue

  • JDK-8289711: Add container configuration data to mbeans

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/9372/head:pull/9372
$ git checkout pull/9372

Update a local copy of the PR:
$ git checkout pull/9372
$ git pull https://git.openjdk.org/jdk pull/9372/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 9372

View PR using the GUI difftool:
$ git pr show -t 9372

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/9372.diff

xpbob avatar Jul 05 '22 04:07 xpbob

:wave: Welcome back xpbob! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

bridgekeeper[bot] avatar Jul 05 '22 04:07 bridgekeeper[bot]

@xpbob The following labels will be automatically applied to this pull request:

  • core-libs
  • serviceability

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

openjdk[bot] avatar Jul 05 '22 04:07 openjdk[bot]

@DamonFool Thanks for review. The code has been updated

xpbob avatar Jul 06 '22 03:07 xpbob

/csr

AlanBateman avatar Jul 06 '22 05:07 AlanBateman

@AlanBateman has indicated that a compatibility and specification (CSR) request is needed for this pull request.

@xpbob please create a CSR request for issue JDK-8289711 with the correct fix version. This pull request cannot be integrated until the CSR request is approved.

openjdk[bot] avatar Jul 06 '22 05:07 openjdk[bot]

It's not clear that introducing this as a standard API is the right thing to do. Are you 100% confident that the concepts of "CPU quota", "CPU shares", "CPU period", "soft limit" etc. will last the test of time and that we don't be back here next year with another PR to deprecate or replace this API? I don't disagree that exposing a MXBean could be useful for monitoring/management purposes but I think we have to be cautious getting getting locked in. A possible starting point for prototyping purposes and getting feedback is a JDK-specific MXBean (module jdk.management).

AlanBateman avatar Jul 06 '22 05:07 AlanBateman

I do think this could be useful.

Having said that, some of this has been discussed in https://bugs.openjdk.org/browse/JDK-8199944 (and been considered as won't fix at the time). I tend to agree that it should be a JDK (and even an OS) specific bean. The relevant information is only available on Linux. +1 on making this JDK specific. First step is probably to get consensus on a CSR.

jerboaa avatar Jul 06 '22 10:07 jerboaa

@AlanBateman @jerboaa That's a good idea.Adding a special bean is only available on Linux systems.I do not know the process of creating a CSR, can you help me create a CSR

xpbob avatar Jul 10 '22 14:07 xpbob

@AlanBateman @jerboaa That's a good idea.Adding a special bean is only available on Linux systems.I do not know the process of creating a CSR, can you help me create a CSR

See https://wiki.openjdk.org/display/csr/CSR+FAQs

jerboaa avatar Jul 11 '22 08:07 jerboaa

@AlanBateman @jerboaa That's a good idea.Adding a special bean is only available on Linux systems.I do not know the process of creating a CSR, can you help me create a CSR

It's too early to think about a CSR, probably a bit premature to create a PR too because it will take a few iterations to come to agreement on what API to expose, if any.

I don't think this should be a standard API, meaning java.management/java.lang.management.ContainerMXBean is not the right place for this. A JDK-specific MXBean is an option but it would only be usable on Linux and maybe only usable when running in a container environment. Working down, it's not clear to me how stable the attributes are and the mapping to both cgroup v1 and v2. There is also overlap with OperatingSystemMXBean which already defines the "TotalSwapSpaceSize" attribute. There's another level of detail beyond that with unusual value -2 to distinguish "not available" from "not supported". So I think there are a few things to think about there.

Another direction to think about is not exposing an API that you can compile against but instead just register a MBean that provides access to the attributes that are interesting for the container environment. By this I mean ManagementFactory.getPlatformMBeanServer().registerMBean(...). This would be enough to browse the MBean and its attributes in any JMX console and may give you a bit more flexibility to expose attributes that are specific to the cgroup version. I'm not saying this is the right direction, I'm just listing it here as something that could be explored. If the main use case is JMX consoles rather than programmatic access then it may not be too bad.

AlanBateman avatar Jul 11 '22 09:07 AlanBateman

@AlanBateman OperatingSystemMXBean makes the CPU and Memory related information for a process available regardless of whether that process is running in a native operating system environment or a containerized environment. It does not provide container configuration information such as its provider (crgoups v1 or v2), CPU shares, CPU quota etc, which can be useful for monitoring and troubleshooting purposes.

poonamparhar avatar Jul 12 '22 15:07 poonamparhar

@AlanBateman OperatingSystemMXBean makes the CPU and Memory related information for a process available regardless of whether that process is running in a native operating system environment or a containerized environment. It does not provide container configuration information such as its provider (crgoups v1 or v2), CPU shares, CPU quota etc, which can be useful for monitoring and troubleshooting purposes.

Yes, I know this but I think there is a lot more thought required before deciding to augment this with another API for the container environment. There are a couple of things to explore.

AlanBateman avatar Jul 12 '22 15:07 AlanBateman

@AlanBateman @jerboaa @poonamparhar @DamonFool Thanks for review. I add mBeans using the registerMBean method. We can get configuration information through JConsole, JMX exporter

xpbob avatar Jul 14 '22 03:07 xpbob

Thanks for review. I add mBeans using the registerMBean method. We can get configuration information through JConsole, JMX exporter

This iteration is a bit confusing because it adds a public interface to java.lang.management. For the registerMBean prototype then you shouldn't need any API changes to the java.management module.

AlanBateman avatar Jul 14 '22 07:07 AlanBateman

Thanks for review. I add mBeans using the registerMBean method. We can get configuration information through JConsole, JMX exporter

This iteration is a bit confusing because it adds a public interface to java.lang.management. For the registerMBean prototype then you shouldn't need any API changes to the java.management module. @AlanBateman Thanks for review. The runtime information is already fetched through the OperatingSystemMXBean, and I'm wrapped through the interface, keeping only the configuration data.Such an interface could be part of management.

xpbob avatar Jul 14 '22 07:07 xpbob

Thanks for review. The runtime information is already fetched through the OperatingSystemMXBean, and I'm wrapped through the interface, keeping only the configuration data.Such an interface could be part of management.

I don't think this feature should be adding to the standard API. Can you move ContainerInfoMXBean to jdk.management and do some experiments?

AlanBateman avatar Jul 14 '22 08:07 AlanBateman

Thanks for review. @AlanBateman I move ContainerInfoMXBean to sun.management, the data is not available, 图片 The red character indicates that it is not available

xpbob avatar Jul 14 '22 12:07 xpbob

Mailing list message from Laurence Cable on serviceability-dev:

agree Alan

On 7/14/22 1:27 AM, Alan Bateman wrote:

mlbridge[bot] avatar Jul 14 '22 14:07 mlbridge[bot]

What happened with the experiment to move ContainerInfoMXBean to jdk.management?

AlanBateman avatar Aug 03 '22 09:08 AlanBateman

Thanks for review. @AlanBateman I found my code bug and it has been fixed. Moving ContainerInfo is feasible

xpbob avatar Aug 03 '22 10:08 xpbob

图片

xpbob avatar Aug 03 '22 10:08 xpbob

Hi, @AlanBateman I updated the code and move ContainerInfoMXBean to jdk.management Can the CSR tag be removed?

xpbob avatar Aug 11 '22 01:08 xpbob

@xpbob This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

bridgekeeper[bot] avatar Sep 08 '22 03:09 bridgekeeper[bot]

@xpbob This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.

bridgekeeper[bot] avatar Oct 28 '22 05:10 bridgekeeper[bot]