Skip to content
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

Upgrade to Karaf 4.2.1 #761

Merged
merged 1 commit into from
Sep 14, 2018
Merged

Upgrade to Karaf 4.2.1 #761

merged 1 commit into from
Sep 14, 2018

Conversation

wborn
Copy link
Member

@wborn wborn commented Sep 11, 2018

Upgrades Karaf to 4.2.1. So far I haven't run into anything major while upgrading.

There seems to be a new Jetty warning that may need to be addressed:

01:18:23.511 [WARN ] [se.jetty.util.thread.ThreadPoolBudget] - Low configured threads: (max=8 - required=1)=7 < warnAt=8 for QueuedThreadPool[ServletModel-37]@4b1f06{STARTING,8<=0<=8,i=0,q=0}[ReservedThreadExecutor@3bc120c4{s=0/1,p=0}]

Besides that it seems to work just as well as the current snapshot build. 😄
Though it could use some additional testing of course.

I've compiled some test artifacts for anyone who wants to help testing. It should also work with the online repository.

This Karaf version also supports Java 9 and 10. So I tested Java 10 on Ubuntu 18.04. The runtime starts and all UIs work properly. Though some errors/stacktraces show due to issues in some dependencies:

The next step is to have another look at the current state of configuring logging using XML instead of properties files (#516).

@kaikreuzer
Copy link
Member

Thanks @wborn, interesting results!

There seems to be a new Jetty warning that may need to be addressed

I would like to keep the thread number low, so the easiest way to address this might be to configure the logger to swallow messages from this class.

Though it could use some additional testing of course.

Did some initial testing, which also looks good so far. I think we can move forward with that.

Though some errors/stacktraces show due to issues in some dependencies

Could you check if the xstream warning can be addressed by adding --illegal-access=permit to the JVM call? Not sure what to do about rrd4j, though...

The next step is to have another look at the current state of configuring logging using XML

We can do that as a second step after the upgrade (and @cweitkamp might want to work on the logging part as he had already started looking into it).

@wborn
Copy link
Member Author

wborn commented Sep 11, 2018

I would like to keep the thread number low, so the easiest way to address this might be to configure the logger to swallow messages from this class.

I've now changed the log level so it is suppressed. You can read more about the reasons for adding this warning in jetty/jetty.project#2798. I.e. the HTTP/2 implementation requires more threads and there are also some false positive warnings for which a fix was merged.

Could you check if the xstream warning can be addressed by adding --illegal-access=permit to the JVM call?

It will still keep showing these warnings because this is the default mode.

According to the documentation:

permit: This mode opens packages in JDK 9 that existed in JDK 8 to code on the class path. This allows code on class path that relies on the use of setAccessible to break into JDK internals, or to do other illegal access on members of classes in these packages, to work as per previous releases. This enables both static access (such as, by compiled bytecode) and deep reflective access. Deep reflective access is accomplished through the platform's reflection APIs. The first reflective-access operation to any such package causes a warning to be issued. However, no warnings are issued after the first occurrence. This single warning describes how to enable further warnings. This mode is the default for JDK 9 but will change in a future release.

It does pickup my mode changes because when I first

export JAVA_OPTS="--illegal-access=warn"

and then start openHAB it shows more details about the reflective-access operations:

WARNING: Illegal reflective access by com.thoughtworks.xstream.core.util.Fields (file:/home/wouter/software/openhab/userdata/cache/org.eclipse.osgi/99/0/.cp/lib/xstream-1.4.7.jar) to field java.lang.reflect.Proxy.h                                         
WARNING: Illegal reflective access by com.thoughtworks.xstream.converters.reflection.AbstractAttributedCharacterIteratorAttributeConverter (file:/home/wouter/software/openhab/userdata/cache/org.eclipse.osgi/99/0/.cp/lib/xstream-1.4.7.jar) to method java.text.AttributedCharacterIterator$Attribute.getName()
WARNING: Illegal reflective access by com.thoughtworks.xstream.core.util.Fields (file:/home/wouter/software/openhab/userdata/cache/org.eclipse.osgi/99/0/.cp/lib/xstream-1.4.7.jar) to field java.awt.font.TextAttribute.instanceMap
WARNING: Illegal reflective access by io.netty.util.internal.PlatformDependent0 (file:/home/wouter/software/openhab/userdata/cache/org.eclipse.osgi/222/0/.cp/lib/io.netty.netty-all-4.0.32.Final.jar) to field java.nio.Buffer.address
WARNING: Illegal reflective access by io.netty.channel.nio.NioEventLoop (file:/home/wouter/software/openhab/userdata/cache/org.eclipse.osgi/222/0/.cp/lib/io.netty.netty-all-4.0.32.Final.jar) to field sun.nio.ch.SelectorImpl.selectedKeys
WARNING: Illegal reflective access by io.netty.channel.nio.NioEventLoop (file:/home/wouter/software/openhab/userdata/cache/org.eclipse.osgi/222/0/.cp/lib/io.netty.netty-all-4.0.32.Final.jar) to field sun.nio.ch.SelectorImpl.publicSelectedKeys
WARNING: Illegal reflective access by io.netty.channel.nio.NioEventLoop (file:/home/wouter/software/openhab/userdata/cache/org.eclipse.osgi/222/0/.cp/lib/io.netty.netty-all-4.0.32.Final.jar) to field sun.nio.ch.SelectorImpl.selectedKeys
WARNING: Illegal reflective access by io.netty.channel.nio.NioEventLoop (file:/home/wouter/software/openhab/userdata/cache/org.eclipse.osgi/222/0/.cp/lib/io.netty.netty-all-4.0.32.Final.jar) to field sun.nio.ch.SelectorImpl.publicSelectedKeys

We can do that as a second step

That's OK with me since it probably has more impact than just upgrading Karaf.

Unless there are more review comments the only work remaining is more additional testing.

@wborn wborn changed the title [WIP] Upgrade to Karaf 4.2.1 Upgrade to Karaf 4.2.1 Sep 11, 2018
@kaikreuzer
Copy link
Member

there are also some false positive warnings for which a fix was merged.

But we are upgrading to the unfixed version as it seems :-)

