healthmgr: aurora heron shell controller #2283
Conversation
| LOG.severe(e.getMessage()); | ||
| } catch (IllegalAccessException e) { | ||
| LOG.severe(e.getMessage()); | ||
| } |
There was a problem hiding this comment.
Need we throw an exception if we could not initialize stateMgr and stateMgrAdaptor? Otherwise, the AuroraHeronShellController is in a wrong state.
There was a problem hiding this comment.
Eventually any exception has to be caught before AuroraScheduler.initialize(). If the exception is not caught here, where shall we catch them?
There was a problem hiding this comment.
Bubble up a RuntimeException to external logic. Otherwise, the program will execute with a bad state but it is no way executing normally and recovering with such a bad state.
There was a problem hiding this comment.
done: throw to external logic
| } | ||
|
|
||
| StMgr contaienrInfo = stateMgrAdaptor.getPhysicalPlan(topologyName).getStmgrs(containerId); | ||
| String ip = contaienrInfo.getHostName(); |
There was a problem hiding this comment.
call the variable host or hostname. ip is confusing here.
|
|
||
| HttpURLConnection con = NetworkUtils.getHttpConnection(url); | ||
| NetworkUtils.sendHttpPostRequest(con, "X", payload.getBytes()); | ||
| boolean ret = NetworkUtils.checkHttpResponseCode(con, 200); |
There was a problem hiding this comment.
Have a try-catch-finally block and put con.disconnect() in finally
| Context.environ(localConfig), | ||
| AuroraContext.getHeronAuroraPath(localConfig), | ||
| Context.verbose(localConfig)); | ||
| switch (klass) { |
There was a problem hiding this comment.
This implementation here is really ugly and bad.
One alternative is to use constant enum;
another alternative is to use a boolean to decide which one to use since you have only two cases and it will fail otherwise.
Polymorphism can be another one.
| @@ -71,6 +71,8 @@ | |||
| PACKING_CLASS ("heron.class.packing.algorithm", Type.STRING), | |||
| REPACKING_CLASS ("heron.class.repacking.algorithm", Type.STRING), | |||
| STATE_MANAGER_CLASS ("heron.class.state.manager", Type.STRING), | |||
| AURORA_CONTROLLER_CLASS ("heron.class.scheduler.aurora.controller", | |||
There was a problem hiding this comment.
given you have only 2 options, you can consider this is a boolean. You could evolve from it in future if needed, but I seriously doubt that.
|
👍 LGTM |
|
thanks @maosongfu |
* healthmgr add aurora heron-shell controller * fix style * wrapper class * address maosong comment * address maosong comment: throw excetion to external logic * update example yaml * fix style
follow #2230:
The healthMgr/Dhalion resolver (RestartContainerResolver) calls Aurora scheduler to restart container. This PR adds a aurora scheduler which does not depend on Aurora CLI and which can be called inside Aurora container.