-
Notifications
You must be signed in to change notification settings - Fork 295
Stopping a task with plugin initiated workflow should not panic #1588
Stopping a task with plugin initiated workflow should not panic #1588
Conversation
@IzabellaRaulin, do you mind open an issue with the steps to reproduce this issue? |
@candysmurf, I created #1589 - there you can find step by step how to reproduce that issue |
control/available_plugin.go
Outdated
@@ -236,7 +236,7 @@ func (a *availablePlugin) Stop(r string) error { | |||
return a.client.Kill(r) | |||
} | |||
|
|||
// Kill assumes aplugin is not able to here a Kill RPC call | |||
// Kill assumes a plugin is not able to here a Kill RPC call |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/here/hear/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch - I changed it to "hear"
t.state = core.TaskStopped | ||
break | ||
t.Unlock() | ||
done = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After removing g.killed() in control/plugin/client/grpc.go, where are we closing the channel to be able to receive "closing of killChan" here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works exactly in the same way as for regular spin() - so it happens here: https://github.com/intelsdi-x/snap/blob/master/scheduler/task.go#L360
ed6ab41
to
e1c8da6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
LGTM |
Thank You |
Fixes #1589
Before:
Stopping a task with streaming schedule caused a panic:
After
Stopping the task behaves as expected:
Summary of changes:
done
to true after receiving that killChan has been Closedg.Killed()
from control/plugin/client/grpc.go because:Testing done:
@intelsdi-x/snap-maintainers