New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for platform MBeanServer #83

Closed
cruftex opened this Issue Sep 20, 2015 · 6 comments

Comments

Projects
None yet
4 participants
@cruftex
Member

cruftex commented Sep 20, 2015

The spec says on page 129:

Simple in-process implementations might simply use the platform MBeanServer, accessed using
ManagementFactory.getPlatformMBeanServer(). The caching implementation must document
how to resolve the MBeanServer that was used to store cache management information.

However, the spec indirectly enforces that a separate MBeanServer is registered, because it wants to retrieve it by the id stored in the property org.jsr107.tck.management.agentId.

The Oracle Coherence does register its beans always with the platform MBeanServer. They sneaked around the TCK shortcoming by providing a server wrapper which constantly produces the id Coherence WrapperMBeanServer for the platform MBeanServer.

All other existing implementations did copy the approach of the RI and provided a custom implementation of MBeanServerBuilder to produce a separate MBeanServer which the TCK can pick up.

The TCK should support the simple approach, registering the bean with the platform MBeanServer, straight forward. All implementations except Oracle produced extra code to register a separate MBeanServer to comply with the TCK, however, the spec does not require that.

At other places the Spec clearly says, that implementation must register with the platform mbean server. See JavaDoc of CacheManager.enableManagement().

Controls whether management is enabled. If enabled the {@link CacheMXBean}
for each cache is registered in the platform MBean server. The platform
MBeanServer is obtained using
{@link ManagementFactory#getPlatformMBeanServer()}.

The Spec should be clear whether this is mandatory or not at all places.

@cruftex

This comment has been minimized.

Member

cruftex commented Sep 20, 2015

The needed change is trivial. The method TestSupport.resolveMBeanServer() is now:

  public static MBeanServer resolveMBeanServer() {

    if (System.getProperty("javax.management.builder.initial") == null) {
      throw new RuntimeException("You must set the 'javax.management.builder" +
          ".initial' property so that the TCK can find your MBeanServer'");
    }
    if (System.getProperty("org.jsr107.tck.management.agentId") == null) {
      throw new RuntimeException("You must set the 'org.jsr107.tck.management.agentId'" +
          "system property so that the TCK can find your MBeanServer agent by " +
          "its ID.'");
    }

    String agentId = System.getProperty("org.jsr107.tck.management.agentId");
    ArrayList<MBeanServer> mBeanServers = MBeanServerFactory.findMBeanServer(agentId);
    if (mBeanServers.size() < 1) {
      throw new CacheException("The specification requires registration of " +
          "MBeans in an implementation specific MBeanServer. A search for an " +
          "MBeanServer did not find any.");
    }
    return mBeanServers.get(0);
  }

It can be changed to:

  public static MBeanServer resolveMBeanServer() {
    String agentId = System.getProperty("org.jsr107.tck.management.agentId");
    ArrayList<MBeanServer> mBeanServers = MBeanServerFactory.findMBeanServer(agentId);
    if (mBeanServers.size() < 1) {
      throw new CacheException("The specification requires registration of " +
          "MBeans in an implementation specific MBeanServer. A search for an " +
          "MBeanServer did not find any.");
    }
    return mBeanServers.get(0);
  }

If agentId is null, just the first (and only) MBeanServer gets picked.

I can check this thoroughly this and make a pull request. Any comments?

@ljacomet

This comment has been minimized.

ljacomet commented May 15, 2016

@cruftex Ehcache 3 also takes the approach of registering MBeans with the platform MBeanServer and provides a bridge for the TCK execution.

I agree it would be nice if the TCK was testing the real runtime behaviour and not a fabricated one.

@cruftex

This comment has been minimized.

Member

cruftex commented Dec 14, 2016

Preparing an PR.

Cache implementations need to change behavior and register with the platform MBean server.

The TCK guide needs an update. The properties javax.management.builder.initial and org.jsr107.tck.management.agentId are no longer needed.

Cache implementations that want to make the transition before TCK 1.1 is released, can use some glue code that makes the platform MBean server available to the TCK under a specified ID. See, for example: https://github.com/cache2k/cache2k/blob/master/cache2k-jcache/src/main/java/org/cache2k/jcache/provider/tckGlue/TckMBeanServerBuilder.java

@cruftex

This comment has been minimized.

Member

cruftex commented Dec 14, 2016

PRs for RI and TCK are ready.

@cruftex cruftex added the PR-Pending label Dec 14, 2016

@vbekiaris

This comment has been minimized.

Contributor

vbekiaris commented Mar 13, 2017

Fixed by #119 & jsr107/RI#56

@vbekiaris vbekiaris closed this Mar 13, 2017

@gregrluck

This comment has been minimized.

Member

gregrluck commented Nov 4, 2017

The two properties were not removed from the surefire plugin.

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