[
https://issues.apache.org/jira/browse/TEZ-3077?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15223058#comment-15223058
]
Siddharth Seth commented on TEZ-3077:
-------------------------------------
Thanks for the updated patch [~kshukla]. This looks a lot better in terms of
the APIs. Some comments.
- Can the existing preWarm method be changed to invoke the new one with a
timeout of 0 ? Similar to what has been done for the existing waitTillReady
method.
- In waitTillReady
{code}
+ if ((timeout > 0) &&
+ Time.monotonicNow() - startTime >= timeout) {
+ return false;
{code}
This check should be after checking the updated status to be READY. Otherwise
we could end up timing out in the last iteration even if the state did change
to READY.
{code}long sleepTime = (SLEEP_FOR_READY > timeout) ?
SLEEP_FOR_READY - timeout : SLEEP_FOR_READY;{code}
Should this be {code}
long sleepTime = (SLEEP_FOR_READY > timeout) ?
timeout : SLEEP_FOR_READY;
{code}
Even better would be to sleep for whatever time is actually left.
{code}
long now = Time.monotonicNow();
if (startTime + timeout > now) {
long sleepTime = Math.min(SLEEP_FOR_READY, startTime + timeout - now);
Thread.sleep(sleepTime);
} else {
return false;
}
{code}
On the unit test, could you please look at testStopRetriesUntilTimeout - and
see if a test can be added along these lines. i.e. it actually validates that
attempts were made to get the appReport, and a final timeout - rather than
returning success.
> TezClient.waitTillReady should support timeout
> ----------------------------------------------
>
> Key: TEZ-3077
> URL: https://issues.apache.org/jira/browse/TEZ-3077
> Project: Apache Tez
> Issue Type: Bug
> Reporter: Sergey Shelukhin
> Assignee: Kuhu Shukla
> Attachments: TEZ-3077.001.patch, TEZ-3077.002.patch
>
>
> Also preWarm.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
