[SPARK-6766][Streaming] Fix issue about StreamingListenerBatchSubmitted and StreamingListenerBatchStarted #5414

zsxwing · 2015-04-08T07:43:27Z

This PR includes:

Send StreamingListenerBatchSubmitted when JobSet is submitted
Fix StreamingListenerBatchStarted.batchInfo.processingStartTime
Fix a type: completedaBatchInfos -> completedBatchInfos

…reamingListenerBatchStarted.batchInfo.processingStartTime; fix a typo

zsxwing · 2015-04-08T07:45:43Z

cc @tdas

SparkQA · 2015-04-08T07:48:26Z

Test build #29849 has started for PR 5414 at commit 493f978.

jerryshao · 2015-04-08T07:58:02Z

Good catch, LGTM :).

SparkQA · 2015-04-08T09:11:49Z

Test build #29849 has finished for PR 5414 at commit 493f978.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.
This patch does not change any dependencies.

AmplabJenkins · 2015-04-08T09:11:53Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29849/
Test PASSed.

tdas · 2015-04-09T06:24:03Z

streaming/src/main/scala/org/apache/spark/streaming/ui/StreamingJobProgressListener.scala

Good catch. OMG!

tdas · 2015-04-09T07:46:51Z

This looks good to me at first glance, but I am concerned that such a thing went unnoticed for so long despite unit tests in StreamingListenerSuite. Let me think whether we can improve the tests. If so, we definitely should add tests as part of this PR so that such a thing cannot happen again.

tdas · 2015-04-09T07:52:33Z

streaming/src/main/scala/org/apache/spark/streaming/scheduler/JobScheduler.scala

I think this can be refactored to dedup lines 139 and 144.

I think it is hard to dedup cause of ordering call.

Right, handleJobStart will set jobSet.hasStarted to true.

How about

val isFirstJobOfJobSet = !jobSet.hasStarted jobSet.handleJobStart(job) if (isFirstJobOfJobSet) { listenerBus.post(StreamingListenerBatchStarted(jobSet.toBatchInfo)) }

Good idea. Updated.

tdas · 2015-04-09T08:02:40Z

