Skip to content

Conversation

Aaronontheweb
Copy link
Member

@Aaronontheweb Aaronontheweb commented Dec 19, 2024

Changes

harden CircuitBreakerSpecs.Must_increment_failure_count_on_callTimeout_before_call_finishes

Checklist

For significant changes, please ensure that the following have been completed (delete if not relevant):

harden `CircuitBreakerSpecs.Must_increment_failure_count_on_callTimeout_before_call_finishes`
// meant to run as detached task
Task.Run(() => breaker.Instance.WithSyncCircuitBreaker(() => Thread.Sleep(Dilated(TimeSpan.FromSeconds(1)))));
var t = Task.Run(() => breaker.Instance.WithSyncCircuitBreaker(() => Thread.Sleep(Dilated(TimeSpan.FromSeconds(1)))));
await AwaitConditionAsync(() => t.Status >= TaskStatus.Running); // need to kick off the task before we can check the latch
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This spec is pretty racy due to non-determinism of the TaskScheduler - now we force our assertions to wait until the Task.Run actually gets scheduled for execution, because otherwise the breaker never gets a chance to be scheduled and this will throw the timing off.

_ = breaker.Instance.WithCircuitBreaker(() => Task.Run(SayHi));
Enumerable.Range(1, 4).ForEach(_ => breaker.Instance.WithCircuitBreaker(() => Task.Run(ThrowException)));
await WaitForTaskToBeScheduled(breaker.Instance.WithCircuitBreaker(() => Task.Run(SayHi)));
await WaitForTaskToBeScheduled(Enumerable.Range(1, 4).Select(_ => breaker.Instance.WithCircuitBreaker(() => Task.Run(ThrowException))).ToList());
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issues we've been running into this spec are due to scheduler lag again - when the CPU count is low these tasks may not get scheduled to run immediately.

@Aaronontheweb
Copy link
Member Author

These circuit breaker specs are kind of sketchy in the first place IMHO, as many of them rely on imprecise system clock behavior and the non-deterministic scheduling of the TaskScheduler. Doing my best to clean this up without just disabling them.

@Aaronontheweb Aaronontheweb enabled auto-merge (squash) December 19, 2024 13:32
@Aaronontheweb
Copy link
Member Author

This has been impacted by #7419 a couple of times; we can debug that separately

@Aaronontheweb Aaronontheweb merged commit 2a82323 into akkadotnet:dev Dec 19, 2024
10 of 12 checks passed
@Aaronontheweb Aaronontheweb deleted the harden-Akka.Tests-specs branch December 19, 2024 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant