Patch still has trailing whitespace issues.
+ /** Max time to wait for ResourceManger start
Please rephrase to "max time to wait to establish a connection to the ResourceManager when the NodeManager starts"
+ /** Time interval for each NM attempt to connect RM
Rephrase to "Time interval between each NM attempt to connect to the ResourceManager"
You can use the same descriptions in yarn-default.xml. Not sure if "After that period of time, NM will throw out exceptions" is valid in yarn-default.xml. A better description could mention that the NM will shutdown if it cannot connect to the RM within the specified max time period.
Description should also mention how to use -1 to retry forever.
Earlier comment had a point of switching to using SECONDS instead of MS for users to understand more easily.
+ // this.hostName = InetAddress.getLocalHost().getCanonicalHostName();
- Please remove commented out code if not being used.
Unit test does not really seem to be testing the flow of RESOURCEMANAGER_CONNECT_WAIT_MS being set to -1. waitForEver is being explicitly set to true/false based on the updater's ctor and not really based on the config value. If that flow cannot be tested, it might be better to remove the additional complexity from the test.
Also, patch will need to be updated due to https://issues.apache.org/jira/browse/HADOOP-9112.