Karaf
  1. Karaf
  2. KARAF-1243

Karaf JMX Config MBean behaves in unpredictable ways

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.2.5
    • Fix Version/s: 2.2.6, 3.0.0
    • Component/s: karaf-config
    • Labels:
      None

      Description

      Although the API of the Karaf Config MBean looks like it offers synchronous calls, in reality
      this is not true. I ran into some issues with this, because I tried to install configurations
      for bundles to be picked up (see attached maven test case project to be used with a default running
      Karaf 2.2.5 container)

      It seems that the config bundle may need quite a long time to finish applying the configuration
      changes, but returns BEFORE this is finished in background.

      I have searched around, dug through code, but found no way to determine at which time I can
      safely assume that a configuration change is finished and I can continue with installing the bundle
      that may pick up this configuration (in some cases the default configuration is enough, but more
      often I want to override that...). So I finally ended up with polling for the configuration changes,
      but even that sometimes fails and I could loop infinitely and get no result (got an empty Map from proplist method).

      From the JavaDoc of the MBean I would - as said - expect that I do not have to do that, but could
      assume that after a create or propset call returns, the configuration is active. I would have been
      a bit happier, if the config MBean offered JMX events that notify about the completion of a
      configuration change, but after digging deeper it seems that even that would not be an optimal solution.

      From my understanding the underlying implementation is a mix of Karaf maintaining its own config
      files inside the etc folder plus updating data inside the OSGi ConfigAdmin Service.

      The good thing about the console commands is that they implement a strategy that really
      matches the idea of the OSGi ConfigAdmin service (the service always publishes complete configurations
      as Dictionary instances). So on the console the config commands maintain state between config:edit
      and config:update. Sending off config:update will publish all collected changes as one configuration
      instance.

      The API on the JMX level was more or less directly derived from the way the config console commands
      work, but tries to avoid maintaining state - so there's no equivalent to config:edit / config:update.
      Instead, propset, propdel and propappend immediately trigger propagation of simple changes to the
      configuration. So on JMX level the granularity is not sending of a new Dictionary instance, but
      reflects the operations to create such an instance. Instead of publishing one change a configuration
      containing 20 properties would trigger 20 changes.

      This will also cause multiple notifications for any "interested" configuration change listener ...
      resulting not only in more load, but also possibly in inconsistent configuration sub stages to cope
      with: Imagine a service requires two configuration parameters to work correctly. Instead of getting
      one configuration notification that contains the two parameters, it will get two notifications and thus will have to handle the case that the second parameter is missing (desperately hoping that sooner or later this second parameter will appear ...).

      Phew ' ... so I would suggest that this API should be changed:

      • guarantee that on return from an MBean method that changes a configuration, this configuration
        can be assumed to be active (so after creating a query call will return the equivalent created config).
      • if this appears to be impossible, the bean should at least provide JMX events a client can listen
        to in order to be notified when a configuration call is finished.
      • in order to play nicely inside the OSGi framework, it should be possible to create configurations
        by calling void create(String pid, Map<String, String> config)
      • propset / propappend / propdel should be deprecated or (better) directly removed. (They make sense
        on console, but not for JMX).
      1. karaf2.2.x-config.patch
        53 kB
        Christian Schneider
      2. KarafConfigMBeanTest.zip
        3 kB
        Jürgen Kindler

        Issue Links

          Activity

          Hide
          Jürgen Kindler added a comment -

          Test case inside a Maven project - just start Karaf and build the project.

          Show
          Jürgen Kindler added a comment - Test case inside a Maven project - just start Karaf and build the project.
          Hide
          Jean-Baptiste Onofré added a comment -

          We discussed with Christian about this issue. We will fix that.

          Show
          Jean-Baptiste Onofré added a comment - We discussed with Christian about this issue. We will fix that.
          Hide
          Christian Schneider added a comment -

          Here is my first version of a patch for 2.2.x.

          It contains:

          • clean up the duplicate code between mbeans and shell
          • Trigger install for FileInstall to minimize latency of updates
            . Add update method to mbean to write all properties at once
          Show
          Christian Schneider added a comment - Here is my first version of a patch for 2.2.x. It contains: clean up the duplicate code between mbeans and shell Trigger install for FileInstall to minimize latency of updates . Add update method to mbean to write all properties at once
          Hide
          Jean-Baptiste Onofré added a comment -

          I'm reviewing it.

          Show
          Jean-Baptiste Onofré added a comment - I'm reviewing it.
          Hide
          Jürgen Kindler added a comment -

          For now I was not able to build karaf-2.2.x with the patch ...

          Show
          Jürgen Kindler added a comment - For now I was not able to build karaf-2.2.x with the patch ...
          Hide
          Jean-Baptiste Onofré added a comment -

          I will review the patch and at least add the configuration.update() on Karaf 2.2.x propset() method.

          Anyway, I think that the Christian's patch should not be apply like this as the signature of ConfigMBean changed. It should not be the case on a maintenance branch.

          Show
          Jean-Baptiste Onofré added a comment - I will review the patch and at least add the configuration.update() on Karaf 2.2.x propset() method. Anyway, I think that the Christian's patch should not be apply like this as the signature of ConfigMBean changed. It should not be the case on a maintenance branch.
          Hide
          Christian Schneider added a comment -

          JB please let me do this change. I will not apply the patch. Instead only a small fix to the update issue.

          Show
          Christian Schneider added a comment - JB please let me do this change. I will not apply the patch. Instead only a small fix to the update issue.
          Hide
          Jean-Baptiste Onofré added a comment -

          No worries, I was thinking that you are waiting for a review of your patch. Go ahead

          Show
          Jean-Baptiste Onofré added a comment - No worries, I was thinking that you are waiting for a review of your patch. Go ahead
          Hide
          Jamie goodyear added a comment -

          Hi Christian,

          Could you update us on the status of this issue?

          If you can apply a smaller patch for fixing 2.2.6 in the event that the current one would break compatibility, and a different one for Karaf 3.0.0 that would be fine. If you apply different fixes then just clone this issue; the original one for 2.2.6, and the clone for 3.0.0.

          Show
          Jamie goodyear added a comment - Hi Christian, Could you update us on the status of this issue? If you can apply a smaller patch for fixing 2.2.6 in the event that the current one would break compatibility, and a different one for Karaf 3.0.0 that would be fine. If you apply different fixes then just clone this issue; the original one for 2.2.6, and the clone for 3.0.0.
          Hide
          Jean-Baptiste Onofré added a comment -

          AFAIK, Christian has committed on trunk and he's going to commit on karaf-2.2.x branch.

          Show
          Jean-Baptiste Onofré added a comment - AFAIK, Christian has committed on trunk and he's going to commit on karaf-2.2.x branch.
          Hide
          Christian Schneider added a comment -

          The issue should be fixed on trunk and 2.2.x

          Show
          Christian Schneider added a comment - The issue should be fixed on trunk and 2.2.x

            People

            • Assignee:
              Christian Schneider
              Reporter:
              Jürgen Kindler
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development