Unless there are more review comments the only work remaining is more additional testing.

Ok, let's schedule that for the weekend. Maybe @martinvw and others want to do some testing until then as well.

Signed-off-by: Wouter Born <eclipse@maindrain.net>
@wborn
Copy link
Member Author

wborn commented Sep 12, 2018

Maybe @martinvw and others want to do some testing until then as well.

A new test release has been created that uses the default bundled Jetty version and filters the ThreadPoolBudget warning.

You can download the test artifacts from:
https://github.com/wborn/openhab-distro/releases/tag/2.4.0-k421.20180912

I've also let Travis create some Docker images 🐳 using this Karaf 4.2.1 test release, for the tags see:
https://hub.docker.com/r/wborn/openhab-k421/tags/

@openhab-bot
Copy link
Collaborator

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

https://community.openhab.org/t/help-testing-karaf-4-2-1/51321/1

@BClark09
Copy link
Member

BClark09 commented Sep 12, 2018

image

I built an apt package out of the tar you provided, installs and updates well. Don't have the courage/time to run it on my home system yet (to test on an arm chip) but I've had no new errors with location, language or the Astro/NTP/Systeminfo binding.

https://openhab.jfrog.io/openhab/linuxpkg-testing/pool/main/2.4.0~20180912202921/openhab2_2.4.0~20180912202921-1_all.deb

@wborn, would you like me to post that deb file link in the OH thread?

@wborn
Copy link
Member Author

wborn commented Sep 12, 2018

Thanks for the feedback and Debian package @BClark09 ! The reflective access warnings show you've been testing on a newer Java version as well. 😉

would you like me to post that deb file link in the OH thread?

Your package also works well so far on my Ubuntu 18.04 VM with Java 10 so I'm OK with that. 👍

@BClark09
Copy link
Member

Haven't found a way to break it yet, will try on my RPi as soon as I can!

Nice one!

@BClark09
Copy link
Member

BClark09 commented Sep 13, 2018

Running on my RPi now and Z-Wave, Zigbee, Sonos devices are responsive and working well.

I had to use openJDK-9 to test later Java functionality, a few errors did pop up but restarting the Pi meant that these didn't show up again. Many warnings about all the .xml files in z-wave though:

The XML document '/ESH-INF/thing/zwaveproducts/wd100_0_0.xml' in module 'org.openhab.binding.zwave' could not be parsed: The XmlConfigDescriptionProvider must not be null!
java.lang.IllegalArgumentException: The XmlConfigDescriptionProvider must not be null!

This is probably unrelated to the PR?

It doesn't look like there will be any Oracle or Zulu 9/10/11 builds for 32 bit systems. So I'm hoping arm64 architecture will be more widely adopted.

@kaikreuzer
Copy link
Member

It doesn't look like there will be any Oracle or Zulu 9/10/11 builds for 32 bit systems.

Do you read any news about that or do you just reckon based on the fact that Zulu so far only provides Java 8 builds?

@BClark09
Copy link
Member

I read that Oracle won't be providing them, but only assumed (probably incorrectly) that since Zulu haven't provided any yet, that they were doing the same.

@wborn
Copy link
Member Author

wborn commented Sep 13, 2018

