hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jason Lowe (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-7244) ShuffleHandler is not aware of disks that are added
Date Mon, 16 Oct 2017 20:34:00 GMT

    [ 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-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-issues-help@hadoop.apache.org


Mime
View raw message