Varun Saxena commented on YARN-3528:


Few comments :

# Amongst the test failures which have come in QA report above, except one test 
each in TestDeletionService and TestResourceLocalizationService, all are 
related to your code change. 
# You need to make a call to {{ServerSocketUtil#getPort}} only in places where 
the port is used for binding to a socket. You have used it elsewhere as well.
# In {{TestNodeManagerReboot#createNMConfig}}, below changes would lead to bind 
exception. Because at the time of setting config, call to 
ServerSocketUtil#getPort will return 49152(if free) and both NM_ADDRESS and 
NM_LOCALIZER_ADDRESS will be set using same port. The start port should be 
different at the time of second call to getPort.
-      conf.set(YarnConfiguration.NM_ADDRESS, "");
-      conf.set(YarnConfiguration.NM_LOCALIZER_ADDRESS, "");
+      conf.set(YarnConfiguration.NM_ADDRESS,
+          "" + ServerSocketUtil.getPort(49152, 10));
+      conf.set(YarnConfiguration.NM_LOCALIZER_ADDRESS, ""
+          + ServerSocketUtil.getPort(49152, 10));
# Same problem as above exists in {{TestNodeManagerResync#createNMConfig}} 
which will lead to test failures due to BindException.
# In {{TestNMContainerTokenSecretManager}}, {{TestRMAppLogAggregationStatus}} 
and {{TestNMTokenSecretManagerInNM}}, you do not need to get port from 
ServerSocketUtil to set NodeID
# In {{TestNMWebServer#testNMWebApp}}, you do not need to call 
ServerSocketUtil#getPort to create the token. Token is not used for socket 
# In {{TestRMApplicationHistoryWriter}} and {{TestAMRMRPCResponseId}}, call to 
MockRM#registerNode does not need a unique port to bind to. So again we do not 
need to get port from ServerSocketUtil
# Same applies for {{TestAMRestart}} wherever MockNM constructor is invoked.
# In {{TestAMRestart}}, you need to have different ports wherever more than one 
MockNM object is created. This is leading to test failure in QA report above.
# Nit: Formatting of below piece of code in {{TestNMWebServer}} is not correct.
       Token containerToken =
-          BuilderUtils.newContainerToken(containerId, "", 1234, user,
+ BuilderUtils.newContainerToken(containerId,
+          "", ServerSocketUtil.getPort(49152, 10), user,

> Tests with 12345 as hard-coded port break jenkins
> -------------------------------------------------
>                 Key: YARN-3528
>                 URL: https://issues.apache.org/jira/browse/YARN-3528
>             Project: Hadoop YARN
>          Issue Type: Bug
>    Affects Versions: 3.0.0
>         Environment: ASF Jenkins
>            Reporter: Steve Loughran
>            Assignee: Brahma Reddy Battula
>            Priority: Blocker
>              Labels: test
>         Attachments: YARN-3528-002.patch, YARN-3528-003.patch, 
> YARN-3528-004.patch, YARN-3528.patch
> A lot of the YARN tests have hard-coded the port 12345 for their services to 
> come up on.
> This makes it impossible to have scheduled or precommit tests to run 
> consistently on the ASF jenkins hosts. Instead the tests fail regularly and 
> appear to get ignored completely.
> A quick grep of "12345" shows up many places in the test suite where this 
> practise has developed.
> * All {{BaseContainerManagerTest}} subclasses
> * {{TestNodeManagerShutdown}}
> * {{TestContainerManager}}
> + others
> This needs to be addressed through portscanning and dynamic port allocation. 
> Please can someone do this.

This message was sent by Atlassian JIRA

Reply via email to