Some companies (e.g. RedHat) completely skip supporting the short-term rapid release versions (Java 9, 10). The next Java 11 LTS release will probably have a lot more traction also because the public updates end for the Java 8 LTS release in 2019. In the mean time it's nice to see what issues need to be addressed for supporting the next Java 11 LTS release.

@wborn
Copy link
Member Author

wborn commented Sep 13, 2018

Many warnings about all the .xml files in z-wave though:
...
This is probably unrelated to the PR?

I've seen the same warnings too occasionally with other bindings (astro, sonos) after just starting the runtime. Apparently eclipse-archived/smarthome#5616 was created to resolve it. There have been no reports in the community about it again since it was fixed. So far I've only seen it when using Java 10. I'll keep testing and see if it is reproducible also with Java 8.

@kaikreuzer
Copy link
Member

Interestingly, I just had the same in my fresh openHAB IDE setup (standard, Java8, no Karaf). So somehow this issue seems to be back.

@5iver
Copy link
Contributor

5iver commented Sep 14, 2018

Many warnings about all the .xml files in z-wave though:
...
This is probably unrelated to the PR?

No, these are not related to this PR. I have seen them for a while, but very sporadically. Just got the ones below yesterday after an OH restart (snapshot 1361). They are usually for the zwave binding, but not always.

2018-09-13 13:10:55.261 [WARN ] [org.eclipse.smarthome.config.xml.osgi.XmlDocumentBundleTracker] - The XML document '/ESH-INF/thing/bridge-server.xml' in module 'org.openhab.binding.zoneminder' could not be parsed: The XmlConfigDescriptionProvider must not be null!

2018-09-13 13:10:55.347 [WARN ] [org.eclipse.smarthome.config.xml.osgi.XmlDocumentBundleTracker] - The XML document '/ESH-INF/thing/thing-types.xml' in module 'org.openhab.binding.upnpcontrol' could not be parsed: The XmlConfigDescriptionProvider must not be null!

I'm currently running Oracle Java 8...

java version "1.8.0_181"
Java(TM) SE Runtime Environment (build 1.8.0_181-b13)
Java HotSpot(TM) 64-Bit Server VM (build 25.181-b13, mixed mode)

@kaikreuzer
Copy link
Member

but only assumed (probably incorrectly) that since Zulu haven't provided any yet, that they were doing the same.

Just found on their website: "Java 6, 7, 8 including JDK 8 Compact Profiles, plus future LTS releases" - so we should hopefully see a Java 11 version of Zulu embedded once it comes out. So I guess it also makes most sense for us to do testing with Java 11 soon as we will most likely rather support Java 8 and Java 11 than anything inbetween.

Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks!

@kaikreuzer kaikreuzer merged commit e101923 into openhab:master Sep 14, 2018
@wborn wborn deleted the karaf-4.2.1 branch September 14, 2018 15:18
@BClark09
Copy link
Member

BClark09 commented Sep 14, 2018

So I guess it also makes most sense for us to do testing with Java 11 soon as we will most likely rather support Java 8 and Java 11 than anything inbetween.

Seeing that Karaf 4.2.1 should also support JDK 11 I was able to compile the early access openJDK 11 for armhf. There were a few errors the first run of openHAB, but after a restart it seems to behaving and the UIs and NTP/Astro things seem to be working properly. Which is a good start I guess 🙂

pi@raspberrypi:~ $ java --version
openjdk version "11-armhf" 2018-09-25
OpenJDK Runtime Environment (build 11-armhf+28)
OpenJDK Server VM (build 11-armhf+28, mixed mode)

pi@raspberrypi:~ $ sudo systemctl status openhab2
● openhab2.service - openHAB 2 - empowering the smart home
   Loaded: loaded (/usr/lib/systemd/system/openhab2.service; enabled; vendor preset: enabled)
   Active: active (running) since Fri 2018-09-14 16:48:03 UTC; 12min ago
     Docs: https://www.openhab.org/docs/
           https://community.openhab.org
  Process: 21086 ExecStop=/usr/share/openhab2/runtime/bin/karaf stop (code=exited, status=0/SUCCESS)
 Main PID: 21246 (java)
   CGroup: /system.slice/openhab2.service
           └─21246 /usr/bin/java -Dopenhab.home=/usr/share/openhab2 -Dopenhab.conf=/etc/openhab2 -Dopenhab.runtime=/usr/share/openhab2/runtime -Dopenhab.userdata=/var/lib/openhab2 -Dopenhab.logdir=/var/log/openhab2 -Dfelix.cm.dir=/var/li

