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

Channel executor not injected #5544

Closed
wants to merge 6 commits into from
Closed

Channel executor not injected #5544

wants to merge 6 commits into from

Conversation

eaba
Copy link
Contributor

@eaba eaba commented Jan 27, 2022

Fixes #5541

Changes

From var config = system.Settings.Config.GetConfig("akka.channel-scheduler"); to var config = system.Settings.Config.GetConfig("akka.actor.channel-scheduler");

@eaba eaba changed the title https://github.com/akkadotnet/akka.net/issues/5541 Channel executor not injected Jan 27, 2022
var config = @"
akka.actor.default-dispatcher = {
executor = channel-executor
channel-executor.priority = normal
Copy link
Contributor Author

@eaba eaba Jan 27, 2022

Choose a reason for hiding this comment

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

channel-executor not injected when channel-executor.priority is not explicitly set by user @Zetanova @Aaronontheweb

var cfg = config.GetConfig("channel-executor");

Copy link
Contributor

Choose a reason for hiding this comment

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

I made already some improvements to config loading in PR #5515

The channel-scheduler is a standalone component and akka.channel-scheduler is correct,
but I copied the default config block into the wrong place in the default config:

channel-scheduler {
parallelism-min = 4 #same as for ForkJoinDispatcher
parallelism-factor = 1 #same as for ForkJoinDispatcher
parallelism-max = 64 #same as for ForkJoinDispatcher
work-max = 10 #max executed work items in sequence until priority loop
work-interval = 500 #time target of executed work items in ms
work-step = 2 #target work item count in interval / burst
}

Please change the default config in Pigeon.conf and not akka.channel-scheduler

Copy link
Contributor

Choose a reason for hiding this comment

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

This fact needs to be documented properly, it does not reflect what we have right now in the documentation here:
https://getakka.net/articles/actors/dispatchers.html#channelexecutor

@Arkatufus
Copy link
Contributor

Need to add these codes here: https://github.com/akkadotnet/akka.net/blob/dev/src/core/Akka/Configuration/Pigeon.conf#L354

channel-executor {
  priority = normal
}

@Arkatufus
Copy link
Contributor

Also need to document these enum values in the code above: https://github.com/akkadotnet/akka.net/blob/dev/src/core/Akka/Dispatch/ChannelSchedulerExtension.cs#L456-L467

@Aaronontheweb
Copy link
Member

Also need to document these enum values in the code above: https://github.com/akkadotnet/akka.net/blob/dev/src/core/Akka/Dispatch/ChannelSchedulerExtension.cs#L456-L467

The fact that Idle and Background are set for the same value - does that seem like a mistake?

@Zetanova
Copy link
Contributor

Zetanova commented Feb 1, 2022

I put only all names from windows process priority into the enum
They can be used but only have the same effect.

See:
#5543 (comment)

@Arkatufus
Copy link
Contributor

I can't merge this, it still doesn't address the original issue

@eaba
Copy link
Contributor Author

eaba commented Feb 3, 2022

I can't merge this, it still doesn't address the original issue

Yes @Arkatufus it does not. This was like showing what the issue was and luckily @Zetanova already created a PR that addresses that! We can actually close this, right? @Aaronontheweb

@Arkatufus
Copy link
Contributor

No, his PR does not address this issue.

@eaba
Copy link
Contributor Author

eaba commented Feb 4, 2022

at Akka.Dispatch.ChannelExecutorConfigurator..ctor(Config config, IDispatcherPrerequisites prerequisites) in D:\a\1\s\src\core\Akka\Dispatch\AbstractDispatcher.cs:line 124

   at Akka.Dispatch.MessageDispatcherConfigurator.ConfigureExecutor() in D:\a\1\s\src\core\Akka\Dispatch\AbstractDispatcher.cs:line 327

   at Akka.Dispatch.DispatcherConfigurator..ctor(Config config, IDispatcherPrerequisites prerequisites) in D:\a\1\s\src\core\Akka\Dispatch\Dispatchers.cs:line 590

   at Akka.Dispatch.Dispatchers.ConfiguratorFrom(Config cfg) in D:\a\1\s\src\core\Akka\Dispatch\Dispatchers.cs:line 535

   at Akka.Dispatch.Dispatchers.LookupConfigurator(String id) in D:\a\1\s\src\core\Akka\Dispatch\Dispatchers.cs:line 439

   at Akka.Dispatch.Dispatchers..ctor(ActorSystem system, IDispatcherPrerequisites prerequisites, ILoggingAdapter logger) in D:\a\1\s\src\core\Akka\Dispatch\Dispatchers.cs:line 342

   at Akka.Actor.Internal.ActorSystemImpl.ConfigureDispatchers() in D:\a\1\s\src\core\Akka\Actor\Internal\ActorSystemImpl.cs:line 470

   at Akka.Actor.Internal.ActorSystemImpl..ctor(String name, Config config, ActorSystemSetup setup, Nullable`1 guardianProps) in D:\a\1\s\src\core\Akka\Actor\Internal\ActorSystemImpl.cs:line 102

   at Akka.Actor.ActorSystem.CreateAndStartSystem(String name, Config withFallback, ActorSystemSetup setup) in D:\a\1\s\src\core\Akka\Actor\ActorSystem.cs:line 281

   at Akka.Tests.Dispatch.ChannelExecutorSpec.Should_Successfully_Create_Channel_Executor_ActorSystem() in D:\a\1\s\src\core\Akka.Tests\Dispatch\ChannelExecutorSpec.cs:line 19

@Aaronontheweb
Copy link
Member

The real issue here is that the old channel-executor HOCON still needs to work, but right now it doesn't. We need to add a backwards compat spec and make sure the old HOCON can cause the system to load the new code.

@Zetanova
Copy link
Contributor

Zetanova commented Feb 4, 2022

I

The real issue here is that the old channel-executor HOCON still needs to work, but right now it doesn't. We need to add a backwards compat spec and make sure the old HOCON can cause the system to load the new code.

@Aaronontheweb The config section of channel-scheduler was placed in the wrong section akka.actor the Pigeon.conf
it was always intended to be in akka section directly and in every post with a config example it is under akka.channel-scheduler

@Arkatufus
Copy link
Contributor

Superseeded by #5568

@Arkatufus Arkatufus closed this Feb 23, 2022
@eaba eaba deleted the Fix__channel_scheduler_extension branch February 24, 2022 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Non-documented breaking change of channel-executor configuration
4 participants