Plugin clean-up in case of task stop

[andrzej-k]

Let's assume that collector plugin retrieves metrics from the database, and in order to do that it needs to establish connection to the database. Instead of opening and closing DB connection within CollectMetrics() during each interval, it opens connection to DB once and then re-uses it.
Now user decides to stop task which is using aforementioned plugin, this will result in DB connection not being closed.
Maybe we need a method on a plugin side to do clean-up in such cases?

[jcooklin]

@andrzej-k: I think this is achieved today with the sticky routing/caching strategy #539 #670. Would you agree?

[andrzej-k]

Well, to me it seems that sticky routing helps greatly, since plugin instance (for example: the one with opened DB connection) will be used consistently. But still in case of stopping the task, the plugin will just receive SIGKILL, and won't be able to do any actions based on this (like closing DB connection). One solution to this, proposed by @danielscottt, was to change SIGKILL to other, catch-able signal and then handle that signal on the plugin side. The other solution could be to extend client-side Plugin interface (implemented by plugins) to include also Kill() or Cleanup() func (in addition to GetConfigPolicy()) which could be called by framework once plugin is to be killed - but that would probably require re-work of all existing plugins.

[iwankgb]

We are facing issues with SIGKILL while working with one of our projects:

1. Collector plugin launches long running process in that is needed to return metrics.
2. Plugin process is killed with SIGKILL
3. Child process mentioned in step 1 becomes a zombie.

As of now we are going to use systemd to launch snapd as it will kill all the zombies. It would be great if we could handle termination signal gracefully and plugin could take care of its children processes/opened sockets/other resources on its own.

Contact me if you want to get more detailed description.

[candysmurf]

@andrzej-k, GRPC lib-go provides such capability to kill/ping between servers(plugins) and the Snap GRPC client. Probably we can think along this line to connecting task killing,... @IRCody is working on a related bug fix currently. I think all these interactions will have to consider the implication @andrzej-k mentioned.

[iwankgb]

@candysmurf - I am aware of implications that @andrzej-k mentioned and I do not expect collector interface to be extended before 1.0. Would there be a chance to sent another signal, prior to SIGKILL to indicate that plugin is to be shut down in lets say 5 seconds?
It could work as a temporary fix until more robust version is developed.

[candysmurf]

@iwankgb, a good question. Cody just added this PR (#1192) for enforcing plugin timeout. Will this help? Please share your thoughts.

[iwankgb]

@candysmurf - #1192 won't help, I'm afraid. It seems to affect collecting, processing and publishing but not actual plugin killing. As far as I understand logic in ExecutablePlugin.Kill() would need to be altered.

[candysmurf]

@iwankgb, if PR (#1192) won't help, please state your problem and your suggestion is very welcome. If it's a separate issue, feel free to open an issue. thank you.

[iwankgb]

@candysmurf - isn't this comment clear enough? #711 (comment)

[candysmurf]

@iwankgb, As it mixed with @andrzej-k's, I think they're not exact the same. Do you like to pull your comment into a new issue?