[
https://issues.apache.org/jira/browse/YARN-5611?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15633911#comment-15633911
]
Sunil G commented on YARN-5611:
-------------------------------
Thanks [~rohithsharma] for the patch.
Few comments
1. ApplicationUpdateTimeoutMapProto -> ApplicationUpdateTimeoutProto. May not
need to have "map" in the name of proto.
2. Few more comment about the map "applicationTimeouts" in
{{updateApplicationTimeouts}} of ClientRMService
3. I think we need to create an event internally here for
{{RPCUtil.getRemoteException}}.
{code}
if (applicationTimeouts.isEmpty()) {
String message =
"At least one ApplicationTimeoutType should be configured for
updating timeouts.";
RMAuditLogger.logFailure(callerUGI.getShortUserName(),
AuditConstants.UPDATE_APP_TIMEOUTS, "UNKNOWN",
"ClientRMService",
message, applicationId);
throw RPCUtil.getRemoteException(message);
}
{code}
4. There is a any chance that NEW_SAVING could fail and app could no longer be
in next state. We could consider apps which are from ACCEPTED state, correct?
Any specific reason to consider NEW_SAVING?
{code}
if (RMAppState.NEW.equals(application.getState())
|| COMPLETED_APP_STATES.contains(application.getState())) {
{code}
5. In {{validateISO8601AndConvertToLocalTimeEpoch}}, instead of using {{long
currentTimeMillis = System.currentTimeMillis();}}, its better to use Clock.
6. In {{validateISO8601AndConvertToLocalTimeEpoch}}, {{if (expireTime <
currentTimeMillis)}} helps to validate timeout's of past. But still there could
be a delta time from current time to next timeout check thread. Could that also
to be validated here? Or such entries will be discarded by the monitor thread?
7. In general thought and thinking out loud, {{Map<ApplicationTimeoutType,
Long> }} why we need a map here. Could we have {{List<ApplicationTimeouts>}}
and each *ApplicationTimeouts* could have its type and real timeout value in
long. I am not seeing much of benefit from map here as app timeout types could
no be more than 3/4 atmost.
> Provide an API to update lifetime of an application.
> ----------------------------------------------------
>
> Key: YARN-5611
> URL: https://issues.apache.org/jira/browse/YARN-5611
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: resourcemanager
> Reporter: Rohith Sharma K S
> Assignee: Rohith Sharma K S
> Labels: oct16-hard
> Attachments: 0001-YARN-5611.patch, 0002-YARN-5611.patch,
> 0003-YARN-5611.patch, YARN-5611.0004.patch, YARN-5611.0005.patch,
> YARN-5611.v0.patch
>
>
> YARN-4205 monitors an Lifetime of an applications is monitored if required.
> Add an client api to update lifetime of an application.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
