-
Notifications
You must be signed in to change notification settings - Fork 1.1k
close #2581 - fixed PhiAccrualFailureDetector clock roll over #2583
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
close #2581 - fixed PhiAccrualFailureDetector clock roll over #2583
Conversation
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.
Annotated PR
// handle an edge case when the Clock rolls over | ||
// see https://github.com/akkadotnet/akka.net/issues/2581 | ||
[Fact] | ||
public void PhiAccrualHistory_can_roll_over() |
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.
Reproduction spec
|
||
namespace Akka.Remote.Tests | ||
{ | ||
public class PhiAccrualModelBasedSpecs |
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.
Added model-based test here just to make sure all of the math was being done correctly inside the PhiAccrualFailureDetector
.
/// <returns>TBD</returns> | ||
public static readonly Clock DefaultClock = () => Environment.TickCount & Int32.MaxValue; | ||
/// <returns>A clock instance.</returns> | ||
public static readonly Clock DefaultClock = () => MonotonicClock.GetTicks(); |
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.
Key fix to the issue here. Switched to 64 bit timer instead of the previous 32bit implementation with Environment.TickCount
.
20c69b5
to
01566b9
Compare
var mean = history.Mean; | ||
var stdDeviation = EnsureValidStdDeviation(history.StdDeviation); | ||
return Phi(timeDiff, mean + AcceptableHeartbeatPauseMillis, stdDeviation); | ||
unchecked // in the extremely rare event of a clock roll-over |
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.
Made this unchecked in the extremely rare event of a clock rollover. Realistically, this will never happen.
Deadline failure detector was using MS when it should have been using ticks, hence the failure. Fixed in latest commit. |
No description provided.