Make loggers to not block ActorSystem creation #4424
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Made async loggers startup to be configurable, and disabled by default, because probably this leads to CI suite to block without threads available. Lots of Basically with new setting disabled it should be the same as earlier. CI should pass. If so, I will add few specs to verify that when some of the systems are running loggers in async way, this is fine. |
|
So, now we have logger creation task with async support. When When setting is disabled, it is performing async/await call with blocking |
| /// Gets the logger start timeout. | ||
| /// </summary> | ||
| /// <value>The logger start timeout.</value> | ||
| public bool LoggerAsyncStart { get; private set; } |
There was a problem hiding this comment.
Can you try running it with this setting enabled to true everywhere? Looks like the test suite is running without it
There was a problem hiding this comment.
Yes - want to check if test suite will work well. Because it seems to me that it is blocking even without async version.
And before introducing this setting, the code was using "async" implementation by default (so I added this setting to be able to disable new behavior) - and test suite kind of stopped.
Probably I need to wait longer. Just checking out CI output and it does not change for some time
|
Ok, looks like it works (does not hang) when async run setting is disabled. I will enable it now. |
|
Ok, works well - only need to add api approvement |
|
Note: will need to disable new setting by default before merging this. |
|
And will need to add spec showing that without setting and with not-working logger ActorService is handing on Create call, but with setting enabled system is created without issues |
|
Finally found failing spec: LoggingBus_should_stop_all_loggers_on_termination |
|
Fixed it. Now should pass - at least under Linux |
|
Failed specs: .NET Framework: It might be racy. Will try to fix it. .NET Core (Windows): .NET Core Linux: Looks like all this specs are racy and are not related to the PR. But I will try making fixes for them, or at least to improve them |
|
@IgorFedchenko have a merge conflict here |
|
@Aaronontheweb Yeah, @Arkatufus did great job on skipping specs, some of them where already skipped in my code. |
|
Looks like a real test failure: Akka.DistributedData.Tests.ReplicatorSpecs.Bugfix_4198_PNCounterDictionary_Merge
System.Collections.Generic.KeyNotFoundException : The given key was not present in the dictionary. |
|
@Aaronontheweb Not sure about is it real - passing locally, and failing in this code: Within(TimeSpan.FromSeconds(5), () =>
{
AwaitAssert(() =>
{
foreach (var (probe, replicator, _) in probes)
{
replicator.Tell(Dsl.Get(_keyC, new ReadAll(_timeOut)), probe);
// Failure is here:
probe.ExpectMsg<GetSuccess>().Get(_keyC).Entries["x"].ShouldBe(new BigInteger(30.0d));
}
});
});So seems like this is due to some increased delays. |
|
Looks like some tests were hanging in Akka.Streams because I tried to make them non-racy. Probably having some deadlocks there during threads starvation. |
|
Works well now. Good. |
This was referenced May 26, 2020
ActorSystem.Create() blocking indefinitely. [Current status of Akka.NET .NET Standard Support]
#4054
Closed
This was referenced May 26, 2020
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Continuation of #4422 , trying to make loggers to start without blocking and not blow up the test suite