Sep 14 16:48:03 raspberrypi systemd[1]: Stopped openHAB 2 - empowering the smart home.
Sep 14 16:48:03 raspberrypi systemd[1]: Started openHAB 2 - empowering the smart home.
Sep 14 16:48:31 raspberrypi karaf[21246]: WARNING: An illegal reflective access operation has occurred
Sep 14 16:48:31 raspberrypi karaf[21246]: WARNING: Illegal reflective access by com.thoughtworks.xstream.core.util.Fields (file:/var/lib/openhab2/cache/org.eclipse.osgi/99/0/.cp/lib/xstream-1.4.7.jar) to field java.lang.reflect.Proxy.h
Sep 14 16:48:31 raspberrypi karaf[21246]: WARNING: Please consider reporting this to the maintainers of com.thoughtworks.xstream.core.util.Fields
Sep 14 16:48:31 raspberrypi karaf[21246]: WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
Sep 14 16:48:31 raspberrypi karaf[21246]: WARNING: All illegal access operations will be denied in a future release

@wborn
Copy link
Member Author

wborn commented Sep 14, 2018

That's good news regarding JDK 11 and a great accomplishment! :-) Did it take long to compile OpenJDK on a Pi or did you cross compile it on another machine?

To simplify JDK 11 testing, I just created an openhab-docker branch that makes Travis create Debian and Alpine amd64 Docker images using the JDK 11 Early-Access Builds. For the available tags see my wborn/openhab-jdk11 Docker Hub repository.

So far I haven't found any new issues with these images besides the known XStream warnings and no longer working RRD4J addon.

@fbacchella
Copy link

I have merged a commit that should help: rrd4j/rrd4j@4b791a7

@fbacchella
Copy link

But any way the problem is with the NIO backend only. I'm not sure it's the best backend, as it's doing mmap and I don't think it fits well rrd4j use case.

@wborn
Copy link
Member Author

wborn commented Sep 15, 2018

Thanks @fbacchella! With a local build of the rrd4j master integrated in the openHAB RRD4J persistence bundle the exceptions seem to be resolved on Java 11 now. 👍 We currently use an ancient rrd4j version (2.2.1, released in 2015) but so far it seems to be a drop in replacement. :-)

I also noticed the current version of MaryTTS in the voice addon no longer works with newer Java versions (marytts/marytts#783). Should we create a single (similar to eclipse-archived/smarthome#4369) or multiple issues for these Java compatibility issues @kaikreuzer?

@maggu2810
Copy link
Contributor

IMHO separated issues are better as they could be closed / fixed by respective merge requests one by one.

We could introduce a label or prefix to mark that issues.

@wborn
Copy link
Member Author

wborn commented Sep 15, 2018

IMHO separated issues are better

That's what I would prefer myself too. A single Java compatibility issue would probably be a never ending issue with new Java versions being released every 6 months. 😉

@kaikreuzer
Copy link
Member

Separate issues in the respective repos make sense, but I'd also suggest to create one overarching issue here in openhab-distro about Java 11 support where we can keep track of all the open issues that need to be tackled.

@kaikreuzer
Copy link
Member

@wborn Sounds as if you are also already close to creating PRs for rrd4j and maryTTS, right?

@BClark09
Copy link
Member

BClark09 commented Sep 17, 2018

Did it take long to compile OpenJDK on a Pi or did you cross compile it on another machine?

About 4 hours on the Pi3! 😉

As another note, I got the following error when using openJDK 9 on my RPi 3 when trying to use BasicUI (looking at an eventsource). Looking at openHAB's forums, it may be an issue with openJDK 8 too:

13:43:05.382 [ERROR] [ge.internal.WriterInterceptorExecutor] - MessageBodyWriter not found for media type=text/event-stream, type=class org.glassfish.jersey.media.sse.OutboundEvent, genericType=class org.glassfish.jersey.media.sse.OutboundEvent.
13:43:23.340 [ERROR] [ge.internal.WriterInterceptorExecutor] - MessageBodyWriter not found for media type=text/event-stream, type=class org.glassfish.jersey.media.sse.OutboundEvent, genericType=class org.glassfish.jersey.media.sse.OutboundEvent.

The error isn't present when using openJDK 11. At the moment, my home is fully functional with the exception of MaryTTS so this is looking very promising.

@wborn
Copy link
Member Author

wborn commented Sep 17, 2018

I'll create some follow up issues for supporting Java 11.

close to creating PRs for rrd4j and maryTTS, right?

Currently both projects only have a fix for the Java compatibility issues in an unreleased version. 😞

Looking at openHAB's forums, it may be an issue with openJDK 8 too

These errors are also subject for discussion in:

@openhab-bot
Copy link
Collaborator

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

https://community.openhab.org/t/low-configured-threads-warning/52084/4

@wborn wborn mentioned this pull request Dec 29, 2018
@wborn wborn added this to the 2.4 milestone Feb 28, 2019
@wborn wborn added the karaf label Jun 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants