Skip to content

Commit 5d33e3e

Browse files
author
Marcelo Vanzin
committed
[SPARK-18750][yarn] Avoid using "mapValues" when allocating containers.
That method is prone to stack overflows when the input map is really large; instead, use plain "map". Also includes a unit tests that was tested and caused stack overflows without the fix.
1 parent fe409f3 commit 5d33e3e

File tree

2 files changed

+83
-5
lines changed

2 files changed

+83
-5
lines changed

resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/LocalityPreferredContainerPlacementStrategy.scala

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -129,9 +129,9 @@ private[yarn] class LocalityPreferredContainerPlacementStrategy(
129129
val largestRatio = updatedHostToContainerCount.values.max
130130
// Round the ratio of preferred locality to the number of locality required container
131131
// number, which is used for locality preferred host calculating.
132-
var preferredLocalityRatio = updatedHostToContainerCount.mapValues { ratio =>
132+
var preferredLocalityRatio = updatedHostToContainerCount.map { case(k, ratio) =>
133133
val adjustedRatio = ratio.toDouble * requiredLocalityAwareContainerNum / largestRatio
134-
adjustedRatio.ceil.toInt
134+
(k, adjustedRatio.ceil.toInt)
135135
}
136136

137137
for (i <- 0 until requiredLocalityAwareContainerNum) {
@@ -145,7 +145,7 @@ private[yarn] class LocalityPreferredContainerPlacementStrategy(
145145

146146
// Minus 1 each time when the host is used. When the current ratio is 0,
147147
// which means all the required ratio is satisfied, this host will not be allocated again.
148-
preferredLocalityRatio = preferredLocalityRatio.mapValues(_ - 1)
148+
preferredLocalityRatio = preferredLocalityRatio.map { case (k, v) => (k, v - 1) }
149149
}
150150
}
151151

@@ -218,7 +218,8 @@ private[yarn] class LocalityPreferredContainerPlacementStrategy(
218218

219219
val possibleTotalContainerNum = pendingHostToContainerCount.values.sum
220220
val localityMatchedPendingNum = localityMatchedPendingAllocations.size.toDouble
221-
pendingHostToContainerCount.mapValues(_ * localityMatchedPendingNum / possibleTotalContainerNum)
222-
.toMap
221+
pendingHostToContainerCount.map { case (k, v) =>
222+
(k, v * localityMatchedPendingNum / possibleTotalContainerNum)
223+
}.toMap
223224
}
224225
}
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package org.apache.spark.deploy.yarn
19+
20+
import scala.collection.mutable.{HashMap, HashSet, Set}
21+
22+
import org.apache.hadoop.conf.Configuration
23+
import org.apache.hadoop.yarn.api.records._
24+
import org.apache.hadoop.yarn.client.api.AMRMClient.ContainerRequest
25+
import org.mockito.Mockito._
26+
27+
import org.apache.spark.{SparkConf, SparkFunSuite}
28+
29+
class LocalityPlacementStrategySuite extends SparkFunSuite {
30+
31+
test("handle large number of containers and tasks (SPARK-18750)") {
32+
// Run the test in a thread with a small stack size, since the original issue
33+
// surfaced as a StackOverflowError.
34+
var error: Throwable = null
35+
36+
val runnable = new Runnable() {
37+
override def run(): Unit = try {
38+
runTest()
39+
} catch {
40+
case e: Throwable => error = e
41+
}
42+
}
43+
44+
val thread = new Thread(new ThreadGroup("test"), runnable, "test-thread", 32 * 1024)
45+
thread.start()
46+
thread.join()
47+
48+
assert(error === null)
49+
}
50+
51+
private def runTest(): Unit = {
52+
val resource = Resource.newInstance(8 * 1024, 4)
53+
val strategy = new LocalityPreferredContainerPlacementStrategy(new SparkConf(),
54+
new Configuration(), resource)
55+
56+
val totalTasks = 32 * 1024
57+
val totalContainers = totalTasks / 16
58+
val totalHosts = totalContainers / 16
59+
60+
val hosts = (1 to totalHosts).map { i => (s"host_$i", totalTasks % i) }.toMap
61+
val containers = (1 to totalContainers).map { i =>
62+
ContainerId.fromString(s"container_12345678_0001_01_$i")
63+
}
64+
val count = containers.size / hosts.size / 2
65+
66+
val hostToContainerMap = new HashMap[String, Set[ContainerId]]()
67+
hosts.keys.take(hosts.size / 2).zipWithIndex.foreach { case (host, i) =>
68+
val hostContainers = new HashSet[ContainerId]()
69+
containers.drop(count * i).take(i).foreach { c => hostContainers += c }
70+
hostToContainerMap(host) = hostContainers
71+
}
72+
73+
strategy.localityOfRequestedContainers(containers.size * 2, totalTasks, hosts,
74+
hostToContainerMap, Nil)
75+
}
76+
77+
}

0 commit comments

Comments
 (0)