[SPARK-28414][WEBUI] UI updates to show resource info in Standalone #25409

Ngone51 · 2019-08-11T15:46:30Z

What changes were proposed in this pull request?

Since SPARK-27371 has supported GPU-aware resource scheduling in Standalone, this PR adds resources info in Standalone UI.

How was this patch tested?

Updated JsonProtocolSuite and tested manually.

Master page:

Worker page

Application page

dongjoon-hyun · 2019-08-11T17:14:42Z

core/src/main/scala/org/apache/spark/deploy/master/ui/MasterPage.scala

+      .map { case (rName, rInfoArr) =>
+      rName -> rInfoArr.map(_._2).reduce(_ + _)
+    }
+    if (totalInfo.isEmpty) "None" else formatResourcesUsed(totalInfo, usedInfo)


Could you use "" instead of "None"?

updated, thanks.

SparkQA · 2019-08-11T17:25:03Z

Test build #108932 has finished for PR 25409 at commit d039676.

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

SparkQA · 2019-08-12T02:13:15Z

Test build #108944 has finished for PR 25409 at commit cdc171a.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

HyukjinKwon · 2019-08-12T02:59:56Z

core/src/main/scala/org/apache/spark/deploy/worker/Worker.scala

@@ -242,6 +243,22 @@ private[deploy] class Worker(
          System.exit(1)
        }
    }
