[SPARK-3957]: show broadcast variable resource usage info in UI #2851

CodingCat · 2014-10-20T03:12:49Z

show the broadcast variables resource usage in UI

SparkQA · 2014-10-20T03:19:48Z

QA tests have started for PR 2851 at commit 8f75476.

This patch merges cleanly.

SparkQA · 2014-10-20T03:20:54Z

QA tests have finished for PR 2851 at commit 8f75476.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- sealed abstract class BlockId extends Serializable

shivaram · 2014-10-20T16:42:59Z

core/src/main/scala/org/apache/spark/HeartbeatReceiver.scala

Would this send a BlockStatus for each broadcast variable on a heartbeat ? If we have hundreds or thousands of broadcast variables I wonder if the message size will become huge. Could we send deltas somehow ?

good point, I will make this PR this evening

CodingCat · 2014-10-22T05:51:18Z

SparkQA · 2014-10-22T06:04:46Z

QA tests have started for PR 2851 at commit c07c306.

This patch merges cleanly.

SparkQA · 2014-10-22T06:19:45Z

QA tests have started for PR 2851 at commit 1eff69e.

This patch merges cleanly.

SparkQA · 2014-10-22T06:42:26Z

QA tests have finished for PR 2851 at commit c07c306.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- sealed abstract class BlockId extends Serializable

SparkQA · 2014-10-22T06:54:17Z

QA tests have finished for PR 2851 at commit 1eff69e.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- throw new SparkException("Failed to load class to register with Kryo", e)
- sealed abstract class BlockId extends Serializable

rxin · 2014-10-22T07:49:17Z

Can't we just have a general block reporting mechanism rather than special casing for broadcast?

SparkQA · 2014-10-22T12:04:45Z

QA tests have started for PR 2851 at commit 473519a.

This patch merges cleanly.

CodingCat · 2014-10-22T12:13:41Z

@rxin I'm not sure, maybe we don't need that, because currently all RDD blocks are not reported via network, but by calling post(...) from the driver

SparkQA · 2014-10-22T12:38:25Z

QA tests have finished for PR 2851 at commit 473519a.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- sealed abstract class BlockId extends Serializable

SparkQA · 2014-10-22T13:04:47Z

QA tests have started for PR 2851 at commit 23b1819.

This patch merges cleanly.

SparkQA · 2014-10-22T13:38:32Z

QA tests have finished for PR 2851 at commit 23b1819.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- sealed abstract class BlockId extends Serializable

SparkQA · 2014-10-22T13:49:51Z

QA tests have started for PR 2851 at commit 2db9dd9.

This patch merges cleanly.

SparkQA · 2014-10-22T15:00:39Z

QA tests have finished for PR 2851 at commit 2db9dd9.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- sealed abstract class BlockId extends Serializable

CodingCat · 2014-10-23T20:59:53Z

@andrewor14 do you want to take a look at this patch?

rxin · 2014-10-23T21:46:48Z

Hi @CodingCat - This is a great & handy feature, but what I meant was that we should NOT have custom code that tracks broadcast blocks. We can have special UIs for reporting broadcast blocks for convenience, but the underlying reporting mechanism should be the same across all block types. BlockManager and the associated code are complicated enough (pretty terribly designed by us already), and we should not further complicate it by adding custom code for x, y, and z. We should make this reporting general enough (or use existing general reporting code path) so we can easily report other blocks in the future.

CodingCat · 2014-10-23T21:54:41Z

@rxin, I see

then I will try to refactor the reporting mechanism (currently piggyback in heartbeat) to make it more general....

shivaram · 2014-10-23T22:00:14Z

