Disable Tomcat's MBean Registry by default and provide a property to enable it

[philwebb]

See this thread https://tomcat.markmail.org/thread/kayfacujrpt2diht which appears to suggest we might be able to disable JMX.

[wilkinsona]

I tried this a while ago. The best I came up with was to provide a stubbed out org.apache.tomcat.util.modeler.Registry implementation. I couldn't find a way to plug it in without using reflection as the Registry instance is retrieved via a static method backed by a private field so I abandoned it. I wonder if Meecroweave figured out something smarter.

[wilkinsona]

Meecroweave uses reflection to swap out the value of the static registry field on org.apache.tomcat.util.modeler.Registry.

I'll measure the gains and then we can decide if the performance improvement is worth the reflective hack.

[wilkinsona]

The benefits in terms of startup time are negligible for most people. I believe that's because the background preinitializer causes the expensive descriptor parsing to be done in parallel on a separate thread. The benefits will be more apparent in CPU constrained environments where the preinitializer may be disabled.

The benefits in terms of memory usage are more noticeable. With a NoOpRegistry in place an app that previously required -Xmx13M will start with -Xmx11M. This was tested with Tomcat 9.0.17.

[wilkinsona]

With Tomcat 9.0.19 the same app starts with -Xmx9M by default (thanks to apache/tomcat#146) and with -Xmx7M with NoOpRegistry. That ~2MB reduction becomes increasingly attractive when considered as a percentage of the required heap.

[wilkinsona]

@markt-asf Would you consider a Tomcat enhancement that allows JMX to be switched off in a less hacky manner? Saving 2MB of heap is quite appealing, and it would be even more appealing if we didn't have to resort to reflection to do so.

[markt-asf]

It isn't exclusively my call but I have no objections to an enhancement request to do that. If you open an enhancement request in Bugzilla I'll add it to what is currently a rather long TODO list.

[wilkinsona]

Thanks. I've opened https://bz.apache.org/bugzilla/show_bug.cgi?id=63361.

[wilkinsona]

With thanks to @markt-asf, Tomcat 9.0.20 has a new Registry.disableRegistry() method that we can call to disable Tomcat's MBean registration. I think we should disable it by default and provide a mechanism to switch it back on. We could perhaps use spring.jmx.enabled or we may want a Tomcat-specific property.

[wilkinsona]

We think a Tomcat-specific property would be better. If we rely on spring.jmx.enabled it won't be possible to get things back into the state that they would be in 2.1.x when spring.jmx.enabled is set to false. Something like server.tomcat.registry.enabled.

[wilkinsona]

A side-effect of disabling the registry is that some Tomcat metrics are lost due to them being MBean-based. HTTP session metrics are retained, but thread pool, cache, servlet, and global request metrics are all lost. It may be possible to bind some of those metrics differently to avoid using MBeans as a source.

[wilkinsona]

We're going to document the need to enable the registry if you want the MBean-based metrics. I've opened micrometer-metrics/micrometer#1433 to see if Micrometer can avoid using Tomcat's MBeans for more of its metrics.

[dreis2211]

I think this breaks the build at the moment. The Tomcat EE tests don't have that method available as they pull in an older version of tomcat as it seems. Would the following be a viable solution?

	Method disableMethod = ReflectionUtils.findMethod(Registry.class, "disableRegistry");
	if (disableMethod != null) {
		ReflectionUtils.invokeMethod(disableMethod, null);
	}

In the end this involves reflection, which I guess you wanted to avoid @wilkinsona

[wilkinsona]

That code shouldn't run at all on TomEE as it's intended to be specific to embedded Tomcat. I'll take a look. Thanks, @dreis2211.

[sverhagen]

	if (disableMethod != null) {
		ReflectionUtils.invokeMethod(disableMethod, null);
	}

Would this code not also "break" a situation where multiple tests call TomcatServletWebServerFactory.getWebServer? I have a project with one @SpringBootTest and one test that starts the application through SpringApplication.run, and the second call to getWebServer calls the following code in Tomcat's Registry for the second time, so that registry already holds an instance of NoDescriptorRegistry and thus logs the warning.

    public static synchronized void disableRegistry() {
        if (registry == null) {
            registry = new NoDescriptorRegistry();
        } else {
            log.warn(sm.getString("registry.noDisable"));
        }

    }

I put "break" (above) in quotes, but since we fail our tests on unexpected warnings in the log, I have now a broken test. It's an unfortunate warning. (I understand I can fix it using the server.tomcat.mbeanregistry.enabled=true, but it's not quite pretty.)

[wilkinsona]

Unfortunately, the mechanism by which Tomcat's registry is disabled and the logging which results if it's called multiple times are out of Spring Boot's control. I've opened https://bz.apache.org/bugzilla/show_bug.cgi?id=63981 to see if the Tomcat team could make an improvement.

[1element]

Thanks for addressing the issue to the Tomcat team. I stumbled upon this false-positive warning (The MBean registry cannot be disabled because it has already been initialised) because we use a different port for the management server in our application (management.server.port is set).

[Alanne-Soares]

By default Tomcat is enabled as 'false' we should do it like this:

server.tomcat.mbeanregistry.enabled=true

[davidmichaelkarr]

I find myself now looking at ways to force this flag to to be true unconditionally. We needed it to be true for dynatrace integration, but my naive self thought that just stating that in the instructions for developers would be enough. We control the start script, so I can just add "-Dserver.tomcat.mbeanregistry.enabled=true". It just seems odd to have to do this.