[
https://issues.apache.org/jira/browse/YARN-679?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15439909#comment-15439909
]
Daniel Templeton commented on YARN-679:
---------------------------------------
I made it a bit farther. Additional comments:
* I see that the pattern of the code from the cause trumping the code parameter
shows up in several other places. I see what's you're going for, but I really
dislike the idea of passing in two arguments for the same data with one being
ignored. I'm open to discuss it.
* Should the {{ServiceStateException}} and {{ExitUtil.ExitException}} have a
constructor that only takes a message and exit code?
* Shouldn't need the period after {code}* {@inheritDoc}.{code}
* I'd rather see {code} private static final Logger LOG =
LoggerFactory.getLogger(
HadoopUncaughtExceptionHandler.class);{code} break on the equals instead
of the paren.
Here are also some language issues from the package docs:
* Should have an "and" instead of a comma: {code} down the CLI arguments, to
execute an operation without having to{code}
* Should be "out" not "our": {code}exit if it times our or a second interrupt
is received.{code}
* I'd rephrase: {code}It will instantiate the service classname provided, by
zero-args constructor. Or, if none is available, falling back to a constructor
that takes a {@code String} as its parameter, on the assumption that the
parameter is the service name.{code} as {code}It will instantiate the service
classname provided, using the no-args constructor, or if no such constructor is
available, it will fall back to a constructor with a single {@code String}
parameter, passing the service name as the parameter value.{code}
* Missing a close paren before the comma here: {code}{@code STATE.STOPPED},
escalated into a non-zero return code{code}
* Extra space before the dash (and add another hyphen to make it a dash or use
{{—}}): {code}(prepare configuration files -covered later){code}.
* Same here: {code}and control-C signals -calling {@code stop()} on the service
when signalled{code}
* And here: {code}parse the command line -it just becomes the responsibility of
the{code}
* And here: {code}(prepare configuration files --covered later){code}
* Here the hyphen should be a comma: {code}and start it -triggering shutdown
when signalled{code}
* Same here: {code}It adds two methods to the service interface -and hence two
new features{code}, though I suppose a dash could make sense...
* And here: {code}returned by the method -so services may signal failures
simply by returning{code}
* And here: {code}Exceptions are converted into exit codes -but rather than
simply{code}
* "Signalled" is misspelled several times. Should be "signaled"
* {code}(there is way covered later){code} is missing an "a"
* Here: {code}commands can be implemented as such services, so integrating with
YARN's{code}, the "so" should be a "thus"
* Same here: {code}initialized, so allowing services to tune their
configuration data before{code}
* This hyphen is spurious: {code}-which may be needed to trigger the injection
of HDFS or YARN resources{code}
* Same here: {code}of returning error codes to signal failures -and for {code}
* And here: {code}these classes -simply to force in the common resources{code}
* And here: {code}files are merged -with the latest file on the command line
being the{code}
* Remove the space before the comma: {code}have a "something went wrong" exit
code , exceptions <i>may</i>{code}
* "such" should be "the": {code}the service instance and such like{code}
* Missing an "i" in "is": {code}exit code of 0 s created{code}
* Here the "-so" should just be a comma: {code}stop the service if a shutdown
request is received -so ensuring that{code}
Maybe after the weekend I'll be up to finishing the review. :)
> 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]
