Siddharth Seth commented on TEZ-3317:

Some late comments. Think we should address some of them in a separate jira.

- Progress in case of 0 inputs. Not sure if this is handled (suspect it's 
returning 0.0f)
- TezProcessorContextImpl - != comparison on a float. Will this work as 
- getProgress on the processor throws an IOException and InterruptedException. 
Why is IOException special cased?
- Similar changes required to AbstractLogicalOutput?
- The Timer threads that are setup don't have any kind of synchronization. 
There's no synchronization on the processors when accessing the progress 
either. Think visibility of variables within the timed thread that reports 
progress can be a problem. e.g. totalBytesRead and totalFileBytes are updated 
by the worker thread, but read by the thread started by the timer in 
- Does the time interval in the MapProcessor (100ms at the moment) need to be 
linked to any other config property / made configurable?
- [Nit] UnorderedKVReader - return 0l instead of 0.0f?
- TestMapProcessor - generateInputSplit, writeSplitFiles. This looks very 
similar to what exists in MapUtils. Can that be re-used?
- ShuffleManager.getNumCompletedInputs. Expose the int value, instead of 
exposing the internal AtomicInteger
- The Processors seem to share code for reporting timed progress. Can this be 
moved into a common base, or a helper?
- Specific reason to use a Timer? Otherwise can we move to a 

> Speculative execution starts too early due to 0 progress
> --------------------------------------------------------
>                 Key: TEZ-3317
>                 URL: https://issues.apache.org/jira/browse/TEZ-3317
>             Project: Apache Tez
>          Issue Type: Improvement
>            Reporter: Jonathan Eagles
>            Assignee: Kuhu Shukla
>             Fix For: 0.9.0
>         Attachments: TEZ-3317.001.patch, TEZ-3317.002.patch, 
> TEZ-3317.003.patch, TEZ-3317.004.patch, TEZ-3317.005.patch, 
> TEZ-3317.006.patch, TEZ-3317.007.patch, TEZ-3317.008.patch, 
> TEZ-3317.009.patch, TEZ-3317.010.patch
> Don't know at this point if this is a tez or a PigProcessor issue. There is 
> some setProgress chain that is keeping task progress from being correctly 
> reported. Task status is always zero, so as soon as the first task finishes, 
> tasks up to the speculation limit are always launched.

This message was sent by Atlassian JIRA

Reply via email to