squito commented on a change in pull request #24935: [SPARK-28005][YARN] Remove
unnecessary log from SparkRackResolver
URL: https://github.com/apache/spark/pull/24935#discussion_r296788625
##########
File path:
resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/SparkRackResolver.scala
##########
@@ -70,18 +70,20 @@ private[spark] class SparkRackResolver(conf:
Configuration) extends Logging {
val nodes = new ArrayBuffer[Node]
// dnsToSwitchMapping is thread-safe
val rNameList = dnsToSwitchMapping.resolve(hostNames.toList.asJava).asScala
- if (rNameList == null || rNameList.isEmpty) {
+ if (rNameList == null) {
hostNames.foreach(nodes += new NodeBase(_, NetworkTopology.DEFAULT_RACK))
logInfo(s"Got an error when resolving hostNames. " +
s"Falling back to ${NetworkTopology.DEFAULT_RACK} for all")
} else {
- for ((hostName, rName) <- hostNames.zip(rNameList)) {
- if (Strings.isNullOrEmpty(rName)) {
- nodes += new NodeBase(hostName, NetworkTopology.DEFAULT_RACK)
- logDebug(s"Could not resolve $hostName. " +
- s"Falling back to ${NetworkTopology.DEFAULT_RACK}")
- } else {
- nodes += new NodeBase(hostName, rName)
+ if (rNameList.nonEmpty) {
+ for ((hostName, rName) <- hostNames.zip(rNameList)) {
Review comment:
if `hostNames` is nonEmpty, but `rNameList` is empty, this will just
silently skip everything, right? that shouldn't happen, but as we're calling
code outside of spark a little defensiveness would be good here. I think the
calling code requires the returned list to be the same length as `hostNames`.
I am also biased against early returns, but here I think it really is the
simplest alternative.
(sorry for so much quibbling about such a small thing ...)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
