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

ActorTaskScheduler QueueTask for LongRunning #1410

Closed
Zetanova opened this issue Nov 8, 2015 · 8 comments
Closed

ActorTaskScheduler QueueTask for LongRunning #1410

Zetanova opened this issue Nov 8, 2015 · 8 comments

Comments

@Zetanova
Copy link
Contributor

Zetanova commented Nov 8, 2015

Some external Sources are lazy initializing background threads with the LongRunning Task hint.
Like with RX Subject on the first OnNext()
With the current ActorTaskScheduler behavior the actor will lock at Line64 TryExecuteTask(task);
The deault behavir of the TaskPoolScheduler would be to start a detached explicit Thread for this Task.

@Zetanova
Copy link
Contributor Author

Zetanova commented Nov 9, 2015

The Cancled Check is missing at line65
if (task.IsFaulted || task.IsCanceled)
Rethrow(task, null);

@Danthar
Copy link
Member

Danthar commented Nov 12, 2015

Can you confirm this bug with a Test/Spec ?
After which, you are most welcome to do a PR for this :)

@Zetanova
Copy link
Contributor Author

I created a Spec, but of couse it is failing.

There are multiple possible options for the ActorTaskScheduler

  1. It creates only a detetched Thread on TaskCreationOptions.LongRunning
  2. It uses the CompleteTask-Message only for a TaskCreationOptions.AttachedToParent

@Aaronontheweb
Copy link
Member

@Zetanova so your spec proves definitively that this is an issue.

What's the appropriate fix?

@Danthar
Copy link
Member

Danthar commented Nov 20, 2015

@Zetanova your right about the check on line 65. That should include the IsCanceled check.

However i'm not sure what you mean with the possible options for the ActorTaskScheduler. As in, how would that solve the problem?

@Zetanova
Copy link
Contributor Author

Currently the ActorTaskScheduler is executing all task-types with the help of the CompleteTask-Message.
Some Libraries can have hidden side-effects that they are requesting a task internaly
for a long running process.
Like System.Reactive on the first call of OnNext() method.

if currently something is requesting a LongRunning Task, it will probably lock the actor with it.

If something requires a Long Running Task for lets say an infinit message loop
it normaly is requesting the Task with the TaskCreationOptions.LongRunning flag
and correctly assume that it can run this task for a long period.
A separate and new created thread is used for this task by the TaskPoolScheduler.

I didn't thought it through but maybe the ActorTaskScheduler can behave like the normal TaskPoolScheduler for all Task-types beside TaskCreationOptions.AttachedToParent
and only using the CompleteTask-Message on tasks that are attached as childs
with the AttachedToParent flag set.

The ActorTaskScheduler schould allways create a new thread for the a LongRunning -Task
and yes, it is running then outside of the akka context.

@Aaronontheweb
Copy link
Member

@Zetanova got it - so this is possibly related to #1346 (see PR #1347) and also the use of TaskCreationOptions.LongRunning. Could you modify your PR to include a change for this on ActorTaskScheduler?

Aaronontheweb added a commit to Aaronontheweb/akka.net that referenced this issue Nov 26, 2015
@JeffCyr
Copy link
Contributor

JeffCyr commented Nov 29, 2015

I'm a bit confused by the comments and commits in this issue. ActorTaskScheduler's purpose is to execute a task in the Actor's context, it make sense to spin a thread when the flag LongRunning is set, but it does not make sense to execute a task outside the actor context ever.

See Issue #1481 and PR #1482, I implemented the LongRunning flag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants