[ https://issues.apache.org/jira/browse/YARN-7244?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16206560#comment-16206560 ]
Jason Lowe commented on YARN-7244: ---------------------------------- Thanks for updating the patch! Nit: The whitespace separating the private static fields from the MockShuffleHandler definition was lost. MockShuffleHandler2 should remain a static class. Nit: it would be nice if the javadocs added in AuxiliaryService were formatted to fit the style of the other method docs in this class. This change seems unnecessary: {noformat} --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/ContainerManagerImpl.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/ContainerManagerImpl.java @@ -224,8 +227,9 @@ private NMTimelinePublisher nmMetricsPublisher; public ContainerManagerImpl(Context context, ContainerExecutor exec, - DeletionService deletionContext, NodeStatusUpdater nodeStatusUpdater, - NodeManagerMetrics metrics, LocalDirsHandlerService dirsHandler) { + DeletionService deletionContext, + NodeStatusUpdater nodeStatusUpdater, NodeManagerMetrics metrics, + LocalDirsHandlerService dirsHandler) { super(ContainerManagerImpl.class.getName()); this.context = context; this.dirsHandler = dirsHandler; {noformat} TestAuxServices.auxiliaryLocalPathHandler should be final and named like a constant, e.g.: MOCK_AUX_PATH_HANDLER AuxiliaryLocalPathHandlerImpl should be package-private. In TestContainerManager#testAuxPathHandler: {code} + try { + Path p = auxiliaryLocalPathHandler.getLocalPathForRead("test"); + assertTrue(p != null); + } catch (Exception e) { + fail("Unexpected Exception!" + e); + } {code} This should just let the exception be thrown to fail the test directly rather than catch it explicitly only to fail in a generic way. It would be less code to use a spy object on a real LocalDirsHandlerService that is initialized with the conf rather than a mock that calls real methods on an object without properly initializing it. > ShuffleHandler is not aware of disks that are added > --------------------------------------------------- > > Key: YARN-7244 > URL: https://issues.apache.org/jira/browse/YARN-7244 > Project: Hadoop YARN > Issue Type: Bug > Reporter: Kuhu Shukla > Assignee: Kuhu Shukla > Attachments: YARN-7244.001.patch, YARN-7244.002.patch, > YARN-7244.003.patch, YARN-7244.004.patch, YARN-7244.005.patch, > YARN-7244.006.patch, YARN-7244.007.patch, YARN-7244.008.patch, > YARN-7244.009.patch > > > The ShuffleHandler permanently remembers the list of "good" disks on NM > startup. If disks later are added to the node then map tasks will start using > them but the ShuffleHandler will not be aware of them. The end result is that > the data cannot be shuffled from the node leading to fetch failures and > re-runs of the map tasks. -- This message was sent by Atlassian JIRA (v6.4.14#64029) --------------------------------------------------------------------- To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org