[jira] [Commented] (YARN-804) mark AbstractService init/start/stop methods as final
[
https://issues.apache.org/jira/browse/YARN-804?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13684173#comment-13684173
]
Steve Loughran commented on YARN-804:
-
# code calls init() or start() on a composite service (or subclass)
# {{CompositeService}} inits all of its children in {{serviceInit()}}, starts
in {{serviceStart()}}
# in {{serviceStop()}} it does a best-effort teardown of all services in the
states INITED or higher (but not NOTINITED) -in reverse order. This policy
provides the teardown experience needed by the mapreduce code.
# if init or startup fails, then {{CompositeService}} will throw the exception
straight out of the serviceInit/Start method, leaving {{AbstractService}} to
handle it by calling {{AbstractService.stop}}, hence to
{{CompositeService.stop()}} or that of the subclass.
As before, subclasses can be clever about when they propagate the
serviceStart() operation up to the CompositeService, allowing them to create
and init services, add them as new services to manage -then call
{{super.serviceStart()}} to have the Composite service manage the rest of their
lifecycle -both start and stop.
If there is one optional change that subclasses of composite services don't
need to do, it is catch any exceptions in the init/start code, then wrap them
in RuntimeException to get them past the {{Service}} lifecycle method
signatures. Exceptions can be left to be thrown up from the {{serviceInit()}}
and {{serviceStart()}} methods, where they are caught and (if need be) wrapped
by RuntimeExceptions and rethrown after the {{Service.stop()}} operation is
invoked. I didn't go through all the code and remove that catch and rethrow, as
would have increased the size of the patch for little real benefit. A few of
the tests did fail from some of the exception strings changing (by the
AbstractService-level wrapping) -I had to change their assertions from
{{exception.getMessage().startsWith(some text)}} to
{{exception.getMessage().contains(some text)}}.
There's one other thing that we catch up on is if, during an {{serviceInit()}}
operation, the instance of {{Configuration}} passed down is changed. That's
logged at debug level for the curious, people trying to track down why an
attempt to use a shared config instance between peer services in a
{{CompositeService}} isn't working for one of the services in the set.
Sometimes it happens, primarily when a service wants to convert it to some
{{YarnConfig}}, {{JobConfig}} etc, purely for the role-specific helper methods.
mark AbstractService init/start/stop methods as final
-
Key: YARN-804
URL: https://issues.apache.org/jira/browse/YARN-804
Project: Hadoop YARN
Issue Type: Sub-task
Components: api
Affects Versions: 2.1.0-beta
Reporter: Steve Loughran
Assignee: Vinod Kumar Vavilapalli
Attachments: YARN-804-001.patch
Now that YARN-117 and MAPREDUCE-5298 are checked in, we can mark the public
AbstractService init/start/stop methods as final.
Why? It puts the lifecycle check and error handling around the subclass code,
ensuring no lifecycle method gets called in the wrong state or gets called
more than once.When a {{serviceInit(), serviceStart() serviceStop()}}
method throws an exception, it's caught and auto-triggers stop.
Marking the methods as final forces service implementations to move to the
stricter lifecycle. It has one side effect: some of the mocking tests play up
-I'll need some assistance here
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (YARN-804) mark AbstractService init/start/stop methods as final
[
https://issues.apache.org/jira/browse/YARN-804?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13683584#comment-13683584
]
Siddharth Seth commented on YARN-804:
-
Are the same semantics as AbstractService enforced for the CompositeService as
well ? Are users expected to call super.init / super.serviceInit to take care
of all the Services which are part of the composite service, or will
CompositeService just take of this ? Otherwise, it may make sense to re-open
YARN-811.
mark AbstractService init/start/stop methods as final
-
Key: YARN-804
URL: https://issues.apache.org/jira/browse/YARN-804
Project: Hadoop YARN
Issue Type: Sub-task
Components: api
Affects Versions: 2.1.0-beta
Reporter: Steve Loughran
Assignee: Vinod Kumar Vavilapalli
Attachments: YARN-804-001.patch
Now that YARN-117 and MAPREDUCE-5298 are checked in, we can mark the public
AbstractService init/start/stop methods as final.
Why? It puts the lifecycle check and error handling around the subclass code,
ensuring no lifecycle method gets called in the wrong state or gets called
more than once.When a {{serviceInit(), serviceStart() serviceStop()}}
method throws an exception, it's caught and auto-triggers stop.
Marking the methods as final forces service implementations to move to the
stricter lifecycle. It has one side effect: some of the mocking tests play up
-I'll need some assistance here
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (YARN-804) mark AbstractService init/start/stop methods as final
[
https://issues.apache.org/jira/browse/YARN-804?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13682699#comment-13682699
]
Hadoop QA commented on YARN-804:
{color:red}-1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12587689/YARN-804-001.patch
against trunk revision .
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:green}+1 tests included{color}. The patch appears to include 1 new
or modified test files.
{color:green}+1 javac{color}. The applied patch does not increase the
total number of javac compiler warnings.
{color:green}+1 javadoc{color}. The javadoc tool did not generate any
warning messages.
{color:green}+1 eclipse:eclipse{color}. The patch built with
eclipse:eclipse.
{color:green}+1 findbugs{color}. The patch does not introduce any new
Findbugs (version 1.3.9) warnings.
{color:green}+1 release audit{color}. The applied patch does not increase
the total number of release audit warnings.
{color:red}-1 core tests{color}. The patch failed these unit tests in
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common:
org.apache.hadoop.yarn.client.TestAMRMClientAsync
{color:green}+1 contrib tests{color}. The patch passed contrib unit tests.
Test results:
https://builds.apache.org/job/PreCommit-YARN-Build/1221//testReport/
Console output: https://builds.apache.org/job/PreCommit-YARN-Build/1221//console
This message is automatically generated.
mark AbstractService init/start/stop methods as final
-
Key: YARN-804
URL: https://issues.apache.org/jira/browse/YARN-804
Project: Hadoop YARN
Issue Type: Sub-task
Components: api
Affects Versions: 2.1.0-beta
Reporter: Steve Loughran
Assignee: Vinod Kumar Vavilapalli
Attachments: YARN-804-001.patch
Now that YARN-117 and MAPREDUCE-5298 are checked in, we can mark the public
AbstractService init/start/stop methods as final.
Why? It puts the lifecycle check and error handling around the subclass code,
ensuring no lifecycle method gets called in the wrong state or gets called
more than once.When a {{serviceInit(), serviceStart() serviceStop()}}
method throws an exception, it's caught and auto-triggers stop.
Marking the methods as final forces service implementations to move to the
stricter lifecycle. It has one side effect: some of the mocking tests play up
-I'll need some assistance here
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
