[SPARK-4072][Core]Display Streaming blocks in Streaming UI #6672

zsxwing · 2015-06-05T15:34:14Z

Replace #6634

This PR adds SparkListenerBlockUpdated to SparkListener so that it can monitor all block update infos that are sent to BlockManagerMasaterEndpoint, and also add new tables in the Storage tab to display the stream block infos.

zsxwing · 2015-06-05T15:34:23Z

cc @tdas

SparkQA · 2015-06-05T17:20:27Z

Test build #34281 has finished for PR 6672 at commit 797ee4b.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class SparkListenerBlockUpdated(updateBlockInfo: UpdateBlockInfo) extends SparkListenerEvent
- class StorageListener(storageStatusListener: StorageStatusListener)

tdas · 2015-06-05T23:06:24Z

Awesome! Screenshot looking pretty good. Here are some nit picky comments.

RDD, Receiver Blocks, these titles are same size as "Storage" at the top. Should be smaller.
Address / Exec ID format is little confusing. Might be a good idea to ignore the exec ID. If some one needs to relate it to the executor, they can use the hostname to find it themselves on the executors page.
How will it look if one of the 2 copies of the blocks fall off from the executor?

We should not drop blocks from the UI, because the Storage page is meant to be a true representation of the what is going on in the cluster. Instead we could make multiple collapsible sections, one for each input stream
Let's not put the broadcast blocks in the page yet, because we are not even sure whether its worth putting those blocks, whether people even need to debug that. And whether showing the individual pieces is worth it or not. Lets keep those questions outside the scope of this PR, and address them in a separate PR.

zsxwing · 2015-06-05T23:35:50Z

core/src/main/scala/org/apache/spark/storage/BlockStatusListener.scala

How will it look if one of the 2 copies of the blocks fall off from the executor?

2x will become 1x. When one of 2 copies is removed, an update info with StorageLevel.None will be sent. Then removeBlockFromBlockManager will create a new StorageLevel with replication = 1 for this block.

That's cool then. Will take a more detailed look at the code soon.

On Fri, Jun 5, 2015 at 4:36 PM, Shixiong Zhu notifications@github.com
wrote:

In core/src/main/scala/org/apache/spark/storage/BlockStatusListener.scala
#6672 (comment):

useOffHeap = externalBlockStoreSize > 0,

deserialized = storageLevel.deserialized,

replication = newLocations.size

)

