[
https://issues.apache.org/jira/browse/TEZ-3437?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15549450#comment-15549450
]
Siddharth Seth commented on TEZ-3437:
-------------------------------------
{code}
+ public float getProgress() throws ProgressFailedException,
InterruptedException {
+ try {
+ return (mrReader != null) ? mrReader.getProgress() : 0.0f;
+ } catch (IOException e) {
+ throw new ProgressFailedException("getProgress encountered IOException
", e);
+ } catch (InterruptedException e) {
+ throw new ProgressFailedException("getProgress was Interrupted ", e);
+ }
}
{code}
Why catch an InterruptedException here? The method signature already declares
this as being throwable.
Signature of getProgrress in ConcatenatedMergedKeyValuesInput... this is also
catching InterruptedException. [~kshukla] - could you please look at all
implementations of getProgress for correct signatures, exception handling in
the next patch.
{code}
+ public void shutDownProgressTaskService() {
+ scheduledExecutorService.shutdownNow();
+ }
{code}
Also requires a null check (The variable is setup on invocation of a method,
outside the constructor). In fact schedulerExecutorService should be volatile.
Other than these comments - looks ok to me from a quick glance.
> Improve synchronization and the progress report behavior for Inputs from
> TEZ-3317
> ---------------------------------------------------------------------------------
>
> Key: TEZ-3437
> URL: https://issues.apache.org/jira/browse/TEZ-3437
> Project: Apache Tez
> Issue Type: Bug
> Reporter: Kuhu Shukla
> Assignee: Kuhu Shukla
> Attachments: TEZ-3437.001.patch, TEZ-3437.002.patch,
> TEZ-3437.003.patch, TEZ-3437.004.patch, TEZ-3437.005.patch,
> TEZ-3437.006.patch, TEZ-3437.007.patch, TEZ-3437.008.patch
>
>
> Follow up from TEZ-3317 to improve the getProgress thread synchronization and
> replace timerTasks with ScheduledExecutorService.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
