-
Notifications
You must be signed in to change notification settings - Fork 306
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
FISH-659: Fault Tolerance 3.0 full pass #5066
Merged
Merged
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
Managed executor retain invocation context upon submission, fixing the problem of interceptor getting fresh state because invocation manager returned wrong application name
Resulted in "No free work or queue space" when CAS would fail just 5 times. Fix NPE in FaultToleranceInvocation.toString.
Determining app name and composing String methodId unnecessarily slows down. App name is determined once per method context now. FT data are now deleted upon undeployment by matching classes' classloaders against undeployed app's classloader. Expiration of the cache was removed, removing need for target object and expiration checks. FaultToleranceServiceStub introduces StubContext as encapsulation of MethodKey which would otherwise need to be public.
jbee
approved these changes
Dec 22, 2020
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a pair of fresh eyes is what that needed.
Good work @pdudits !
...src/main/java/fish/payara/microprofile/faulttolerance/service/FaultToleranceServiceImpl.java
Show resolved
Hide resolved
...fault-tolerance/src/main/java/fish/payara/microprofile/faulttolerance/service/MethodKey.java
Show resolved
Hide resolved
MattGill98
approved these changes
Dec 22, 2020
jenkins test please |
1 similar comment
jenkins test please |
Looking them up via logical name triggers the activator which would create them in domain config if they are not materialized in domain.xml yet
jenkins test please |
JamesHillyard
pushed a commit
to JamesHillyard/Payara
that referenced
this pull request
Sep 17, 2021
FISH-659: Fault Tolerance 3.0 full pass
JamesHillyard
pushed a commit
to JamesHillyard/Payara
that referenced
this pull request
Sep 17, 2021
FISH-659: Fault Tolerance 3.0 full pass
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.
Description
Small refactorings in Fault Tolerance implementation to make all TCK test pass.
The core change was to return back to standard managed thread pools. These pools propagate application context correctly so the issue with having incorrect application name on invocation stack no longer applies.
However the need for application name was greatly reduced in next optimization, where actual class and method reference are used as keys into state map, further stabilizing the tests.
Also discovered and fixed is memory leak in
FaultTolerancePolicy
's cache where entires were not deleted upon undeployment, which was troublesome especially for run of TCK. With these two changes together in became apparent that cleanup of method data can be removed, as data has relatively small footprint and keeping them removes confusion on what state of the system is.Important Info
Testing
New tests
Full FT 3.0 TCK passes:
payara/MicroProfile-TCK-Runners#133
Testing Environment
Documentation
Documentation will require update, as all threadpool and cleanup related parameters are not applied anymore.
Notes for Reviewers
/cc @jbee, if you have bit of time, check if I didn't misunderstood any of the original intentions of the code.