[
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: [email protected]
For additional commands, e-mail: [email protected]