@rxin - Here is a simpler design -- What if we report all broadcast blocks to the master when they are added to a block manager as well (tellMaster = true instead of false in https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/broadcast/TorrentBroadcast.scala#L95 )

It might be the same network traffic as any reporting mechanism we come up with -- The only concern then is to make sure that we don't wait for the master RPC, and we could do that if tellMaster is asynchronous ?

shivaram · 2014-10-23T22:02:32Z

Sorry that link should have been https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/broadcast/TorrentBroadcast.scala#L181

CodingCat · 2014-10-23T22:25:12Z

Hi, @shivaram, do you mean we send the report with tell instead of askDriverWithReply....hmmm...

what's the original motivation to send BlockInfo "synchronously"?

CodingCat · 2014-10-23T22:28:01Z

I mean Akka's tell, not

 private def tell(message: Any) {
    if (!askDriverWithReply[Boolean](message)) {
      throw new SparkException("BlockManagerMasterActor returned false, expected true.")
    }
  }

(.....why we have such a method....)

CodingCat · 2014-10-30T22:29:54Z

haven't forgotten this, I will make it done tomorrow

SparkQA · 2014-11-02T20:03:13Z

Test build #22766 has finished for PR 2851 at commit 6e4915d.

This patch fails Scala style tests.
This patch does not merge cleanly.
This patch adds the following public classes (experimental):
- case class SparkListenerBlockUpdate(blockManagerId: BlockManagerId, blockId: BlockId,
- class DefaultSparkListenerEventFilter
- sealed abstract class BlockId extends Serializable
- class BroadcastInfo(

CodingCat · 2015-03-12T21:00:17Z

Hi, @squito , would you mind taking further review when you have time?

squito · 2015-03-13T14:51:58Z

core/src/main/scala/org/apache/spark/HeartbeatReceiver.scala

did you mean to remove the update of executorLastSeen? seems like maybe a mistake in the merge?

squito · 2015-03-13T15:32:57Z

Hi @CodingCat , I took another look through everything. My comments are all very minor. But I think I'd still like to get some more thoughts from others on how the broadcast block info is passed along. As I mentioned earlier, it is also possible to get it into TaskEnd events, like the RDD blocks. I'm torn, so I'd like to hear what others think. Sorry it is taking so long to get feedback ...

squito · 2015-03-13T15:36:34Z

core/src/main/scala/org/apache/spark/ui/storage/BroadcastPage.scala

by worker, not by block

SparkQA · 2015-03-13T20:03:10Z

Test build #28577 has finished for PR 2851 at commit 26977c9.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2015-03-13T22:08:20Z

Test build #28585 has finished for PR 2851 at commit 49ee118.

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

SparkQA · 2015-03-14T02:14:00Z

Test build #28601 has finished for PR 2851 at commit 9b1bccf.

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

CodingCat · 2015-03-14T12:02:45Z

Hi, @squito , thanks for the comments, I made some revision on the patch

squito · 2015-03-14T13:36:04Z

thanks for the updates! just one small style comment.
FWIW, after thinking about it more, I am in favor of your approach of sending the block updates, just want to get some confirmation.

lgtm (with the minor style update)

SparkQA · 2015-03-15T14:11:00Z

Test build #28626 has finished for PR 2851 at commit 1aec3a8.

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

CodingCat · 2015-03-15T14:12:13Z

thanks @squito

I updated that again

JoshRosen · 2015-10-16T20:08:49Z

Hey @CodingCat, it looks like this PR is significantly out of date. Do you mind closing it for now? You can always re-open / re-submit once we're ready to work on this feature again. Thanks!

CodingCat · 2015-10-16T23:59:51Z

sure, I will close this temporarily

shivaram reviewed Oct 20, 2014
View reviewed changes

shivaram mentioned this pull request Oct 21, 2014

[SPARK-3958] TorrentBroadcast cleanup / debugging improvements. #2844

Closed

CodingCat changed the title ~~[WIP]SPARK-3957: show broadcast variable resource usage info in UI~~ [SPARK-3957]: show broadcast variable resource usage info in UI Oct 22, 2014

CodingCat force-pushed the SPARK-3957 branch from c07c306 to 1eff69e Compare October 22, 2014 06:14

CodingCat force-pushed the SPARK-3957 branch from 1eff69e to 473519a Compare October 22, 2014 11:59

CodingCat force-pushed the SPARK-3957 branch from 2db9dd9 to 6e4915d Compare November 2, 2014 19:55

squito reviewed Mar 13, 2015
View reviewed changes

core/src/main/scala/org/apache/spark/ui/storage/BroadcastPage.scala Outdated

Copy link

Contributor

squito Mar 13, 2015

Choose a reason for hiding this comment

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

by worker, not by block

address comments except merging case one

26977c9

display storage level of broadcast partitions

49ee118

revise the stuffs

9b1bccf

stylistic fix

1aec3a8

CodingCat closed this Oct 16, 2015

[SPARK-3957]: show broadcast variable resource usage info in UI #2851

[SPARK-3957]: show broadcast variable resource usage info in UI #2851

Uh oh!

Conversation

CodingCat commented Oct 20, 2014

Uh oh!

SparkQA commented Oct 20, 2014

Uh oh!

SparkQA commented Oct 20, 2014

Uh oh!

shivaram Oct 20, 2014

Choose a reason for hiding this comment

Uh oh!

CodingCat Oct 21, 2014

Choose a reason for hiding this comment

Uh oh!

CodingCat commented Oct 22, 2014

Uh oh!

SparkQA commented Oct 22, 2014

Uh oh!

SparkQA commented Oct 22, 2014

Uh oh!

SparkQA commented Oct 22, 2014

Uh oh!

SparkQA commented Oct 22, 2014

Uh oh!

rxin commented Oct 22, 2014

Uh oh!

SparkQA commented Oct 22, 2014

Uh oh!

CodingCat commented Oct 22, 2014

Uh oh!

SparkQA commented Oct 22, 2014

Uh oh!

SparkQA commented Oct 22, 2014

Uh oh!

SparkQA commented Oct 22, 2014

Uh oh!

SparkQA commented Oct 22, 2014

Uh oh!

SparkQA commented Oct 22, 2014

Uh oh!

CodingCat commented Oct 23, 2014

Uh oh!

rxin commented Oct 23, 2014

Uh oh!

CodingCat commented Oct 23, 2014

Uh oh!

shivaram commented Oct 23, 2014

Uh oh!

shivaram commented Oct 23, 2014

Uh oh!

CodingCat commented Oct 23, 2014

Uh oh!

CodingCat commented Oct 23, 2014

Uh oh!

CodingCat commented Oct 30, 2014

Uh oh!

SparkQA commented Nov 2, 2014

Uh oh!

CodingCat commented Mar 12, 2015

Uh oh!

squito Mar 13, 2015

Choose a reason for hiding this comment

Uh oh!

squito commented Mar 13, 2015

Uh oh!

squito Mar 13, 2015

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Mar 13, 2015

Uh oh!

SparkQA commented Mar 13, 2015

Uh oh!

SparkQA commented Mar 14, 2015

Uh oh!

CodingCat commented Mar 14, 2015

Uh oh!

squito commented Mar 14, 2015

Uh oh!