GitHub user srdo opened a pull request:
https://github.com/apache/storm/pull/2733
STORM-3119: Build Storm with Java 10
https://issues.apache.org/jira/browse/STORM-3119
There are some changes in this PR that are only tangentially related to
building with JDK10, namely fixes to some flaky tests that would have made it a
pain to add extra test runs to every PR.
Changes:
* Add Java 10 to the Travis build matrix. Storm-cassandra and storm-hive
are excluded, because Cassandra and Hive aren't Java 9+ compatible yet.
* Add relevant Java EE APIs to components that use Jersey. The Java EE APIs
aren't available without add-modules in Java 9+, so we should include them
explicitly as dependencies. This is already done in most of the components, but
missed a few spots.
* The integration test Vagrant box has been updated so the user's name is
vagrant rather than ubuntu, fix this in the scripts
* Added environment variable to allow people to use Java 10 in integration
tests run via Vagrant
* Fixed the integration test so it runs on Java 10. The default
worker.childopts include removed JVM parameters related to GC logging. There
doesn't appear to be parameters that work on both Java 8 and 10, so we'll need
to add to the documentation how people should override the worker.childopts, if
they want to run on Java 10.
* Some of the Time code didn't make sense, e.g. checking for null on the
final THREAD_SLEEP_TIMES_NANOS map. It was left over from an earlier refactor
of Time. Cleaned it up, and deleted the deprecated Time methods. Also made sure
that when simulated time is advanced, idle threads are removed from the
THREAD_SLEEP_TIMES_NANOS immediately. When the LocalCluster waits for the
topology to be idle, it checks whether all timer threads are in the
THREAD_SLEEP_TIMES_NANOS map. It looks to me like there was no guarantee that
sleeping threads actually got a chance to run when time was advanced, because
they remained in the THREAD_SLEEP_TIMES_NANOS map until they happened to be
scheduled. With bad luck, a thread might end up being counted as idle when it
just hadn't exited the Time.sleepUntilNanos loop yet.
* The metrics-tests were doing assertions in a racy way. We can't guarantee
that metrics fall in exact buckets, because the metrics are updated from the
executor async loops, which aren't included in the threads covered by
LocalCluster.waitForIdle. I've replaced the checks with validation that there
are a certain number of metrics buckets, and that the metrics counters sum to
the expected value.
Example of a failing metrics test:
```
classname: org.apache.storm.metrics-test / testname:
test-custom-metric-with-multi-tasks
expected: (clojure.core/= [1 0 0 0 0 0 2] (clojure.core/subvec
(org.apache.storm.metrics-test/lookup-bucket-by-comp-id-&-metric-name! "2"
"my-custom-metric") 0 N__3207__auto__))
actual: (not (clojure.core/= [1 0 0 0 0 0 2] [1 0 0 0 0 0 0]))
at: test_runner.clj:105
```
In this case the missing 2 counts happen in a later bucket than the test is
checking for, because the executor async loop happened to not have run when the
metrics system added the last bucket.
Let me know if I need to split this PR up, or if a change is unclear. I ran
the tests a few times on Travis, and they appear stable.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/srdo/storm java10
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/2733.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2733
----
commit 0bef3d3b8c136d65aa9c4305f613de087c8f9f49
Author: Stig Rohde Døssing <srdo@...>
Date: 2018-06-18T20:10:36Z
STORM-3119: Build Storm with Java 10
----
---
