-
Notifications
You must be signed in to change notification settings - Fork 10
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
fix(connections): use Unis to enable internal connection retry behaviour #244
Conversation
/build_test |
Workflow started at 1/18/2024, 4:20:45 PM. View Actions Run. |
CI build and push: All tests pass ✅ |
1 similar comment
CI build and push: All tests pass ✅ |
3e8ad58
to
2a6dfe9
Compare
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.
lgtm
/build_test |
Workflow started at 1/19/2024, 3:05:31 PM. View Actions Run. |
CI build and push: All tests pass ✅ |
1 similar comment
CI build and push: All tests pass ✅ |
Welcome to Cryostat3! 👋
Before contributing, make sure you have:
main
branch[chore, ci, docs, feat, fix, test]
To recreate commits with GPG signature
git fetch upstream && git rebase --force --gpg-sign upstream/main
Related to #243
Related to #147
Description of the change:
Refactors internal
TargetConnectionManager
implementation to useUni
. The unusedexecuteConnectedTaskAsync
is replaced withexecuteConnectedTaskUni
, which executes on the current thread but in the reactive style.executeConnectedTask
, which is used in many places and is generally how Cryostat performs remote operations on JMX or Agent HTTP targets, is refactored to callexecuteConnectedTaskUni
internally.executeDirect
is also refactored to behave similarly toexecuteConnectedTaskUni
, but with the distinction that operations called through this method do not use the target connection cache, target connection locks, etc., so they are independent of any other operations or of discovery events. BothexecuteDirect
andexecuteConnectedTaskUni
are configured such that certain classes of exceptions trigger retries when caught, where other exceptions are allowed to propagate normally.Motivation for the change:
The "retry-on-certain-exceptions" behaviour allows Cryostat to internally re-attempt remote operations on targets when the operation fails due to certain exceptions which are known to happen sporadically and transiently. This increases Cryostat's reliability from the client's point of view, since rather than the client needing to send a new request to re-attempt after these failures, Cryostat will perform re-attempts on its own until either success or timeout.
How to manually test:
./smoketest.bash -Ogtr
onstart
recording that is already there. I occasionally see sporadic JMX connection failures when trying this before this PR.