Java16 support #409
Merged
Java16 support #409
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Wow, macOS does seem to have a rather unprecise scheduler :-) |
16 tasks
Ladicek
force-pushed
the
java16-support
branch
2 times, most recently
from
39346c0 to
441a2ac
Compare
Ladicek
force-pushed
the
java16-support
branch
4 times, most recently
from
f33f07b to
0ee7a49
Compare
|
why do we have only windows checks? That's an interesting choice ;) |
michalszynkiewicz
previously approved these changes
May 10, 2021
Ladicek
force-pushed
the
java16-support
branch
3 times, most recently
from
a221e1e to
a5a972c
Compare
|
Look at the latest commit, I'm doing some debugging as there's a weird failure that never happened on Linux. That commit will disappear and we'll run CI on 8, 11, 16 and 17-ea on Linux, Windows and Mac. |
Ladicek
force-pushed
the
java16-support
branch
12 times, most recently
from
fc2999f to
a543044
Compare
Ladicek
force-pushed
the
java16-support
branch
3 times, most recently
from
e8462db to
fa023a2
Compare
In addition to Java 8 and 11, which are currently the Java LTS releases we care about, also run CI builds with 16 and, after a known bug in JBoss Class File Writer is fixed, 17-ea. The idea is that we'll run builds with all Java LTS versions that we care about, the latest Java GA release, and currently upcoming Java release. This means that when Java 17 goes GA, we'll run with 8, 11, 17 and 18-ea. Note that 17 will be an LTS release, so it will remain in the list even when 18 goes GA. At some point, we'll also want to drop 8. When it comes to the release build, we want to produce a MR JAR, so we have to run the build with Java 11. Note that we still produce a Java 8 bytecode by default.
This has two parts: - remove useless configuration from POM, this is already configured in SmallRye Parent; - make sure `<release>8</release>` is used for base layer compilation by creating the `build-release-8` file.
… from SmallRye Parent
This means mainly renames and adding some javadoc.
…strategies Since `CompletionStage` doesn't support cancellation, we don't use thread blocking operations in `CompletionStage` strategies, and we don't interrupt threads that run `CompletionStage` strategies, it is worthless to try to handle interruptions. We should only check if interruption happened in synchronous and `Future`-based asynchronous strategies, where thread blocking operations are used. Of course interruption may happen for another reason, but we can't do anything about that. This seems to be a bigger deal than it really is, because there was only one remaining `CompletionStage`-based strategy that tried to handle interruptions: fallback. This commit removes that handling and also improves fallback test coverage.
Some tests rely on system time and scheduling to happen with high accuracy. For example, we schedule a timeout to happen after 200 ms, sleep 100 ms and then verify that timeout didn't happen yet. These are bad unit tests for sure, but I certainly don't feel like deleting them. Instead, let's only run them on Linux, where we know they pass. Another option would be to increase those times; instead of 200 ms, we could schedule the timeout to happen after 2 seconds, and sleep 1 second instead of 100 ms. That would significantly increase the likelihood of these tests passing, but it would also make them much slower.
Previously, `AsyncTimeout` used the `InvocationContext` event system to communicate with the `Timeout` strategy. This had 2 flaws in presence of retries: - the set of event handlers grew up with each retry, resulting in unnecessary attempts to complete a `Future` that has already been completed (silly, but harmless); - a delayed (due to thread scheduling) timeout event from one retry iteration could trigger immediate timeout for subsequent retry iteration (big problem!). With this commit, `AsyncTimeout` and `Timeout` use the `InvocationContext` contextual data system and don't suffer from these issues. This is because the `Timeout` class picks up the contextual data item that's used for communicating with `AsyncTimeout` synchronously, so that it's always the one belonging to the "current" retry iteration, and even deletes it immediately after picking it up.
The `CircuitBreakerRollingWindowTest` uses a `@CircuitBreaker` with a delay of 300 ms. It used to sleep 300 ms and then verify that the circuit breaker state changed. It may easily happen that the thread that's supposed to sleep for 300 ms only sleeps for, say, 290 ms, and then obviously the circuit breaker doesn't change state yet. The fix in this commit is simple: increase the sleep time to 400 ms. That should decrease the likelihood of sleeping less than 300 ms to minimum.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.