blocks.put(blockId,

BlockUIData(

blockId,

newStorageLevel,

memSize,

diskSize,

externalBlockStoreSize,

newLocations))

} else {

// If isValid is not true, it means we should drop the block.

blocksInBlockManager -= blockId

removeBlockFromBlockManager(blockId, blockManagerId)

How will it look if one of the 2 copies of the blocks fall off from the
executor?

2x will become 1x. When one of 2 copies is removed, an update info with
StorageLevel.None will be sent. Then removeBlockFromBlockManager will
create a new StorageLevel with replication = 1 for this block.

—
Reply to this email directly or view it on GitHub
https://github.com/apache/spark/pull/6672/files#r31860010.

zsxwing · 2015-06-06T14:37:25Z

New screenshot

SparkQA · 2015-06-06T16:22:20Z

Test build #34365 has finished for PR 6672 at commit 80f6c6d.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class SparkListenerBlockUpdated(updateBlockInfo: UpdateBlockInfo) extends SparkListenerEvent
- class StorageListener(storageStatusListener: StorageStatusListener)

SparkQA · 2015-06-08T09:16:45Z

Test build #34418 has finished for PR 6672 at commit 2baa161.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class SparkListenerBlockUpdated(updateBlockInfo: UpdateBlockInfo) extends SparkListenerEvent
- class StorageListener(storageStatusListener: StorageStatusListener)

zsxwing · 2015-06-08T14:25:31Z

retest this please

SparkQA · 2015-06-08T16:13:57Z

Test build #34437 has finished for PR 6672 at commit 2baa161.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class SparkListenerBlockUpdated(updateBlockInfo: UpdateBlockInfo) extends SparkListenerEvent
- class StorageListener(storageStatusListener: StorageStatusListener)

tdas · 2015-06-18T02:21:48Z

@JoshRosen Can you take a look at this PR, especially the SparkListener updates.

tdas · 2015-06-20T01:18:27Z

core/src/main/scala/org/apache/spark/scheduler/SparkListener.scala

Isnt UpdateBlockInfo private[spark]? Then its not right that a public class refer to an internal class that people will not be able to refer to.

Right. Just added a new develop class BlockUpdatedInfo. And I also updated JsonProtocol to support the new SparkListenerBlockUpdated.

SparkQA · 2015-06-22T11:31:04Z

Test build #35441 has finished for PR 6672 at commit 0b1e47b.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class SparkListenerBlockUpdated(blockUpdatedInfo: BlockUpdatedInfo) extends SparkListenerEvent
- case class BlockUpdatedInfo(
- class StorageListener(storageStatusListener: StorageStatusListener)

zsxwing · 2015-06-22T13:43:00Z

retest this please

SparkQA · 2015-06-22T15:32:55Z

Test build #35451 has finished for PR 6672 at commit 0b1e47b.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class SparkListenerBlockUpdated(blockUpdatedInfo: BlockUpdatedInfo) extends SparkListenerEvent
- case class BlockUpdatedInfo(
- class StorageListener(storageStatusListener: StorageStatusListener)

zsxwing · 2015-06-22T15:47:09Z

retest this please

SparkQA · 2015-06-22T18:24:40Z

Test build #35458 has finished for PR 6672 at commit 0b1e47b.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class SparkListenerBlockUpdated(blockUpdatedInfo: BlockUpdatedInfo) extends SparkListenerEvent
- case class BlockUpdatedInfo(
- class StorageListener(storageStatusListener: StorageStatusListener)

JoshRosen · 2015-06-23T16:57:28Z

I've noticed that you're not persisting the SparkListenerBlockUpdated events in the event logging listener. Since there might be tons of these events, I can understand why we might not want to persist them in the event log, similar to how we don't persist ExecutorMetricsUpdate events. If this is intentional, I think we should add an override and comment to EventLoggingListener to make this more explicit: https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/scheduler/EventLoggingListener.scala#L201

JoshRosen · 2015-06-23T17:25:41Z

core/src/main/scala/org/apache/spark/util/JsonProtocol.scala

What happens if we get an unknown class name here? That situation might occur if we're parsing a newer event log using an older version of the Spark HistoryServer. This point might be moot if we're not persisting these events to the log (see my earlier comment about EventLoggingListener). If this is safe due to lack of persistence, maybe we should just leave a note here explaining that.

That situation might occur if we're parsing a newer event log using an older version of the Spark HistoryServer.

Good point. But actually, I think JsonProtocol only supports backward-compatibility. JsonProtocol.sparkEventFromJson does not handle unknown class names either. Because ReplayListenerBus will throw an exception if parsing a json event log unsuccessfully, it cannot support forward-compatibility.

BTW. This will throw scala.MatchError for an unknown class name.

It looks like ReplayListenerBus ties to log exceptions except when it receives an IOException (which might be fatal from its perspective). AFAIK it will try to keep going when encountering unknown events, but I don't know that there's a test for this. I'll investigate.

ReplayListenerBus will log exceptions except IOException. But it also skips the rest content in the file. BTW, scala.MatchError is a RuntimeException.

tdas · 2015-07-07T02:16:03Z

I see that you have changed the data structures to suit the desired table structure. But since the table structure has to change, i guess the data structures have to change. So I will not take a look at that right now.

SparkQA · 2015-07-07T03:16:24Z

Test build #36627 has finished for PR 6672 at commit 3de2762.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class SparkListenerBlockUpdated(blockUpdatedInfo: BlockUpdatedInfo) extends SparkListenerEvent
- case class BlockUpdatedInfo(
- class StorageListener(storageStatusListener: StorageStatusListener) extends BlockStatusListener

zsxwing · 2015-07-07T14:01:01Z

Updated as per your suggestion:

SparkQA · 2015-07-07T16:06:47Z

Test build #36680 has finished for PR 6672 at commit ccbee07.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class SparkListenerBlockUpdated(blockUpdatedInfo: BlockUpdatedInfo) extends SparkListenerEvent
- case class BlockUpdatedInfo(
- class StorageListener(storageStatusListener: StorageStatusListener) extends BlockStatusListener

tdas · 2015-07-09T08:34:36Z

core/src/main/scala/org/apache/spark/storage/BlockStatusListener.scala

These public functions can get called at different times leading to inconsistent values in the UI. Can you define a method that returns both information in a single synchronized call?

zsxwing · 2015-07-09T14:57:58Z

New screenshot:

SparkQA · 2015-07-09T17:04:23Z

Test build #36934 has finished for PR 6672 at commit e29fb53.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class SparkListenerBlockUpdated(blockUpdatedInfo: BlockUpdatedInfo) extends SparkListenerEvent
- case class BlockUpdatedInfo(
- class StorageListener(storageStatusListener: StorageStatusListener) extends BlockStatusListener

andrewor14 · 2015-07-09T19:25:10Z

Screenshot looks great! We can do it separately but maybe we should remove the RDD table if there are no RDD blocks cached.

tdas · 2015-07-09T20:16:36Z

Everything else looks good, but I just realized that there are no UI tests! There should be tests added to StorageTabSuite, isnt it?

zsxwing · 2015-07-10T00:48:36Z

Everything else looks good, but I just realized that there are no UI tests! There should be tests added to StorageTabSuite, isnt it?

I agree that we should do unit tests for UI stuffs. Let me see how to do some unit tests for StorageTabSuite. Actually I think many Spark UI tabs lack unit tests. Spark only has some UISeleniumSuites to test UI. But UISeleniumSuite is actually an integration test rather than a unit test.

zsxwing · 2015-07-10T02:59:09Z

Screenshot looks great! We can do it separately but maybe we should remove the RDD table if there are no RDD blocks cached.

Updated it in this PR.

SparkQA · 2015-07-10T04:04:39Z

Test build #37004 has finished for PR 6672 at commit df2c1d8.

This patch fails MiMa tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class SparkListenerBlockUpdated(blockUpdatedInfo: BlockUpdatedInfo) extends SparkListenerEvent
- case class BlockUpdatedInfo(
- class StorageListener(storageStatusListener: StorageStatusListener) extends BlockStatusListener

zsxwing · 2015-07-10T04:21:18Z

retest this please

SparkQA · 2015-07-10T04:23:32Z

Test build #37009 has finished for PR 6672 at commit df2c1d8.

This patch fails some tests.
This patch merges cleanly.
This patch adds no public classes.

zsxwing · 2015-07-10T05:00:30Z

retest this please

tdas · 2015-07-10T21:26:04Z

Jenkins, test this please.

SparkQA · 2015-07-10T23:46:51Z

Test build #37055 has finished for PR 6672 at commit df2c1d8.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class SparkListenerBlockUpdated(blockUpdatedInfo: BlockUpdatedInfo) extends SparkListenerEvent
- case class BlockUpdatedInfo(
- class StorageListener(storageStatusListener: StorageStatusListener) extends BlockStatusListener

tdas · 2015-07-11T01:47:33Z

Jenkins, test this please.

SparkQA · 2015-07-11T03:53:56Z

Test build #37072 has finished for PR 6672 at commit df2c1d8.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class SparkListenerBlockUpdated(blockUpdatedInfo: BlockUpdatedInfo) extends SparkListenerEvent
- case class BlockUpdatedInfo(
- class StorageListener(storageStatusListener: StorageStatusListener) extends BlockStatusListener

tdas · 2015-07-14T20:06:54Z

LGTM, merging this in master!

Display Streaming blocks in Streaming UI

797ee4b

zsxwing reviewed Jun 5, 2015
View reviewed changes

Address comments

80f6c6d

Add unit tests

2baa161

zsxwing changed the title ~~[SPARK-4072][Core][WIP]Display Streaming blocks in Streaming UI~~ [SPARK-4072][Core]Display Streaming blocks in Streaming UI Jun 18, 2015

tdas reviewed Jun 20, 2015
View reviewed changes

zsxwing added 2 commits June 22, 2015 11:29

Fix the code style

04838a9

Add a developer api BlockUpdatedInfo

0b1e47b

JoshRosen reviewed Jun 23, 2015
View reviewed changes

zsxwing added 5 commits July 7, 2015 20:13

Fix the code style

ca0ab69

Don't categorize by Executor ID

1e9ef52

Merge branch 'master' into SPARK-4072-2

450fad1

Fix the replication level of blocks

6dc42b4

Fix the code style

ccbee07

tdas reviewed Jul 9, 2015
View reviewed changes

Update as per TD's comments

e29fb53

Add unit tests for StoragePage

54d54af

Use xml query to check the xml elements

df2c1d8

asfgit closed this in fb1d06f Jul 14, 2015

zsxwing deleted the SPARK-4072-2 branch July 15, 2015 07:58

[SPARK-4072][Core]Display Streaming blocks in Streaming UI #6672

[SPARK-4072][Core]Display Streaming blocks in Streaming UI #6672

Uh oh!

Conversation

zsxwing commented Jun 5, 2015

Uh oh!

zsxwing commented Jun 5, 2015

Uh oh!

SparkQA commented Jun 5, 2015

Uh oh!

tdas commented Jun 5, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zsxwing commented Jun 6, 2015

Uh oh!

SparkQA commented Jun 6, 2015

Uh oh!

SparkQA commented Jun 8, 2015

Uh oh!

zsxwing commented Jun 8, 2015

Uh oh!

SparkQA commented Jun 8, 2015

Uh oh!

tdas commented Jun 18, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jun 22, 2015

Uh oh!

zsxwing commented Jun 22, 2015

Uh oh!

SparkQA commented Jun 22, 2015

Uh oh!

zsxwing commented Jun 22, 2015

Uh oh!

SparkQA commented Jun 22, 2015

Uh oh!

JoshRosen commented Jun 23, 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 Jul 7, 2015

Uh oh!

SparkQA commented Jul 7, 2015

Uh oh!

zsxwing commented Jul 7, 2015

Uh oh!

SparkQA commented Jul 7, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zsxwing commented Jul 9, 2015

Uh oh!

SparkQA commented Jul 9, 2015

Uh oh!

andrewor14 commented Jul 9, 2015

Uh oh!

tdas commented Jul 9, 2015

Uh oh!

zsxwing commented Jul 10, 2015

Uh oh!

zsxwing commented Jul 10, 2015

Uh oh!

SparkQA commented Jul 10, 2015

Uh oh!

zsxwing commented Jul 10, 2015

Uh oh!