[
https://issues.apache.org/jira/browse/MAPREDUCE-3535?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13167708#comment-13167708
]
Steve Loughran commented on MAPREDUCE-3535:
-------------------------------------------
Having a look at what I'd done w.r.t service lifecycle
[http://svn.eu.apache.org/viewvc/hadoop/common/branches/HADOOP-3628-2/src/core/org/apache/hadoop/util/Service.java?view=markup]
I avoided this by having the base class do all the checks in final methods and
have overridable {{innerStart()}}, {{innerStop()}} etc. methods that subclasses
would use to perform their custom operations, along with a static class to
actually walk a service into its started state.
There is no reason why the {{AbstractService}} class cannot use the same
technique, without changing the {{Service}} interface. It would declare it's
init/start/stop methods final, do the state change, then delegate to the
{{protected}} methods {{innerInit()}} {{innerStart()}}, {{innerStop()}}. These
would not be externally visible, and not get invoked until the validity of the
operation had been tested.
Effort:
# time to rework the older service lifecycle methods into the new framework, 1
h
# time to go through all the subclasses and rename their init/start/stop
methods to be the inner ones.
We could use a better prefix than "inner" if anyone can think of it.
> Yarn service subclasses don't check for service state before executing their
> state change operations
> ----------------------------------------------------------------------------------------------------
>
> Key: MAPREDUCE-3535
> URL: https://issues.apache.org/jira/browse/MAPREDUCE-3535
> Project: Hadoop Map/Reduce
> Issue Type: Bug
> Components: mrv2
> Affects Versions: 0.23.0, 0.24.0
> Reporter: Steve Loughran
>
> Although there are checks in the lifecycle state change methods (start, stop,
> ...), they only get checked in the superclass. The subclasses don't check it;
> they don't exit the start() method if they are already started, and they
> don't bail out early on a stop if they are already stopped -even when the
> superclass returns without doing anything.
> This means that calling {{Service.start()}} twice may leak resources,
> {{Service.start()}} twice try to shut down twice, etc. And that's on the same
> thread...
> My preferred action here would be for the ave the operations return true if a
> state change took place, the implementation would be synchronised and return
> the correct value
> The subclasses look for this and only execute their state changes took place
> e.g
> {code}
> boolean start() {
> if (!super.start()) return false;
> //do my own startup
> return true;
> }
> {code}
> The {{Service.stop()}} operation is trickier as the subclasses tend to call
> the superclass last for a better unwinding. I think it may be safer to work
> the other way around, but code reviews would be need to ensure that this
> doesn't break assumptions in the subclass about termination order. It may be
> possible to do more complex designs, but it is hard to chain this down a
> hierarchy of classes.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
