[SPARK-28947][K8S] Status logging not happens at an interval for liveness by yaooqinn · Pull Request #25648 · apache/spark

yaooqinn · 2019-09-02T07:28:20Z

What changes were proposed in this pull request?

This pr invoke the start method of LoggingPodStatusWatcherImpl for status logging at intervals.

Why are the changes needed?

This pr invoke the start method of LoggingPodStatusWatcherImpl is declared but never called

Does this PR introduce any user-facing change?

no

How was this patch tested?

manually test

…ness

SparkQA · 2019-09-02T07:44:34Z

Test build #110003 has finished for PR 25648 at commit 2de2db7.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2019-09-02T07:48:10Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/15023/

SparkQA · 2019-09-02T08:04:06Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/15023/

felixcheung · 2019-09-02T23:03:49Z

@mccheah

SparkQA · 2019-09-16T18:30:32Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/15640/

SparkQA · 2019-09-16T18:53:27Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/15640/

...tes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesClientApplication.scala

SparkQA · 2019-09-27T11:50:09Z

Test build #111480 has finished for PR 25648 at commit 8050b94.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2019-09-27T12:22:14Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/16509/

SparkQA · 2019-09-27T12:38:50Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/16509/

...tes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesClientApplication.scala

vanzin · 2019-09-27T18:42:42Z

...managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/submit/ClientSuite.scala

  }

  test("Waiting for app completion should stall on the watcher") {
+    kconf.sparkConf.set(WAIT_FOR_APP_COMPLETION, true)


This isn't technically needed, since watchOrStop will be called regardless, right?

I guess this is needed because it won't stall when false

Sorry, not following. The only code that "stalls" is LoggingPodStatusWatcherImpl, and this test is using a mock, not that implementation. So what exactly is using this config?

BTW that question applies to all changes you're making in this class.

I see, thanks

...ernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/LoggingPodStatusWatcher.scala

...managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/KubernetesTestConf.scala

...ernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/LoggingPodStatusWatcher.scala

SparkQA · 2019-09-28T01:42:12Z

Test build #111511 has finished for PR 25648 at commit 92306f8.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

yaooqinn · 2019-09-28T03:12:48Z

retest this please

SparkQA · 2019-09-28T03:27:43Z

Test build #111515 has finished for PR 25648 at commit 92306f8.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

yaooqinn · 2019-09-29T01:59:24Z

retest this please

SparkQA · 2019-09-29T02:13:45Z

Test build #111548 has finished for PR 25648 at commit 92306f8.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2019-09-29T02:36:44Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/16560/

SparkQA · 2019-09-29T02:53:51Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/16560/

yaooqinn · 2019-10-10T03:17:41Z

retest this please

SparkQA · 2019-10-11T22:35:14Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/16944/

SparkQA · 2019-10-11T22:55:50Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/16944/

yaooqinn · 2019-10-14T02:31:17Z

retest this please

SparkQA · 2019-10-14T02:43:43Z

Test build #112003 has finished for PR 25648 at commit f8eeaae.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2019-10-14T03:18:22Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/17015/

SparkQA · 2019-10-14T03:36:59Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/17015/

vanzin · 2019-10-14T19:42:48Z

...ernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/LoggingPodStatusWatcher.scala

+    logInfo(s"Waiting for application ${conf.appName} with submission ID $sId to finish...")
+    val interval = conf.get(REPORT_INTERVAL)
+    while (!podCompleted) {
+      Thread.sleep(interval)


Instead of this you should be using something like wait/notifyAll so you can actually wakeup this thread when needed.

SparkQA · 2019-10-15T06:15:34Z

Test build #112088 has finished for PR 25648 at commit 232fc22.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2019-10-15T06:46:42Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/17087/

SparkQA · 2019-10-15T07:10:19Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/17087/

yaooqinn · 2019-10-15T07:17:55Z

retest this please

SparkQA · 2019-10-15T07:32:04Z

Test build #112095 has finished for PR 25648 at commit 232fc22.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2019-10-15T08:03:45Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/17094/

SparkQA · 2019-10-15T08:26:41Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/17094/

erikerlandson · 2019-10-15T18:51:54Z

@yaooqinn @vanzin LGTM

vanzin · 2019-10-15T19:33:38Z

Merging to master.

[SPARK-28947][K8S] Status logging not happens at an interval for live…

2de2db7

…ness

dongjoon-hyun added the KUBERNETES label Sep 2, 2019

felixcheung requested a review from erikerlandson September 2, 2019 23:02

vanzin reviewed Sep 26, 2019

View reviewed changes

...tes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesClientApplication.scala Outdated Show resolved Hide resolved

determine log pod status inside the watcher

8050b94

vanzin reviewed Sep 27, 2019

View reviewed changes

erikerlandson reviewed Sep 27, 2019

View reviewed changes

...ernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/LoggingPodStatusWatcher.scala Outdated Show resolved Hide resolved

nit

92306f8

using while loop instead of daemon thread

f8eeaae

vanzin reviewed Oct 14, 2019

View reviewed changes

using wait/notifyAll instead of thread sleep

232fc22

vanzin closed this in 02c5b4f Oct 15, 2019

Conversation

yaooqinn commented Sep 2, 2019

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

SparkQA commented Sep 2, 2019

Uh oh!

SparkQA commented Sep 2, 2019

Uh oh!

SparkQA commented Sep 2, 2019

Uh oh!

felixcheung commented Sep 2, 2019

Uh oh!

SparkQA commented Sep 16, 2019

Uh oh!

SparkQA commented Sep 16, 2019

Uh oh!

Uh oh!

SparkQA commented Sep 27, 2019

Uh oh!

SparkQA commented Sep 27, 2019

Uh oh!

SparkQA commented Sep 27, 2019

Uh oh!

Uh oh!

vanzin Sep 27, 2019

Choose a reason for hiding this comment

Uh oh!

yaooqinn Sep 28, 2019

Choose a reason for hiding this comment

Uh oh!

vanzin Sep 30, 2019

Choose a reason for hiding this comment

Uh oh!

yaooqinn Oct 9, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SparkQA commented Sep 28, 2019

Uh oh!

yaooqinn commented Sep 28, 2019

Uh oh!

SparkQA commented Sep 28, 2019

Uh oh!

yaooqinn commented Sep 29, 2019

Uh oh!

SparkQA commented Sep 29, 2019

Uh oh!

SparkQA commented Sep 29, 2019

Uh oh!

SparkQA commented Sep 29, 2019

Uh oh!

yaooqinn commented Oct 10, 2019

Uh oh!

SparkQA commented Oct 11, 2019

Uh oh!

SparkQA commented Oct 11, 2019

Uh oh!

yaooqinn commented Oct 14, 2019

Uh oh!

SparkQA commented Oct 14, 2019

Uh oh!

SparkQA commented Oct 14, 2019

Uh oh!

SparkQA commented Oct 14, 2019

Uh oh!

vanzin Oct 14, 2019

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Oct 15, 2019

Uh oh!

SparkQA commented Oct 15, 2019

Uh oh!

SparkQA commented Oct 15, 2019

Uh oh!

yaooqinn commented Oct 15, 2019