[
https://issues.apache.org/jira/browse/HADOOP-13035?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15268487#comment-15268487
]
Steve Loughran commented on HADOOP-13035:
-----------------------------------------
One thing to bear in mind about that state change lock is that as the
{{serviceInit}} and {{serviceStart}} methods are called holding the lock, code
of these methods can enter a new state while it's in progress. This is why the
{{init()}} and {{start()}} methods have checks for whether they are still in
the required state on exit. This is important because it allows {{serviceInit}}
and {{serviceStart}} to invoke stop() and not block.
Anything playing with a new state change field would have to handle similar
cases, especially {{serviceInit}} and calling {{start()}} . ( I don't know of
anything which does that; I'm not sure if we should allow it, but it's there.
stop can/may happen, which really complicates things if something like a
contained service ever directly/indirectly called it's parent {{stop()}} call.
There's also some unwinding in {{CompositeService.serviceStop()}} which only
calls {{stop()}} on children which are explicitly in the {{STARTED}}
state...we'd have to make sure that includes in-transition conditions
(probably) or explicitly decided to ignoring the starting stuff.
The more I think about this, an expanded state model, with the existing
accessor/enum retained as is, is probably the way to manage this. Users of the
old API would get the STARTED state when the service was in STARTING/STARTED;
users of the new API would get the detailed value. This would allow the inner
state model to be complete, covering the logic of the various state transitions
without having to also rely on some transitionary states.
There's also some fun when we consider that today, INIT and STARTED are
idempotent
{code}
if (enterState(STATE.INITED) != STATE.INITED) { ...}
{code}
That would actually get more complex with the extra states; perhaps
{{enterState()}} would be changed to return a bool if a state change has
*really* occurred, and both INITED/INITING and STARTED/STARTING treated as
equivalent from the perspective of idempotent transitions. That is,
STARTED.enterState(STARTING) => STARTED, STARTING.enterState(STARTED) =>
STARTED is needed, because it's how the start() operation would transit to its
final state. Whatever patch gets in, making sure it is fully idempotent here
will have to be looked at carefully
> Add states INITING and STARTING to YARN Service model to cover in-transition
> states.
> ------------------------------------------------------------------------------------
>
> Key: HADOOP-13035
> URL: https://issues.apache.org/jira/browse/HADOOP-13035
> Project: Hadoop Common
> Issue Type: Bug
> Reporter: Bibin A Chundatt
> Attachments: 0001-HADOOP-13035.patch, 0002-HADOOP-13035.patch,
> 0003-HADOOP-13035.patch
>
>
> As per the discussion in YARN-3971 the we should be setting the service state
> to STARTED only after serviceStart()
> Currently {{AbstractService#start()}} is set
> {noformat}
> if (stateModel.enterState(STATE.STARTED) != STATE.STARTED) {
> try {
> startTime = System.currentTimeMillis();
> serviceStart();
> ..
> }
> {noformat}
> enterState sets the service state to proposed state. So in
> {{service.getServiceState}} in {{serviceStart()}} will return STARTED .
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