+    resources.foreach { case (rName, _) =>


nit:

resources.keys.foreach { rName => resourcesUsed(rName) = new ResourceInformation(rName, Array.empty[String]) }

updated, thanks.

SparkQA · 2019-08-12T04:38:36Z

Test build #108945 has finished for PR 25409 at commit fbb4382.

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

Ngone51 · 2019-08-12T14:43:44Z

also cc @tgravescs @jiangxb1987 @mengxr

Ngone51 · 2019-08-12T14:49:19Z

core/src/main/scala/org/apache/spark/resource/ResourceInformation.scala

@@ -39,6 +39,18 @@ class ResourceInformation(
    val name: String,
    val addresses: Array[String]) extends Serializable {

+  def + (other: ResourceInformation): ResourceInformation = {


Note this may generates much more intermediate ResourceInformation objects. I do this because ResourceInformation yet is immutable. It may be better if we could make it be mutable in the future.

like I mention above we don't want to make this mutable since end user facing and I think you should use different class to track it there to be more efficient

gatorsmile · 2019-08-12T16:29:16Z

Also cc @gengliangwang

SparkQA · 2019-08-12T17:09:39Z

Test build #108979 has finished for PR 25409 at commit e0fc69a.

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

tgravescs · 2019-08-15T14:50:14Z

Just looking at the screen shots, It would be nice for the worker detailed page to have the free and used resources like the top level page.

tgravescs · 2019-08-15T14:56:49Z

core/src/main/scala/org/apache/spark/deploy/StandaloneResourceUtils.scala

+
+  // used for UI
+  def formatResourceRequirements(requirements: Seq[ResourceRequirement]): String = {
+    requirements.map { req => s"${req.amount} ${req.resourceName}"}.mkString(", ")


nit just use () and cleanup spacing.

like (5gpu, 10fpga) ?

sorry I wasn't clear I meant replace {} in the map with ().
requirements.map(req => ..) since it all fits on one line

tgravescs · 2019-08-15T15:11:35Z

core/src/main/scala/org/apache/spark/deploy/worker/Worker.scala

@@ -185,6 +185,7 @@ private[deploy] class Worker(

  var coresUsed = 0
  var memoryUsed = 0
+  val resourcesUsed = new HashMap[String, ResourceInformation]()


Yeah I don't think you should use REsourceInformation here to track used since like you said its immutable. We don't want to change that to be mutable because its user facing API. Something more like WorkerResourceInfo would make sense

Oh, thank you @tgravescs for reminding this. I'll update it as you suggested.

tgravescs · 2019-08-15T15:12:24Z

core/src/main/scala/org/apache/spark/resource/ResourceInformation.scala

@@ -39,6 +39,18 @@ class ResourceInformation(
    val name: String,
    val addresses: Array[String]) extends Serializable {

+  def + (other: ResourceInformation): ResourceInformation = {


like I mention above we don't want to make this mutable since end user facing and I think you should use different class to track it there to be more efficient

Ngone51 · 2019-08-17T10:45:06Z

It would be nice for the worker detailed page to have the free and used resources like the top level page.

Sure, will add it.

SparkQA · 2019-08-21T11:31:19Z

Test build #109486 has finished for PR 25409 at commit ce89784.

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

tgravescs · 2019-08-23T13:28:05Z

core/src/main/scala/org/apache/spark/deploy/worker/Worker.scala

+    }
+  }
+
+  private def updateResourcesUsed(deltaInfo: Map[String, ResourceInformation], add: Boolean)


I'd say just create 2 functions for this, when looking at how this gets called its not nearly as clear as just saying addResourcesUsed, removeResourceUsed, user has to figure out what other parameter is.
also shrinks the code as you simply need 2 lines:
deltaInfo.foreach { case (rName, rInfo) =>
resourcesUsed(rName) += resourcesUsed(rName) + rInfo
}

good point!

tgravescs · 2019-08-23T13:55:40Z

core/src/main/scala/org/apache/spark/deploy/master/WorkerInfo.scala

+    val clazz = ct.runtimeClass
+    val rf = {
+      clazz match {
+        case _ if clazz.equals(classOf[MutableResourceInfo]) =>


I get why you are doing this, but this just seems more complex that what should be needed. Usually when you see different return types they have some same base class or trait so you don't actually have to look at the class, although in this case you don't have any input to determine type by either. It ends up being a runtime check instead of compile time check. So I think you should just make 2 functions - 1 to get immutable, 1 to get mutable.
alternatively just get it immutable all the time and change the one place its used as mutable to deal with it

Thanks, and I chosen the alternative one, which seems more simply in WorkerInfo. While, the other one actually needs 4 functions(combinations of [immutable, mutable] * [total, used]), which seems noisy in WorkerInfo from my view.

tgravescs · 2019-08-26T15:12:36Z

core/src/main/scala/org/apache/spark/deploy/StandaloneResourceUtils.scala

+    : Map[String, MutableResourceInfo] = {
+    immutableResources.map { case (rName, rInfo) =>
+      val mutableAddress = new mutable.HashSet[String]()
+      rInfo.addresses.foreach(mutableAddress.add)


this can just be: mutableAddress ++= rInfo.addresses

SparkQA · 2019-08-26T17:18:12Z

Test build #109740 has finished for PR 25409 at commit 1e64a33.

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

SparkQA · 2019-08-26T18:30:00Z

Test build #109746 has finished for PR 25409 at commit 77cd726.

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

tgravescs · 2019-08-26T19:51:05Z

test this please

SparkQA · 2019-08-26T22:18:45Z

Test build #109752 has finished for PR 25409 at commit 77cd726.

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

tgravescs

+1. Thanks @Ngone51

Ngone51 · 2019-08-27T16:03:44Z

Thank you all :)

Ngone51 added 5 commits August 10, 2019 23:47

show resources info in master ui

4d3d0c4

show resources info in Worker page

2225a60

add resource info in JsonProtocol

f6c2376

update resources info in ui

bfd27ea

update JsomProtocolSuite

d039676

dongjoon-hyun reviewed Aug 11, 2019

View reviewed changes

dongjoon-hyun added the WEB UI label Aug 11, 2019

Ngone51 added 2 commits August 12, 2019 10:00

fix ut

fd8be13

None -> empty string

cdc171a

fix error

fbb4382

HyukjinKwon reviewed Aug 12, 2019

View reviewed changes

address comment

e0fc69a

Ngone51 commented Aug 12, 2019

View reviewed changes

tgravescs reviewed Aug 15, 2019

View reviewed changes

Ngone51 added 2 commits August 19, 2019 23:22

{} -> ()

8af7706

address comment

ce89784

tgravescs reviewed Aug 23, 2019

View reviewed changes

Ngone51 added 2 commits August 26, 2019 21:06

split updateResourcesUsed into [add/remove]ResourcesUsed

dd2ecb4

simplify get resoucesInfo

1e64a33

tgravescs reviewed Aug 26, 2019

View reviewed changes

address comment

77cd726

tgravescs approved these changes Aug 27, 2019

View reviewed changes

asfgit closed this in 70f4bbc Aug 27, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-28414][WEBUI] UI updates to show resource info in Standalone #25409

[SPARK-28414][WEBUI] UI updates to show resource info in Standalone #25409

Ngone51 commented Aug 11, 2019 •

edited

Loading

dongjoon-hyun Aug 11, 2019 •

edited

Loading

Ngone51 Aug 12, 2019

SparkQA commented Aug 11, 2019

SparkQA commented Aug 12, 2019

HyukjinKwon Aug 12, 2019

Ngone51 Aug 12, 2019

SparkQA commented Aug 12, 2019

Ngone51 commented Aug 12, 2019

Ngone51 Aug 12, 2019

tgravescs Aug 15, 2019

gatorsmile commented Aug 12, 2019

SparkQA commented Aug 12, 2019

tgravescs commented Aug 15, 2019

tgravescs Aug 15, 2019

Ngone51 Aug 17, 2019

tgravescs Aug 19, 2019

Ngone51 Aug 21, 2019

tgravescs Aug 15, 2019

Ngone51 Aug 17, 2019

tgravescs Aug 15, 2019

Ngone51 commented Aug 17, 2019

SparkQA commented Aug 21, 2019

tgravescs Aug 23, 2019

Ngone51 Aug 26, 2019

tgravescs Aug 23, 2019

Ngone51 Aug 26, 2019

tgravescs Aug 26, 2019

SparkQA commented Aug 26, 2019

SparkQA commented Aug 26, 2019

tgravescs commented Aug 26, 2019

SparkQA commented Aug 26, 2019

tgravescs left a comment

Ngone51 commented Aug 27, 2019

[SPARK-28414][WEBUI] UI updates to show resource info in Standalone #25409

[SPARK-28414][WEBUI] UI updates to show resource info in Standalone #25409

Conversation

Ngone51 commented Aug 11, 2019 • edited Loading

What changes were proposed in this pull request?

How was this patch tested?

dongjoon-hyun Aug 11, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Aug 11, 2019

SparkQA commented Aug 12, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Aug 12, 2019

Ngone51 commented Aug 12, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gatorsmile commented Aug 12, 2019

SparkQA commented Aug 12, 2019

tgravescs commented Aug 15, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ngone51 commented Aug 17, 2019

SparkQA commented Aug 21, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Aug 26, 2019

SparkQA commented Aug 26, 2019

tgravescs commented Aug 26, 2019

SparkQA commented Aug 26, 2019

tgravescs left a comment

Choose a reason for hiding this comment

Ngone51 commented Aug 27, 2019

Ngone51 commented Aug 11, 2019 •

edited

Loading

dongjoon-hyun Aug 11, 2019 •

edited

Loading