Jason Lowe commented on YARN-6315:

Thanks for updating the patch!  Sorry for the delay in reviewing.

Why is downloadSize being added as a field to the abstract LocalResource record 
class?  I think it should be treated just like all the other "fields" in this 
class -- access methods in the abstract class but implementation details 
delegated to the concrete classes.  LocalResourceRequest has a {{downloadSize}} 
as well which makes things a bit confusing. 

{{downloadSize}} is being set to the same value as {{size}} in the record 
creation?  That makes me think we don't need to put this in the protocol buffer 
record but instead track this separately in a non-protocol buffer record on the 
NM side.

Thinking out loud here: I'm not sure we really need the client to tell us what 
the size is in the protocol buffer record.  We're already doing a timestamp 
check which should be close enough when combined with the path to uniquely 
identify the resource being localized.  Once we have uniquely identified the 
resource the NM will download for the user then we can track how big the 
download was when the NM localized it in LocalizedResource.  Then we can check 
that size against the size found on disk to verify the resource still looks OK, 
at least at a high level.

Note that YARN-2185 and archives in general may be problematic for this 
approach since what we download becomes something very different on the local 
disk due to the archive unpack process.

> Improve LocalResourcesTrackerImpl#isResourcePresent to return false for 
> corrupted files
> ---------------------------------------------------------------------------------------
>                 Key: YARN-6315
>                 URL: https://issues.apache.org/jira/browse/YARN-6315
>             Project: Hadoop YARN
>          Issue Type: Bug
>    Affects Versions: 2.7.3, 2.8.1
>            Reporter: Kuhu Shukla
>            Assignee: Kuhu Shukla
>            Priority: Major
>         Attachments: YARN-6315.001.patch, YARN-6315.002.patch, 
> YARN-6315.003.patch, YARN-6315.004.patch, YARN-6315.005.patch, 
> YARN-6315.006.patch
> We currently check if a resource is present by making sure that the file 
> exists locally. There can be a case where the LocalizationTracker thinks that 
> it has the resource if the file exists but with size 0 or less than the 
> "expected" size of the LocalResource. This JIRA tracks the change to harden 
> the isResourcePresent call to address that case.

This message was sent by Atlassian JIRA

To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org

Reply via email to