implement MicroProfile Fault Tolerance 3.0 #264
Conversation
Previously, we had the general metrics (invoked/failed) collected directly in the code that invokes the `CompletionStage`-returning method. This is now extraced to a `CompletionStageGeneralMetricsRecorder`, similarly to synchronous and `Future`-returning methods that use `GeneralMetricsRecorder`.
|
We'll have to bump a minor or even a major version. At this point, I'm not sure which it should be. Any opinions @michalszynkiewicz @rhusar @radcortez @kenfinnigan? |
|
My personal opinion: from SmallRye Fault Tolerance perspective, the only breaking change we do is metrics. This doesn't sound like 5.0 material to me, so I'd probably lean towards making it 4.4, but I could easily be persuaded otherwise. |
| @@ -4,55 +4,16 @@ | |||
|
|
|||
| import io.smallrye.faulttolerance.core.FaultToleranceStrategy; | |||
|
|
|||
| /** | |||
| * @author Michal Szynkiewicz, michal.l.szynkiewicz@gmail.com | |||
There was a problem hiding this comment.
I tend to do this when I do larger modifications to the classes, such that I'd have to add another author tag for myself. I find them an unnecessary clutter that convey exactly zero information. Your authorship is carefully preserved in Git history :-)
| import io.smallrye.faulttolerance.core.InvocationContextEvent; | ||
|
|
||
| public class FallbackEvents { | ||
| public static class Defined implements InvocationContextEvent { |
There was a problem hiding this comment.
why not enum values? Sth like:
public enum FallbackEvents implements InvocationContextEvent {
DEFINED,
APPLIED
}
| import io.smallrye.faulttolerance.core.InvocationContextEvent; | ||
|
|
||
| public class GeneralMetricsEvents { | ||
| public static class ExecutionFinished implements InvocationContextEvent { |
There was a problem hiding this comment.
again, seems to me that an enum would be a better fit
There was a problem hiding this comment.
Enum should actually work well here, I'll check.
|
👍 from me to bump the minor |
|
Good point about enums, for some reason I didn't consider them, I don't know why. I'll try :-) |
|
Ah, I know why -- the event system is very primitive, it keys events by |
|
Actually thanks for suggesting enums, I can use the enum-singleton pattern to make the code much shorter. Even though I can't really use public enum FallbackEvents {
DEFINED,
APPLIED;
}I can at least use public class FallbackEvents {
public enum Defined implements InvocationContextEvent {
INSTANCE;
}
public enum Applied implements InvocationContextEvent {
INSTANCE;
}
}which is much better than the original. |
…Tolerance 3.0 The metrics system is now completely based on the event system in `InvocationContext`. All the fault tolerance strategies emit events on interesting places in the code, and there's a dedicated fault tolerance strategy that collects metrics based on those events. This significantly improves source locality, but has some runtime cost. This should be pretty negligible though. One interesting optimization is present: if metrics for fault tolerance are disabled, the dedicated fault tolerance strategy is never added to the chain, so in this case, the cost of metrics is virtually zero.
|
Force-pushed because I amended one of the commits with the enum-singleton stuff. |
|
I have manually reran the jobs and they finished successfully. I have no idea why they still show as "waitin for status" here in the PR. |
|
Strange. For some reason it seems that the GH Actions didn't picked the new commit hash. Might be a bug. If everything is ok, just squash it in master that they should run. |
|
I've created a |
There are 2 big changes in MicroProfile Fault Tolerance 3.0:
Resolves #185.