Skip to content

[SPARK-25922][K8] Spark Driver/Executor "spark-app-selector" label mismatch #23322

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
suxingfate wants to merge 10 commits into from
Closed

[SPARK-25922][K8] Spark Driver/Executor "spark-app-selector" label mismatch #23322

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
89c438c
[SPARK-25922][K8] Spark Driver/Executor spark-app-selector label mism…
Dec 14, 2018
ed2c4ca
Simplify the logic to use spark.app.id if set otherwise use superclas…
suxingfate Dec 15, 2018
fa45013
Merge branch 'master' into SPARK-25922
suxingfate Dec 15, 2018
5ed8e1a
remove unnessesary KubernetesUtils import in KubernetesClusterSchedul…
suxingfate Dec 15, 2018
db3fceb
remove unnessesary UUID import in KubernetesUtils.scala
suxingfate Dec 15, 2018
af73427
refactor applicationId to be neat as one line
suxingfate Dec 15, 2018
778acb4
add unit testing to test applicationId in case spark.app.id is set
suxingfate Dec 15, 2018
bf4bc0e
update style for getOrElse
suxingfate Dec 15, 2018
8844f3d
Merge branch 'master' into SPARK-25922
suxingfate Dec 15, 2018
45ded64
run ./dev/scalafmt to reformat
suxingfate Dec 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion ...main/scala/org/apache/spark/scheduler/cluster/k8s/KubernetesClusterSchedulerBackend.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ package org.apache.spark.scheduler.cluster.k8s

import java.util.concurrent.ExecutorService

import io.fabric8.kubernetes.client.KubernetesClient
import scala.concurrent.{ExecutionContext, Future}

import io.fabric8.kubernetes.client.KubernetesClient

import org.apache.spark.SparkContext
import org.apache.spark.deploy.k8s.Config._
import org.apache.spark.deploy.k8s.Constants._
Expand Down Expand Up @@ -60,6 +61,16 @@ private[spark] class KubernetesClusterSchedulerBackend(
removeExecutor(executorId, reason)
}

/**
* Get an application ID associated with the job.
* This returns the string value of spark.app.id if set, otherwise
* the locally-generated ID from the superclass.
* @return The application ID
*/
override def applicationId(): String = {
conf.getOption("spark.app.id").map(_.toString).getOrElse {super.applicationId}
Copy link
Member

@felixcheung felixcheung Dec 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit style: spacing around { }

Copy link
Member

@felixcheung felixcheung Dec 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, this should use getOrElse(super.applicationId)

Copy link
Contributor Author

@suxingfate suxingfate Dec 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

}

override def start(): Unit = {
super.start()
if (!Utils.isDynamicAllocationEnabled(conf)) {
Expand Down
6 changes: 2 additions & 4 deletions ...scala/org/apache/spark/scheduler/cluster/k8s/KubernetesClusterSchedulerBackendSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class KubernetesClusterSchedulerBackendSuite extends SparkFunSuite with BeforeAn

private val requestExecutorsService = new DeterministicScheduler()
private val sparkConf = new SparkConf(false)
.set("spark.executor.instances", "3")
.set("spark.executor.instances", "3").set("spark.app.id", TEST_SPARK_APP_ID)
Copy link
Member

@felixcheung felixcheung Dec 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test this?


@Mock
private var sc: SparkContext = _
Expand Down Expand Up @@ -100,9 +100,7 @@ class KubernetesClusterSchedulerBackendSuite extends SparkFunSuite with BeforeAn
podAllocator,
lifecycleEventHandler,
watchEvents,
pollEvents) {
override def applicationId(): String = TEST_SPARK_APP_ID
}
pollEvents)
}

test("Start all components") {
Expand Down