Tecsisa / constructr-consul

This library enables to use Consul as cluster coordinator in a ConstructR based node
Apache License 2.0
32 stars 15 forks source link

Session is NOT explicitly destroyed during clean shutdown #49

Open pocman opened 7 years ago

pocman commented 7 years ago

Hi,

My issue is that when running nomad stop, the sessions associated with each tasks are not destroyed and I must wait for the ttl to expires.

In consul documentation, it's indicated that a session will be invalidated:

Node deregistering doesn't solve my issue since the node is still running after the nomad stop, we can't add healthcheck in the conf and session is not destroyed on clean shutdown.

franbh commented 7 years ago

Hi, thanks for your issue. You are right, we should destroy session on system shutdown. I'm going to open an issue at https://github.com/hseeberger/constructr, suggesting the addition of a close() method to Coordination. This would allow us to release Consul-related resources in an elegant way by relying on the postStop() handler of ConstructrMachine. Also, I'm going to close #50 because of the above reasoning. Besides that, it seems that we should use a different HTTP client to communicate with Consul, due to akka-http provoking the java.lang.IllegalStateException: cannot create children while terminating or terminated exception.

pocman commented 7 years ago

Why not use registerOnTermination instead of postStop ?

franbh commented 7 years ago

That would be a quick solution (provided that we switch to a different HTTP client), but I think that Constructr core would benefit of having a clear way of releasing such resources.