Improve FeatureInstaller #3049

J-N-K · 2022-07-24T19:23:12Z

There are some flaws in the FeatureInstaller which this PR tries to solve. While debugging #3025 I found that the periodic sync job sets the config cache before the configuration is applied. This might lead to issues if another call to modified re-sets the configuration to something else but does not update the configuration cache. It is also not ensured that all configuration updates are processed in the order they arrive, so the asynchronous processing of the updates might result in an older configuration overwriting a newer one.

This PR

introduces a queue that holds the configuration supplied to modified and processes them in the correct order
ensures that configuration cache is properly updated when an update is processed (independent from the configuration source)
removed nearly all synchronized as they are no longer needed
some other code clean-up

Signed-off-by: Jan N. Klug github@klug.nrw

Signed-off-by: Jan N. Klug <github@klug.nrw>

J-N-K · 2022-07-26T20:04:56Z

...s/org.openhab.core.karaf/src/main/java/org/openhab/core/karaf/internal/FeatureInstaller.java

            }
-            featuresService.installFeatures(addons,
-                    EnumSet.of(FeaturesService.Option.Upgrade, FeaturesService.Option.NoFailOnFeatureNotFound));
+            featuresService.installFeatures(addons, EnumSet.of(FeaturesService.Option.NoAutoRefreshBundles,


@wborn Do you know if this might cause issues? The reason I added it is that some add-ons cause org.openhab.core to refresh if it is not set. As a result the EventPublisher is unloaded and this service is stopped. If a lot of add-ons are installed at the same time, this will result in an InterruptedException in FeaturesService.installFeatures which in turn shows up as ERROR in the log. At the next start everything is fixed.

It would be great if there is a documentation for the FeaturesService.Option (at least I didn't find it). Do you know of such a document?

I don't think there is much documentation for the FeaturesService besides JavaDocs and the code itself. So if that doesn't help, mailing questions on the Karaf mailing list might help.

Maybe you can figure out what causes the bundle refresh in the first place and prevent it from happening?

The Karaf docs describes the reasons for automatically refreshing bundles:

For instance, a bundle has to be refreshed:

when bundle A uses an import package, and bundle B is installed providing a new version of the package (matching bundle A import version range). Then bundle A has to be refreshed to use the new version of the package.

when bundle A uses an optional import package, and bundle B is installed providing this package. Then, bundle A has to be refreshed to actually use the package.

when bundle A uses a package provided by bundle B, then, bundle A will be refreshed as well. This is kind of "cascading" refresh.

As a result the EventPublisher is unloaded and this service is stopped

Would the feature installation succeed when refreshing occurs while the EventPublisher has reference cardinaltiy optional? If that works, it could continue and for instance post any events whenever the EventPublisher is set again.

I don't think that makes a difference. If the EventPublisher is removed, this is most likely because the org.openhab.core bundle is refreshed. This will automatically refresh the org.openhab.core.karaf bundle and the service is de-activated anyway. I now removed immediate refreshing during installation and added a refresh after all config updates have been processed (but only if the add-on configuration changed).

J-N-K · 2022-07-27T18:56:24Z

I think I found it (or at least one reason). This is the tree for org.openhab.core before any add-on is installed:

Bundle org.openhab.core [148] is currently ACTIVE

org.openhab.core [148]
+- tech.units.indriya [230]
|  +- javax.measure.unit-api [52]
|  +- org.apache.servicemix.bundles.javax-inject [86]
|  +- uom-lib-common [231]
|     +- javax.measure.unit-api [52]
+- org.apache.karaf.services.eventadmin [2]
|  +- org.apache.felix.configadmin [12]
|  |  +- org.apache.felix.coordinator [10]
|  +- org.apache.felix.metatype [4]
+- uom-lib-common [231]
+- si-units [227]
|  +- javax.measure.unit-api [52]
|  +- tech.units.indriya [230]
|  +- si.uom.si-quantity [228]
|  |  +- javax.measure.unit-api [52]
|  +- org.apache.servicemix.bundles.javax-inject [86]
+- javax.measure.unit-api [52]
+- com.google.gson [35]
+- org.apache.felix.scr [68]
|  +- org.apache.felix.configadmin [12]
|  +- org.apache.felix.metatype [4]
|  +- org.osgi.util.promise [9]
|  |  +- org.osgi.util.function [8]
|  +- org.apache.karaf.shell.core [23]
|     +- org.apache.karaf.services.eventadmin [2]
|     +- org.apache.felix.configadmin [12]
|     +- org.ops4j.pax.logging.pax-logging-api [5]
|     |  +- org.apache.karaf.services.eventadmin [2]
|     +- org.jline [25]
|        +- org.apache.sshd.sftp [93]
|        |  +- org.apache.sshd.osgi [91]
|        |  |  +- bcpkix [26]
|        |  |  |  +- bcprov [27]
|        |  |  |  +- bcutil [28]
|        |  |  |     +- bcprov [27]
|        |  |  +- org.ops4j.pax.logging.pax-logging-api [5]
|        |  |  +- bcprov [27]
|        |  +- org.ops4j.pax.logging.pax-logging-api [5]
|        +- org.apache.sshd.scp [92]
|        |  +- org.apache.sshd.osgi [91]
|        |  +- org.ops4j.pax.logging.pax-logging-api [5]
|        +- org.fusesource.jansi [6]
|        +- org.apache.sshd.osgi [91]
+- org.ops4j.pax.logging.pax-logging-api [5]

When installing binding-amazondashbutton the bundle reloads. This is because the binding also installs the feature openhab-runtime-jna and after the installation the tree looks like that:

Bundle org.openhab.core [148] is currently ACTIVE

org.openhab.core [148]
+- tech.units.indriya [230]
|  +- javax.measure.unit-api [52]
|  +- org.apache.servicemix.bundles.javax-inject [86]
|  +- uom-lib-common [231]
|     +- javax.measure.unit-api [52]
+- org.apache.karaf.services.eventadmin [2]
|  +- org.apache.felix.configadmin [12]
|  |  +- org.apache.felix.coordinator [10]
|  +- org.apache.felix.metatype [4]
+- uom-lib-common [231]
+- si-units [227]
|  +- javax.measure.unit-api [52]
|  +- tech.units.indriya [230]
|  +- si.uom.si-quantity [228]
|  |  +- javax.measure.unit-api [52]
|  +- org.apache.servicemix.bundles.javax-inject [86]
+- javax.measure.unit-api [52]
+- com.google.gson [35]
+- org.apache.felix.scr [68]
|  +- org.apache.felix.configadmin [12]
|  +- org.apache.felix.metatype [4]
|  +- org.osgi.util.promise [9]
|  |  +- org.osgi.util.function [8]
|  +- org.apache.karaf.shell.core [23]
|     +- org.apache.karaf.services.eventadmin [2]
|     +- org.apache.felix.configadmin [12]
|     +- org.ops4j.pax.logging.pax-logging-api [5]
|     |  +- org.apache.karaf.services.eventadmin [2]
|     +- org.jline [25]
|        +- org.apache.sshd.sftp [93]
|        |  +- org.apache.sshd.osgi [91]
|        |  |  +- bcpkix [26]
|        |  |  |  +- bcprov [27]
|        |  |  |  +- bcutil [28]
|        |  |  |     +- bcprov [27]
|        |  |  +- org.ops4j.pax.logging.pax-logging-api [5]
|        |  |  +- bcprov [27]
|        |  +- org.ops4j.pax.logging.pax-logging-api [5]
|        +- org.apache.sshd.scp [92]
|        |  +- org.apache.sshd.osgi [91]
|        |  +- org.ops4j.pax.logging.pax-logging-api [5]
|        +- org.apache.sshd.osgi [91]
|        +- com.sun.jna [256]
|        +- org.fusesource.jansi [6]
+- org.ops4j.pax.logging.pax-logging-api [5]

So an optional dependency is fulfilled and because of (2) the core bundle is refreshed. This causes a lot of other bundles to refresh (including all UI bundles). I believe there are similar issues with other bundles/features and this might also be the reason for a lot of strange behavior when installing add-ons, especially if several add-ons are installed at the same time (e.g. after an upgrade). This not only affects the FeatureInstaller but also other services (e.g. the KarafAddonService.

I'm not sure how to address that. We could add openhab-runtime-jna to openhab-runtime-base, this will solve the issue I found here. It does not prevent similar issues and while there may be a general benefit if we provide JNA by default, this might not be the case for other optional dependencies. Maybe also creating an override that blacklists the JNA bundle might work. Another option is to prevent bundle refreshes during add-on-installation and refresh after all configuration changes have been processed.

Signed-off-by: Jan N. Klug <github@klug.nrw>

openhab-bot · 2022-08-18T15:13:10Z

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/featureinstaller-failed-installing-a-list-of-every-binding-ui-and-so-on-null/138242/2

wborn · 2022-08-20T11:36:37Z

We could add openhab-runtime-jna to openhab-runtime-base, this will solve the issue I found here.

Looks like this is a required dependency for #3004 anyhow.

Signed-off-by: Jan N. Klug <github@klug.nrw>

J-N-K · 2022-08-20T13:45:27Z

I moved JNA to an openhab.tp-jna feature and included that in the openhab-core-base feature. This needs an adjustment in openhab-addons: openhab/openhab-addons#13298

wborn

Thanks! Let's test if this fixes all the flaws. 🙂

* Improve FeatureInstaller * Remove unnecessary synchronized and clean up processing * Re-add refeshing bundles after all configuratzion changes are processed * Prevent unnecessary refreshes * Make JNA part of the tp Signed-off-by: Jan N. Klug <github@klug.nrw> GitOrigin-RevId: dca8088

Improve FeatureInstaller

c48ac08

Signed-off-by: Jan N. Klug <github@klug.nrw>

J-N-K changed the title ~~Improve FeatureInstaller~~ [WIP] Improve FeatureInstaller Jul 24, 2022

J-N-K added 3 commits July 24, 2022 21:40

fix spotless

b79c989

Signed-off-by: Jan N. Klug <github@klug.nrw>

Remove unnecessary synchronized and clean up processing

a3f7cc5

Signed-off-by: Jan N. Klug <github@klug.nrw>

further improvements

f3caa1c

Signed-off-by: Jan N. Klug <github@klug.nrw>

J-N-K commented Jul 26, 2022

View reviewed changes

J-N-K marked this pull request as ready for review July 26, 2022 20:05

J-N-K requested a review from a team as a code owner July 26, 2022 20:05

J-N-K added 2 commits July 27, 2022 21:33

re-add refeshing bundles after all configuratzion changes are processed

c0de1bb

Signed-off-by: Jan N. Klug <github@klug.nrw>

prevent unnecessary refreshes

326749f

Signed-off-by: Jan N. Klug <github@klug.nrw>

J-N-K changed the title ~~[WIP] Improve FeatureInstaller~~ Improve FeatureInstaller Jul 29, 2022

J-N-K mentioned this pull request Aug 13, 2022

openHAB add-ons are not shown as installed anymore #3025

Closed

wborn added the bug An unexpected problem or unintended behavior of the Core label Aug 20, 2022

Make JNA part of the tp

122bd1a

Signed-off-by: Jan N. Klug <github@klug.nrw>

J-N-K mentioned this pull request Aug 20, 2022

Adapt to core-changes (added JNA to tp) openhab/openhab-addons#13298

Merged

wborn approved these changes Sep 18, 2022

View reviewed changes

wborn merged commit dca8088 into openhab:main Sep 18, 2022

wborn added this to the 3.4 milestone Sep 18, 2022

J-N-K deleted the improve-featureinstaller branch September 18, 2022 09:27

wborn mentioned this pull request Sep 23, 2022

[vosk] Missing native dependency openhab/openhab-addons#13423

Closed

mherwege mentioned this pull request May 6, 2024

Resolver error introduced with Karaf upgrade to 4.4.5 #4158

Closed

J-N-K mentioned this pull request May 7, 2024

Snapshot 4069: failed to install bundles at startup #4222

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve FeatureInstaller #3049

Improve FeatureInstaller #3049

J-N-K commented Jul 24, 2022 •

edited

Loading

J-N-K Jul 26, 2022

wborn Jul 27, 2022

wborn Aug 20, 2022

J-N-K Aug 20, 2022 •

edited

Loading

J-N-K commented Jul 27, 2022 •

edited

Loading

openhab-bot commented Aug 18, 2022

wborn commented Aug 20, 2022

J-N-K commented Aug 20, 2022

wborn left a comment

Improve FeatureInstaller #3049

Improve FeatureInstaller #3049

Conversation

J-N-K commented Jul 24, 2022 • edited Loading

J-N-K Jul 26, 2022

Choose a reason for hiding this comment

wborn Jul 27, 2022

Choose a reason for hiding this comment

wborn Aug 20, 2022

Choose a reason for hiding this comment

J-N-K Aug 20, 2022 • edited Loading

Choose a reason for hiding this comment

J-N-K commented Jul 27, 2022 • edited Loading

openhab-bot commented Aug 18, 2022

wborn commented Aug 20, 2022

J-N-K commented Aug 20, 2022

wborn left a comment

Choose a reason for hiding this comment

J-N-K commented Jul 24, 2022 •

edited

Loading

J-N-K Aug 20, 2022 •

edited

Loading

J-N-K commented Jul 27, 2022 •

edited

Loading