benedekh / WeLoveClouds

Repository for submissions on the CloudDB course.
0 stars 1 forks source link

Merge branch new commands #179

Closed ghost closed 7 years ago

ghost commented 7 years ago

I'm unsure of how to handle quitting properly, should I attempt to shut down the service and then quit or just quit straight away?

benedekh commented 7 years ago

tl;dr: a 'simple but brutal' System.exit(0) would be fine.

Good question, I had the same issue with the KVServer. I'll have a look how we handle the graceful quit (e.g. closing the ongoing connections, closing the ServerSocket also).

longer explanation:

Ah it is not properly handled on the KVServer either. It should be something like this:

https://github.com/benedekh/WeLoveClouds/blob/master/Replicated_Storage_Service/src/weloveclouds/server/requests/kvecs/ShutdownServer.java

Basically the trick is: weloveclouds.commons.networking.IConnectionHandler extends ICallbackRegister which can register a callback (Runnable). SimpleConnectionHandler implements that interface, and in its run() method's finally section, it runs every time callbacks if there is anyone registered there. So as soon as the respective SimpleConnectionHandler handles that request which initiated the ServerShutdown call, shuts down the server. Since every Socket and Connection is encapsulated in a try-with-resources block their close method will be called automatically.

Aaand that's why it is not handled this way on the KVServer's quit client command. So the simple System.exit(0) will do the trick. -> And now I know why it was necessarry to implement the ICallback. :-) It is due to the fact that ECS expects for a response (SUCCESS) from the KVServer to make sure it received the shutdown request. So that response has to be sent back before the server would shut down. And that's why we need that ICallbackRegister. :-)

ghost commented 7 years ago

Nice explanation, thanks!

benedekh commented 7 years ago

@RMIT-s3529366-Hunton-Bowland Pro hint: CTRL+SHIFT+F auto-formats the respective file that is opened in the editor, according to the Formatting rules.

kep kep

ghost commented 7 years ago

I'm unsure of why that first check is failing, the build works on my machine. Is there some mechanism preventing the ECS from being built with client utils?

benedekh commented 7 years ago

@RMIT-s3529366-Hunton-Bowland I don't know what the issue was, but I made some changes (which Benoit requested) and see if the Travis build works now. Closes #122 as soon as it is merged.

ghost commented 7 years ago

No problem, once Benoit approves the changes I've made in line with his suggestions I'll merge.

banctilrobitaille commented 7 years ago

Looks good, :ship: it