Varun Saxena commented on YARN-3528:

Thanks for updating the patch [~brahmareddy]. Few comments.

# Move {{ServerSocketUtil}} to {{org.apache.hadoop.net}} instead of having it 
in {{org.apache.hadoop.fs}}.
# As this is a utility class which might be used elsewhere as well, can we pass 
an initial port into {{getPort()}} and try with that port first before 
randomizing . We can use this instead of using 49152 everytime.
# Probably can pass number of retries as well instead of fixing it as 10. Let 
the caller decide. Thoughts ?
# Replace {{System.out.println}} by {{LOG}}
# In catch block no need to recreate {{IOException}} and wrapping the caught 
exception. You can directly throw the exception caught as it is also 
{{IOException}} and you are not adding any additional information.
# What's the point of having {{getFreePort()}} ? You can write 0 directly in 
code instead of calling this function or use a constant. Thoughts ?
# If this is a class to be used only by tests, we can move it to test folder ?
# In {{TestNodeManagerShutdown}}, catching {{RuntimeException}} is unnecessary.
# In {{TestNodeManagerShutdown#startContainer}}, if exception is thrown(i.e. no 
free port is available) the code simply continues on to make a call to 
{{rpc.getProxy()}} with {{null}} containerManagerBindAddress. We can probably 
throw and exception so that test fails at correct location.
# Can remove below line from {{BaseContainerManagerTest}}
// String bindAddress = "";

> 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.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