hadoop-mapreduce-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Steve Loughran (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (MAPREDUCE-3535) Yarn service subclasses don't check for service state before executing their state change operations
Date Mon, 12 Dec 2011 19:41:31 GMT

    [ 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

        

Mime
View raw message