-
Notifications
You must be signed in to change notification settings - Fork 28.5k
[SPARK-23078] [CORE] [K8s] allow Spark Thrift Server to run in Kubernetes Cluster mode #20272
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.
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
Conversation
@ozzieba, can we add a test to our integration test set to ensure this works? It's rather late in the Spark 2.3 release, and I'd be apprehensive about adding things that haven't been extensively tested. |
I'm getting stuck on https://github.com/apache-spark-on-k8s/spark-integration/blob/master/integration-test/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/KubernetesSuite.scala#L106, will look again tomorrow |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add [K8s] to the PR title.
we really need to test this...
@@ -328,7 +328,7 @@ object SparkSubmit extends CommandLineUtils with Logging { | |||
printErrorAndExit("Cluster deploy mode is not applicable to Spark shells.") | |||
case (_, CLUSTER) if isSqlShell(args.mainClass) => | |||
printErrorAndExit("Cluster deploy mode is not applicable to Spark SQL shell.") | |||
case (_, CLUSTER) if isThriftServer(args.mainClass) => | |||
case (_, CLUSTER) if (clusterManager != KUBERNETES) && isThriftServer(args.mainClass) => | |||
printErrorAndExit("Cluster deploy mode is not applicable to Spark Thrift server.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't dug through but this might break if there has been any assumption that thrift is not running in cluster mode?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you elaborate on what might go wrong? I have been using it successfully to run queries, is there anything else that should be tested?
@foxish @felixcheung I wrote a test, but I'm having trouble with Minikube on Windows, and I can't run on a remote Kubernetes cluster. I'll try to get minikube running in a different way and keep you updated |
@foxish @felixcheung Now verified my test runs successfully apache-spark-on-k8s/spark-integration#38 |
cc @liyinan926 Do you have some time to verify this? |
@jiangxb1987 Is there any specific owner of the thrift server that we can ping here? The testing looks good - so, all we're waiting for is confirmation from them on the original intent behind disallowing the thrift server in cluster mode; and if this is safe. |
@foxish per SPARK-5176 and the associated PR, it seems there was a technical issue with spark-internal that wouldn't allow Thrift Server to run on YARN cluster mode. It was deemed a minor fix at the time, so perhaps the changes were made in the meantime. Regardless, it does now work at least on Kubernetes |
@andrewor14 @liancheng can you chime in? |
IIUC there was a issue in launching Thrift Server on YARN cluster mode, and I'm not sure whether it has been fixed (maybe @jerryshao can kindly check that?) Anyway that is not a problem on Spark side, therefore should not affect the Kubernetes cluster mode. |
Makes sense. The change LGTM.
…On Jan 29, 2018 10:23 AM, "Jiang Xingbo" ***@***.***> wrote:
IIUC there was a issue in launching Thrift Server on YARN cluster mode,
and I'm not sure whether it has been fixed (maybe @jerryshao
<https://github.com/jerryshao> can kindly check that?) Anyway that is not
a problem on Spark side, therefore should not affect the Kubernetes cluster
mode.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#20272 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AA3U51U4wlskTcUiDRn3JSJUIg2Dx7vfks5tPgyDgaJpZM4Re1NU>
.
|
Sorry I cannot remember the issue. Yarn cluster mode doesn't support thriftserver. |
So how are you expected to contact the thrift server after it's up, and even figure out where it started? Isn't the container unreachable from the outside world unless you do some port mapping on the host? I see part of that is explained in the bug, but sounds like this change needs better user documentation. |
@vanzin In general Kubernetes makes this super easy:
Perhaps the key point is that these are all core Kubernetes features, and not specific to Spark in any way. Users familiar with Kubernetes should be able to find the approach that works best for their environment and use case. |
Great, how about explaining that in some place more visible to Spark users than a PR on github. |
Ultimately I see this as being in the realm of Kubernetes knowledge, rather than Spark knowledge. These features are well-documented in Kubernetes documentation, and I am not sure there's a need to replicate that here. For a user new to both Spark and Kubernetes a combined tutorial could certainly be helpful, but that should likely be a more comprehensive undertaking. |
It's really not that much work to write a couple of sentences about this in the running-on-k8s docs, even if you're just pointing people to the kubernetes documentation. Is that really so controversial? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note to self: Jenkins hasn't run on this.
Is anyone still looking at this? It seems like it should just work? |
Its working for us in our test environment. No issues so far. Only thing to take care is the clean-up of the driver manually when the thrift server is re-installed or started due to any reason. |
Can one of the admins verify this patch? |
@felixcheung I think yes and with #21748, users should be able to run the Thrift server in a pod. |
If this can get a rebase, and maybe a few sentences in the k8s docs, I'll merge. |
still need to run tests #20272 (review) |
I have built spark with the same changes for thrift server. It is running fine for me. |
is this coming with spark 2.4 ? |
Hi, I had rebased this patch to the latest master and this patch was missing another fix to run the STS on K8S cluster mode. Following is the PR Can you please review this once? |
I'm closing this for now since there's another PR, and also I haven't seen an answer to my questions on the bug. |
Is this feature available in spark 2.4? |
What changes were proposed in this pull request?
allow Spark Thrift Server to run in Kubernetes Cluster mode
How was this patch tested?
Edit: see apache-spark-on-k8s/spark-integration#38