[
https://issues.apache.org/jira/browse/YARN-679?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15450404#comment-15450404
]
Daniel Templeton commented on YARN-679:
---------------------------------------
Continuing on...
* I don't see the point of having a {{shutdown()}} method in
{{ServiceShutdownHook}} that is separate from the {{run()}} method.
* {{ServiceLaunchException}} appears to be the only exception you've defined
that doesn't implement your cause exit code promotion.
* This hyphen is spurious: {code} * to build the formatted exception -in the
ENGLISH locale.{code}
* This line {code} * It is still be available as a parameter for the
format.{code} should be {code} * It will also be used as a parameter for the
format.{code}
* In {{InterruptEscalator}} here: {code} ServiceLauncher owner =
ownerRef.get();
if (owner != null) {
sb.append(", owner= ").append(owner.toString());
}{code} it would be nice to also include some note that the owner has
already been reaped if it has. Conveying information through omission is
seldom clear.
* In {{InterruptEscalator.interrupted()}}, if the service has already been
interrupted, it will log the message as a warning twice.
* In {{InterruptEscalator.interrupted()}} here: {code} //wait for that
thread to finish
try {
thread.join(shutdownTimeMillis);
} catch (InterruptedException ignored) {
//ignored
}
forcedShutdownTimedOut = !shutdown.getServiceWasShutdown();
if (forcedShutdownTimedOut) {
LOG.warn("Service did not shut down in time");
}{code} if the shutdown is interrupted, you will erroneously log that the
service did not shut down in time.
* In {{InterruptEscalator.interrupted()}} here: {code} /**
* Previous interrupt handlers. These are not queried.
*/
private final List<IrqHandler> interruptHandlers = new ArrayList<>(2);{code}
the comment doesn't make any sense to me based on the code. I also don't see
any provision made to deal with multiple handlers per signal. Maybe list list
should be a map so you can complain if the handler is already set?
More later...
> add an entry point that can start any Yarn service
> --------------------------------------------------
>
> Key: YARN-679
> URL: https://issues.apache.org/jira/browse/YARN-679
> Project: Hadoop YARN
> Issue Type: New Feature
> Components: api
> Affects Versions: 2.8.0
> Reporter: Steve Loughran
> Assignee: Steve Loughran
> Attachments: YARN-679-001.patch, YARN-679-002.patch,
> YARN-679-002.patch, YARN-679-003.patch, YARN-679-004.patch,
> YARN-679-005.patch, YARN-679-006.patch, YARN-679-007.patch,
> YARN-679-008.patch, YARN-679-009.patch, YARN-679-010.patch,
> YARN-679-011.patch, org.apache.hadoop.servic...mon 3.0.0-SNAPSHOT API).pdf
>
> Time Spent: 72h
> Remaining Estimate: 0h
>
> There's no need to write separate .main classes for every Yarn service, given
> that the startup mechanism should be identical: create, init, start, wait for
> stopped -with an interrupt handler to trigger a clean shutdown on a control-c
> interrupt.
> Provide one that takes any classname, and a list of config files/options
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
