Skip to content
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 #6295 - set default PoolRouter SupervisorStrategy to Restart #6366

Merged
merged 4 commits into from
Jan 26, 2023

Conversation

Aaronontheweb
Copy link
Member

Changes

Fixes #6295 - this fixes a very old bug in Akka.NET where any time a routee on a Pool router restarted, the default supervision directive on the router was set to Escalate which would cause the router itself to restart and kill all of the routees in the process. I believe this was done as a result of misunderstanding what Escalate does and how it works. We now know better and this sets the supervision directive to Restart, which is the safe default we use for everything else.

Checklist

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

@Aaronontheweb Aaronontheweb added akka-actor akka.net v1.4 Issues affecting Akka.NET v1.4 labels Jan 26, 2023
Copy link
Contributor

@Arkatufus Arkatufus left a comment

Choose a reason for hiding this comment

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

LGTM

@Arkatufus Arkatufus enabled auto-merge (squash) January 26, 2023 16:23
@Aaronontheweb
Copy link
Member Author

This will also need to be ported to v1.5 @Arkatufus

@Aaronontheweb
Copy link
Member Author

Akka.Tests.Routing.RoutingSpec.Routers_in_general_must_default_to_all_for_one_always_escalate_strategy - doh, missed that. I'm going to need to fix that test before this can be merged in.

@Arkatufus
Copy link
Contributor

OK

@Arkatufus
Copy link
Contributor

@Aaronontheweb fixed the unit test, can you check the code?


supervisor.Tell(new RoundRobinPool(3).Props(Props.Create(() => new RestartActor(TestActor))));
var router = Sys.ActorOf(new RoundRobinGroup(routes.Select(a => a.Path.ToString())).Props());
Copy link
Member Author

Choose a reason for hiding this comment

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

@Arkatufus this needs to be a RoundRobinPool router

@Aaronontheweb Aaronontheweb merged commit ffd9a9e into v1.4 Jan 26, 2023
@Aaronontheweb Aaronontheweb deleted the fix-6295-router-strategy branch January 26, 2023 23:11
Arkatufus pushed a commit to Arkatufus/akka.net that referenced this pull request Jan 26, 2023
… Restart (akkadotnet#6366)

* close akkadotnet#6295 - set default PoolRouter SupervisorStrategy to Restart
* use the `SupervisionStrategy.DefaultStrategy`
* Fix unit test
* Change unit test to use RoundRobinPool router

(cherry-picked from ffd9a9e)
Aaronontheweb added a commit that referenced this pull request Jan 30, 2023
)

* close #6295 - set default PoolRouter SupervisorStrategy to Restart (#6366)

* close #6295 - set default PoolRouter SupervisorStrategy to Restart
* use the `SupervisionStrategy.DefaultStrategy`
* Fix unit test
* Change unit test to use RoundRobinPool router

(cherry-picked from ffd9a9e)

* Fix actor telemetry unit test

---------

Co-authored-by: Aaron Stannard <aaron@petabridge.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
akka.net v1.4 Issues affecting Akka.NET v1.4 akka-actor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants