[
https://issues.apache.org/jira/browse/YARN-8499?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16853156#comment-16853156
]
Szilard Nemeth commented on YARN-8499:
--------------------------------------
Hi [~Prabhu Joseph]!
Thanks for your patch! This is a nice improvement in overall!
Some comments:
1. In constructor of TimelineStorageMonitor: Can we use an enum for storage
name instead of Strings?
2. In TimelineStorageMonitor#start: I would replace the logging message with
"Scheduling {} storage monitor at interval {}"
3. I would simply use health check in the method's name instead of
healthCheckup. Please also update the logging message accordingly.
4. In TimelineStorageMonitor.MonitorThread#run: Please rephrase logging message
to "assuming Storage is down" (note the "is")
5. In TimelineStorageMonitor.MonitorThread#runThrowables, in the catch clause,
you have a warn log. My intellij complains with: Throwables cannot be used in
combination with formatted log messages. In this case, storageName and e (the
exception itself) is treated as log format string arguments. AFAIK, in this
case, your best bet is to use LOG.warn(String.format(str, args), e);
6. I would rename the method assertStorageUp to checkStorageIsUp.
7. In HBaseStorageMonitor: Please move the static final fields from the bottom
to the top of the class.
> ATS v2 Generic TimelineStorageMonitor
> -------------------------------------
>
> Key: YARN-8499
> URL: https://issues.apache.org/jira/browse/YARN-8499
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: ATSv2
> Reporter: Sunil Govindan
> Assignee: Prabhu Joseph
> Priority: Major
> Labels: atsv2
> Attachments: YARN-8499-001.patch, YARN-8499-002.patch,
> YARN-8499-003.patch, YARN-8499-004.patch, YARN-8499-005.patch,
> YARN-8499-006.patch, YARN-8499-007.patch, YARN-8499-008.patch,
> YARN-8499-009.patch
>
>
> Post YARN-8302, Hbase connection issues are handled in ATSv2. However this
> could be made general by introducing an api in storage interface and
> implementing in each of the storage as per the store semantics.
>
> cc [~rohithsharma] [~vinodkv] [~vrushalic]
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
