Skip to content

Commit c744d66

Browse files
srowenpwendell
srowen
authored and
pwendell
committed
SPARK-1497. Fix scalastyle warnings in YARN, Hive code
(I wasn't sure how to automatically set `SPARK_YARN=true` and `SPARK_HIVE=true` when running scalastyle, but these are the errors that turn up.) Author: Sean Owen <sowen@cloudera.com> Closes #413 from srowen/SPARK-1497 and squashes the following commits: f0c9318 [Sean Owen] Fix more scalastyle warnings in yarn 80bf4c3 [Sean Owen] Add YARN alpha / YARN profile to scalastyle check 026319c [Sean Owen] Fix scalastyle warnings in YARN, Hive code (cherry picked from commit 77f8367) Signed-off-by: Patrick Wendell <pwendell@gmail.com>
1 parent 8efec04 commit c744d66

File tree

6 files changed

+34
-20
lines changed

6 files changed

+34
-20
lines changed

‎dev/scalastyle‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@
1818
#
1919

2020
echo -e "q\n" | sbt/sbt clean scalastyle > scalastyle.txt
21+
# Check style with YARN alpha built too
22+
SPARK_YARN=true sbt/sbt yarn/scalastyle >> scalastyle.txt
23+
# Check style with YARN built too
24+
SPARK_HADOOP_VERSION=2.2.0 SPARK_YARN=true sbt/sbt yarn/scalastyle >> scalastyle.txt
2125
ERRORS=$(cat scalastyle.txt | grep -e "\<error\>")
2226
if test ! -z "$ERRORS"; then
2327
echo -e "Scalastyle checks failed at following occurrences:\n$ERRORS"

‎yarn/alpha/src/main/scala/org/apache/spark/deploy/yarn/ExecutorLauncher.scala‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@ import org.apache.spark.scheduler.SplitInfo
3737
class ExecutorLauncher(args: ApplicationMasterArguments, conf: Configuration, sparkConf: SparkConf)
3838
extends Logging {
3939

40-
def this(args: ApplicationMasterArguments, sparkConf: SparkConf) = this(args, new Configuration(), sparkConf)
40+
def this(args: ApplicationMasterArguments, sparkConf: SparkConf) =
41+
this(args, new Configuration(), sparkConf)
4142

4243
def this(args: ApplicationMasterArguments) = this(args, new SparkConf())
4344

@@ -63,7 +64,8 @@ class ExecutorLauncher(args: ApplicationMasterArguments, conf: Configuration, sp
6364
override def preStart() {
6465
logInfo("Listen to driver: " + driverUrl)
6566
driver = context.actorSelection(driverUrl)
66-
// Send a hello message thus the connection is actually established, thus we can monitor Lifecycle Events.
67+
// Send a hello message thus the connection is actually established, thus we can
68+
// monitor Lifecycle Events.
6769
driver ! "Hello"
6870
context.system.eventStream.subscribe(self, classOf[RemotingLifecycleEvent])
6971
}
@@ -104,8 +106,9 @@ class ExecutorLauncher(args: ApplicationMasterArguments, conf: Configuration, sp
104106
// Allocate all containers
105107
allocateExecutors()
106108

107-
// Launch a progress reporter thread, else app will get killed after expiration (def: 10mins) timeout
108-
// ensure that progress is sent before YarnConfiguration.RM_AM_EXPIRY_INTERVAL_MS elapse.
109+
// Launch a progress reporter thread, else app will get killed after expiration
110+
// (def: 10mins) timeout ensure that progress is sent before
111+
// YarnConfiguration.RM_AM_EXPIRY_INTERVAL_MS elapse.
109112

110113
val timeoutInterval = yarnConf.getInt(YarnConfiguration.RM_AM_EXPIRY_INTERVAL_MS, 120000)
111114
// we want to be reasonably responsive without causing too many requests to RM.
@@ -163,8 +166,8 @@ class ExecutorLauncher(args: ApplicationMasterArguments, conf: Configuration, sp
163166
val appMasterRequest = Records.newRecord(classOf[RegisterApplicationMasterRequest])
164167
.asInstanceOf[RegisterApplicationMasterRequest]
165168
appMasterRequest.setApplicationAttemptId(appAttemptId)
166-
// Setting this to master host,port - so that the ApplicationReport at client has some sensible info.
167-
// Users can then monitor stderr/stdout on that node if required.
169+
// Setting this to master host,port - so that the ApplicationReport at client has
170+
// some sensible info. Users can then monitor stderr/stdout on that node if required.
168171
appMasterRequest.setHost(Utils.localHostName())
169172
appMasterRequest.setRpcPort(0)
170173
// What do we provide here ? Might make sense to expose something sensible later ?
@@ -213,7 +216,8 @@ class ExecutorLauncher(args: ApplicationMasterArguments, conf: Configuration, sp
213216
// TODO: This is a bit ugly. Can we make it nicer?
214217
// TODO: Handle container failure
215218
while ((yarnAllocator.getNumExecutorsRunning < args.numExecutors) && (!driverClosed)) {
216-
yarnAllocator.allocateContainers(math.max(args.numExecutors - yarnAllocator.getNumExecutorsRunning, 0))
219+
yarnAllocator.allocateContainers(
220+
math.max(args.numExecutors - yarnAllocator.getNumExecutorsRunning, 0))
217221
Thread.sleep(100)
218222
}
219223

@@ -230,7 +234,8 @@ class ExecutorLauncher(args: ApplicationMasterArguments, conf: Configuration, sp
230234
while (!driverClosed) {
231235
val missingExecutorCount = args.numExecutors - yarnAllocator.getNumExecutorsRunning
232236
if (missingExecutorCount > 0) {
233-
logInfo("Allocating " + missingExecutorCount + " containers to make up for (potentially ?) lost containers")
237+
logInfo("Allocating " + missingExecutorCount +
238+
" containers to make up for (potentially ?) lost containers")
234239
yarnAllocator.allocateContainers(missingExecutorCount)
235240
}
236241
else sendProgress()

‎yarn/alpha/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocationHandler.scala‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -225,8 +225,8 @@ private[yarn] class YarnAllocationHandler(
225225
val executorHostname = container.getNodeId.getHost
226226
val containerId = container.getId
227227

228-
assert(
229-
container.getResource.getMemory >= (executorMemory + YarnAllocationHandler.MEMORY_OVERHEAD))
228+
assert( container.getResource.getMemory >=
229+
(executorMemory + YarnAllocationHandler.MEMORY_OVERHEAD))
230230

231231
if (numExecutorsRunningNow > maxExecutors) {
232232
logInfo("""Ignoring container %s at host %s, since we already have the required number of
@@ -393,9 +393,10 @@ private[yarn] class YarnAllocationHandler(
393393

394394
// default.
395395
if (numExecutors <= 0 || preferredHostToCount.isEmpty) {
396-
logDebug("numExecutors: " + numExecutors + ", host preferences: " + preferredHostToCount.isEmpty)
397-
resourceRequests = List(
398-
createResourceRequest(AllocationType.ANY, null, numExecutors, YarnAllocationHandler.PRIORITY))
396+
logDebug("numExecutors: " + numExecutors + ", host preferences: " +
397+
preferredHostToCount.isEmpty)
398+
resourceRequests = List(createResourceRequest(
399+
AllocationType.ANY, null, numExecutors, YarnAllocationHandler.PRIORITY))
399400
}
400401
else {
401402
// request for all hosts in preferred nodes and for numExecutors -

‎yarn/stable/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,8 @@ class ApplicationMaster(args: ApplicationMasterArguments, conf: Configuration,
137137
System.getenv(ApplicationConstants.APPLICATION_WEB_PROXY_BASE_ENV)
138138

139139
val params = "PROXY_HOST=" + parts(0) + "," + "PROXY_URI_BASE=" + uriBase
140-
System.setProperty("spark.org.apache.hadoop.yarn.server.webproxy.amfilter.AmIpFilter.params", params)
140+
System.setProperty(
141+
"spark.org.apache.hadoop.yarn.server.webproxy.amfilter.AmIpFilter.params", params)
141142
}
142143

143144
/** Get the Yarn approved local directories. */

‎yarn/stable/src/main/scala/org/apache/spark/deploy/yarn/ExecutorLauncher.scala‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,8 @@ class ExecutorLauncher(args: ApplicationMasterArguments, conf: Configuration, sp
6565
override def preStart() {
6666
logInfo("Listen to driver: " + driverUrl)
6767
driver = context.actorSelection(driverUrl)
68-
// Send a hello message thus the connection is actually established, thus we can monitor Lifecycle Events.
68+
// Send a hello message thus the connection is actually established,
69+
// thus we can monitor Lifecycle Events.
6970
driver ! "Hello"
7071
context.system.eventStream.subscribe(self, classOf[RemotingLifecycleEvent])
7172
}
@@ -95,8 +96,9 @@ class ExecutorLauncher(args: ApplicationMasterArguments, conf: Configuration, sp
9596
// Allocate all containers
9697
allocateExecutors()
9798

98-
// Launch a progress reporter thread, else app will get killed after expiration (def: 10mins) timeout
99-
// ensure that progress is sent before YarnConfiguration.RM_AM_EXPIRY_INTERVAL_MS elapse.
99+
// Launch a progress reporter thread, else app will get killed after expiration
100+
// (def: 10mins) timeout ensure that progress is sent before
101+
// YarnConfiguration.RM_AM_EXPIRY_INTERVAL_MS elapse.
100102

101103
val timeoutInterval = yarnConf.getInt(YarnConfiguration.RM_AM_EXPIRY_INTERVAL_MS, 120000)
102104
// we want to be reasonably responsive without causing too many requests to RM.

‎yarn/stable/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocationHandler.scala‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,8 @@ private[yarn] class YarnAllocationHandler(
276276
allocatedRackCount.put(rack, allocatedRackCount.getOrElse(rack, 0) + 1)
277277
}
278278
}
279-
logInfo("Launching ExecutorRunnable. driverUrl: %s, executorHostname: %s".format(driverUrl, executorHostname))
279+
logInfo("Launching ExecutorRunnable. driverUrl: %s, executorHostname: %s".format(
280+
driverUrl, executorHostname))
280281
val executorRunnable = new ExecutorRunnable(
281282
container,
282283
conf,
@@ -314,8 +315,8 @@ private[yarn] class YarnAllocationHandler(
314315
// `pendingReleaseContainers`.
315316
pendingReleaseContainers.remove(containerId)
316317
} else {
317-
// Decrement the number of executors running. The next iteration of the ApplicationMaster's
318-
// reporting thread will take care of allocating.
318+
// Decrement the number of executors running. The next iteration of
319+
// the ApplicationMaster's reporting thread will take care of allocating.
319320
numExecutorsRunning.decrementAndGet()
320321
logInfo("Completed container %s (state: %s, exit status: %s)".format(
321322
containerId,

0 commit comments

Comments
 (0)