Overall, I realized that there is not tests for StreamingJobProgressListener, which is my bad. :( We really need to add some tests for the StreamingJobProgressListener which tests some basic aspects like this. Could you add a StreamingJobProgressListenerSuite ?

zsxwing · 2015-04-09T08:04:04Z

Could you add a StreamingJobProgressListenerSuite ?

OK. I will add some basic tests for StreamingJobProgressListener

…tener

zsxwing · 2015-04-09T09:27:56Z

Added some tests to StreamingListenerSuite for this issue, and added StreamingJobProgressListenerSuite for some basic tests of StreamingJobProgressListener.

SparkQA · 2015-04-09T09:28:34Z

Test build #29935 has started for PR 5414 at commit 79b4fed.

tdas · 2015-04-09T09:40:30Z

streaming/src/test/scala/org/apache/spark/streaming/StreamingListenerSuite.scala

How long does these two units tests take? I wonder whether it is best (in the interest of minimizing test run times) to collapse all these two unit tests into the earlier "batch info reporting".

[info] StreamingListenerSuite:
[info] - batch info reporting (4 seconds, 976 milliseconds)
[info] - receiver info reporting (847 milliseconds)
[info] - SPARK-6766: batch info should be submitted (667 milliseconds)
[info] - SPARK-6766: processingStartTime of batch info should not be None when starting (691 milliseconds)
[info] ScalaTest
[info] Run completed in 8 seconds, 996 milliseconds.
[info] Total number of tests run: 4
[info] Suites: completed 1, aborted 0
[info] Tests: succeeded 4, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed.

This was my last test result.

Hmm, each of the two new unit tests is taking .7 seconds. Not good. Best to merge them into the first unit test.
The reason its taking time is that its starting a new SparkContext every time, and running batches after that.

SparkQA · 2015-04-09T11:28:36Z

Test build #29935 timed out for PR 5414 at commit 79b4fed after a configured wait of 120m.

AmplabJenkins · 2015-04-09T11:28:40Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29935/
Test FAILed.

zsxwing · 2015-04-09T12:26:24Z

retest this please

SparkQA · 2015-04-09T12:28:15Z

Test build #29937 has started for PR 5414 at commit 79b4fed.

SparkQA · 2015-04-09T14:16:36Z

Test build #29937 has finished for PR 5414 at commit 79b4fed.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.
This patch does not change any dependencies.

AmplabJenkins · 2015-04-09T14:16:40Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29937/
Test PASSed.

SparkQA · 2015-04-09T23:48:30Z

Test build #29985 has started for PR 5414 at commit ca0955b.

zsxwing · 2015-04-09T23:49:37Z

Collapsed the new unit tests together.

SparkQA · 2015-04-10T01:07:09Z

Test build #29985 has finished for PR 5414 at commit ca0955b.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.
This patch does not change any dependencies.

AmplabJenkins · 2015-04-10T01:07:13Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29985/
Test PASSed.

tdas · 2015-04-10T04:30:35Z

streaming/src/test/scala/org/apache/spark/streaming/StreamingListenerSuite.scala

Why this line? Run streams should have already stopped the ssc, isnt it?

Sorry that I misread runStreams. This line is not necessary.

tdas · 2015-04-10T05:34:41Z

Just a few minor comments, otherwise LGTM. Can merge as soon as you update these.

SparkQA · 2015-04-10T07:48:27Z

Test build #30019 has started for PR 5414 at commit 2f85060.

tdas · 2015-04-10T08:51:08Z

The concerned testsuites have passed, I am merging this to master and 1.3 branch. Thanks!

tdas · 2015-04-10T08:54:30Z

I merged it in master, but could not merge it to 1.3 because of conflicts. Could you open another PR to branch 1.3? Would be good to have these bug fixes in 1.3.x

SparkQA · 2015-04-10T09:17:25Z

Test build #30019 has finished for PR 5414 at commit 2f85060.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.
This patch does not change any dependencies.

AmplabJenkins · 2015-04-10T09:17:30Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/30019/
Test PASSed.

…ed and StreamingListenerBatchStarted (backport to branch 1.3) Backport SPARK-6766 #5414 to branch 1.3 Conflicts: streaming/src/main/scala/org/apache/spark/streaming/ui/StreamingJobProgressListener.scala Author: zsxwing <zsxwing@gmail.com> Closes #5452 from zsxwing/SPARK-6766-branch-1.3 and squashes the following commits: cb87e44 [zsxwing] [SPARK-6766][Streaming] Fix issue about StreamingListenerBatchSubmitted and StreamingListenerBatchStarted (backport to branch 1.3)

…tches" lists to StreamingPage This PR adds two lists, `Active Batches` and `Completed Batches`. Here is the screenshot: ![batch_list](https://cloud.githubusercontent.com/assets/1000778/7060458/d8898572-deb3-11e4-938b-6f8602c71a9f.png) Due to [SPARK-6766](https://issues.apache.org/jira/browse/SPARK-6766), I need to merge #5414 in my local machine to get the above screenshot. Author: zsxwing <zsxwing@gmail.com> Closes #5434 from zsxwing/SPARK-6796 and squashes the following commits: be50fc6 [zsxwing] Fix the code style 51b792e [zsxwing] Fix the unit test 6f3078e [zsxwing] Make 'startTime' readable f40e0a9 [zsxwing] Merge branch 'master' into SPARK-6796 2525336 [zsxwing] Rename 'Processed batches' and 'Waiting batches' and also add links a69c091 [zsxwing] Show the number of total completed batches too a12ad7b [zsxwing] Change 'records' to 'events' in the UI 86b5e7f [zsxwing] Make BatchTableBase abstract b248787 [zsxwing] Add tests to verify the new tables d18ab7d [zsxwing] Fix the code style 6ceffb3 [zsxwing] Add "Active Batches" and "Completed Batches" lists to StreamingPage

Send StreamingListenerBatchSubmitted when JobSet is submitted; fix St…

493f978

…reamingListenerBatchStarted.batchInfo.processingStartTime; fix a typo

zsxwing mentioned this pull request Apr 9, 2015

[SPARK-6796][Streaming][WebUI] Add "Active Batches" and "Completed Batches" lists to StreamingPage #5434

Closed

tdas reviewed Apr 9, 2015
View reviewed changes

streaming/src/main/scala/org/apache/spark/streaming/ui/StreamingJobProgressListener.scala

Copy link

Contributor

tdas Apr 9, 2015

Choose a reason for hiding this comment

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

Good catch. OMG!

tdas reviewed Apr 9, 2015
View reviewed changes

zsxwing added 3 commits April 9, 2015 16:17

Refactor as per TD's suggestion

74aed99

Add unit tests for SPARK-6766

fc3a2a1

Add StreamingJobProgressListenerSuite to test StreamingJobProgressLis…

79b4fed

…tener

tdas reviewed Apr 9, 2015
View reviewed changes

Combine unit tests

ca0955b

tdas reviewed Apr 10, 2015
View reviewed changes

Update tests

2f85060

asfgit closed this in 18ca089 Apr 10, 2015

zsxwing mentioned this pull request Apr 10, 2015

[SPARK-6766][Streaming] Fix issue about StreamingListenerBatchSubmitted and StreamingListenerBatchStarted (backport to branch 1.3) #5452

Closed

zsxwing deleted the SPARK-6766 branch April 10, 2015 09:28

[SPARK-6766][Streaming] Fix issue about StreamingListenerBatchSubmitted and StreamingListenerBatchStarted #5414

[SPARK-6766][Streaming] Fix issue about StreamingListenerBatchSubmitted and StreamingListenerBatchStarted #5414

Uh oh!

Conversation

zsxwing commented Apr 8, 2015

Uh oh!

zsxwing commented Apr 8, 2015

Uh oh!

SparkQA commented Apr 8, 2015

Uh oh!

jerryshao commented Apr 8, 2015

Uh oh!

SparkQA commented Apr 8, 2015

Uh oh!

AmplabJenkins commented Apr 8, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tdas commented Apr 9, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tdas commented Apr 9, 2015

Uh oh!

zsxwing commented Apr 9, 2015

Uh oh!

zsxwing commented Apr 9, 2015

Uh oh!

SparkQA commented Apr 9, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Apr 9, 2015

Uh oh!

AmplabJenkins commented Apr 9, 2015

Uh oh!

zsxwing commented Apr 9, 2015

Uh oh!

SparkQA commented Apr 9, 2015

Uh oh!

SparkQA commented Apr 9, 2015

Uh oh!

AmplabJenkins commented Apr 9, 2015

Uh oh!

SparkQA commented Apr 9, 2015

Uh oh!

zsxwing commented Apr 9, 2015

Uh oh!

SparkQA commented Apr 10, 2015

Uh oh!

AmplabJenkins commented Apr 10, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tdas commented Apr 10, 2015

Uh oh!

SparkQA commented Apr 10, 2015

Uh oh!

tdas commented Apr 10, 2015

Uh oh!

tdas commented Apr 10, 2015

Uh oh!

SparkQA commented Apr 10, 2015

Uh oh!