[ 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)