[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/13264 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user zsxwing commented on the pull request: https://github.com/apache/spark/pull/13264#issuecomment-221646004 LGTM. Merging to master / 2.0. Thanks! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/13264#issuecomment-221564246 Merged build finished. Test PASSed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/13264#issuecomment-221563986 **[Test build #59274 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59274/consoleFull)** for PR 13264 at commit [`bbd2959`](https://github.com/apache/spark/commit/bbd29594d75ed1e681b2a525aa8a6bad5bed9508). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/13264#issuecomment-221564250 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59274/ Test PASSed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/13264#issuecomment-221539959 **[Test build #59274 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59274/consoleFull)** for PR 13264 at commit [`bbd2959`](https://github.com/apache/spark/commit/bbd29594d75ed1e681b2a525aa8a6bad5bed9508). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user lgieron commented on a diff in the pull request:
https://github.com/apache/spark/pull/13264#discussion_r64552539
--- Diff:
core/src/test/scala/org/apache/spark/ui/storage/StorageTabSuite.scala ---
@@ -179,6 +179,25 @@ class StorageTabSuite extends SparkFunSuite with
BeforeAndAfter {
assert(storageListener.rddInfoList.size === 2)
}
+ test("verify StorageTab still contains a renamed RDD") {
+{
+ val rddInfo = new RDDInfo(0, "original_name", 1, memOnly, Seq(4))
+ val stageInfo0 = new StageInfo(0, 0, "stage0", 1, Seq(rddInfo),
Seq.empty, "details")
+ bus.postToAll(SparkListenerBlockManagerAdded(1L, bm1, 1000L))
+ bus.postToAll(SparkListenerStageSubmitted(stageInfo0))
+ val blockUpdateInfos1 = Seq(BlockUpdatedInfo(bm1, RDDBlockId(0, 1),
memOnly, 100L, 0L))
+ postUpdateBlocks(bus, blockUpdateInfos1)
+ assert(storageListener.rddInfoList.size == 1)
+}
--- End diff --
Done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user lgieron commented on a diff in the pull request:
https://github.com/apache/spark/pull/13264#discussion_r64545642
--- Diff: core/src/main/scala/org/apache/spark/ui/storage/StorageTab.scala
---
@@ -59,7 +59,15 @@ class StorageListener(storageStatusListener:
StorageStatusListener) extends Bloc
override def onStageSubmitted(stageSubmitted:
SparkListenerStageSubmitted): Unit = synchronized {
val rddInfos = stageSubmitted.stageInfo.rddInfos
-rddInfos.foreach { info => _rddInfoMap.getOrElseUpdate(info.id, info) }
+rddInfos.foreach { info =>
--- End diff --
OK I'm changing this. I'm not a fan of slower code in favor of brevity (as
my impression is that, over time, the entire code base just becomes slow and
fixing a couple bottlenecks won't necessarily help things), but it's the in the
general spirit of Scala (brevity over speed) so my one optimization would be
meaningless anyway.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/13264#discussion_r64485134
--- Diff: core/src/main/scala/org/apache/spark/ui/storage/StorageTab.scala
---
@@ -59,7 +59,15 @@ class StorageListener(storageStatusListener:
StorageStatusListener) extends Bloc
override def onStageSubmitted(stageSubmitted:
SparkListenerStageSubmitted): Unit = synchronized {
val rddInfos = stageSubmitted.stageInfo.rddInfos
-rddInfos.foreach { info => _rddInfoMap.getOrElseUpdate(info.id, info) }
+rddInfos.foreach { info =>
+ _rddInfoMap.get(info.id) match {
+case Some(rddInfo) if info.name != rddInfo.name =>
+ rddInfo.name = info.name
+case None =>
+ _rddInfoMap(info.id) = info
+case _ =>
--- End diff --
oh, I didn't see the `if`, yeah then we need it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/13264#discussion_r64459605
--- Diff: core/src/main/scala/org/apache/spark/ui/storage/StorageTab.scala
---
@@ -59,7 +59,15 @@ class StorageListener(storageStatusListener:
StorageStatusListener) extends Bloc
override def onStageSubmitted(stageSubmitted:
SparkListenerStageSubmitted): Unit = synchronized {
val rddInfos = stageSubmitted.stageInfo.rddInfos
-rddInfos.foreach { info => _rddInfoMap.getOrElseUpdate(info.id, info) }
+rddInfos.foreach { info =>
+ _rddInfoMap.get(info.id) match {
+case Some(rddInfo) if info.name != rddInfo.name =>
+ rddInfo.name = info.name
+case None =>
+ _rddInfoMap(info.id) = info
+case _ =>
--- End diff --
@andrewor14 he was saying `case _` is for `case Some(rddInfo) if info.name
== rddInfo.name =>`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/13264#discussion_r64453178
--- Diff: core/src/main/scala/org/apache/spark/ui/storage/StorageTab.scala
---
@@ -59,7 +59,15 @@ class StorageListener(storageStatusListener:
StorageStatusListener) extends Bloc
override def onStageSubmitted(stageSubmitted:
SparkListenerStageSubmitted): Unit = synchronized {
val rddInfos = stageSubmitted.stageInfo.rddInfos
-rddInfos.foreach { info => _rddInfoMap.getOrElseUpdate(info.id, info) }
+rddInfos.foreach { info =>
+ _rddInfoMap.get(info.id) match {
+case Some(rddInfo) if info.name != rddInfo.name =>
+ rddInfo.name = info.name
+case None =>
+ _rddInfoMap(info.id) = info
+case _ =>
--- End diff --
`map.get` returns an Option, which must be either `Some(...)` or `None`.
The case where the `RDDInfo` didn't exist before falls under the `None` case.
We'll never hit `case _ =>`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/13264#discussion_r64451727
--- Diff: core/src/main/scala/org/apache/spark/ui/storage/StorageTab.scala
---
@@ -59,7 +59,15 @@ class StorageListener(storageStatusListener:
StorageStatusListener) extends Bloc
override def onStageSubmitted(stageSubmitted:
SparkListenerStageSubmitted): Unit = synchronized {
val rddInfos = stageSubmitted.stageInfo.rddInfos
-rddInfos.foreach { info => _rddInfoMap.getOrElseUpdate(info.id, info) }
+rddInfos.foreach { info =>
--- End diff --
@lgieron this is not a performance bottleneck. No need to worry about the
performance considering we are using a lot of Scala closures.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user lgieron commented on a diff in the pull request:
https://github.com/apache/spark/pull/13264#discussion_r64448718
--- Diff: core/src/main/scala/org/apache/spark/ui/storage/StorageTab.scala
---
@@ -59,7 +59,15 @@ class StorageListener(storageStatusListener:
StorageStatusListener) extends Bloc
override def onStageSubmitted(stageSubmitted:
SparkListenerStageSubmitted): Unit = synchronized {
val rddInfos = stageSubmitted.stageInfo.rddInfos
-rddInfos.foreach { info => _rddInfoMap.getOrElseUpdate(info.id, info) }
+rddInfos.foreach { info =>
+ _rddInfoMap.get(info.id) match {
+case Some(rddInfo) if info.name != rddInfo.name =>
+ rddInfo.name = info.name
+case None =>
+ _rddInfoMap(info.id) = info
+case _ =>
--- End diff --
If we take out `case _ =>`, then won't have a match for cached rddInfo
whose name wasn't changed - resulting in exception?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user lgieron commented on a diff in the pull request:
https://github.com/apache/spark/pull/13264#discussion_r64446719
--- Diff: core/src/main/scala/org/apache/spark/ui/storage/StorageTab.scala
---
@@ -59,7 +59,15 @@ class StorageListener(storageStatusListener:
StorageStatusListener) extends Bloc
override def onStageSubmitted(stageSubmitted:
SparkListenerStageSubmitted): Unit = synchronized {
val rddInfos = stageSubmitted.stageInfo.rddInfos
-rddInfos.foreach { info => _rddInfoMap.getOrElseUpdate(info.id, info) }
+rddInfos.foreach { info =>
--- End diff --
It's likely to be slower IMO (as it performs a mutation every time, as
opposed to only mutating when needed - the modified cache page will need to
sent back to RAM at some point, resulting in extra traffic on the bus).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/13264#discussion_r6496
--- Diff: core/src/main/scala/org/apache/spark/ui/storage/StorageTab.scala
---
@@ -59,7 +59,15 @@ class StorageListener(storageStatusListener:
StorageStatusListener) extends Bloc
override def onStageSubmitted(stageSubmitted:
SparkListenerStageSubmitted): Unit = synchronized {
val rddInfos = stageSubmitted.stageInfo.rddInfos
-rddInfos.foreach { info => _rddInfoMap.getOrElseUpdate(info.id, info) }
+rddInfos.foreach { info =>
+ _rddInfoMap.get(info.id) match {
+case Some(rddInfo) if info.name != rddInfo.name =>
+ rddInfo.name = info.name
+case None =>
+ _rddInfoMap(info.id) = info
+case _ =>
--- End diff --
we don't need `case _ =>`. If you use @zsxwing's suggestion above then this
comment doesn't matter anymore.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/13264#discussion_r64436937
--- Diff:
core/src/test/scala/org/apache/spark/ui/storage/StorageTabSuite.scala ---
@@ -179,6 +179,25 @@ class StorageTabSuite extends SparkFunSuite with
BeforeAndAfter {
assert(storageListener.rddInfoList.size === 2)
}
+ test("verify StorageTab still contains a renamed RDD") {
+{
+ val rddInfo = new RDDInfo(0, "original_name", 1, memOnly, Seq(4))
+ val stageInfo0 = new StageInfo(0, 0, "stage0", 1, Seq(rddInfo),
Seq.empty, "details")
+ bus.postToAll(SparkListenerBlockManagerAdded(1L, bm1, 1000L))
+ bus.postToAll(SparkListenerStageSubmitted(stageInfo0))
+ val blockUpdateInfos1 = Seq(BlockUpdatedInfo(bm1, RDDBlockId(0, 1),
memOnly, 100L, 0L))
+ postUpdateBlocks(bus, blockUpdateInfos1)
+ assert(storageListener.rddInfoList.size == 1)
+}
--- End diff --
@lgieron seems weird. could you remove `{}`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/13264#discussion_r64436746
--- Diff: core/src/main/scala/org/apache/spark/ui/storage/StorageTab.scala
---
@@ -59,7 +59,15 @@ class StorageListener(storageStatusListener:
StorageStatusListener) extends Bloc
override def onStageSubmitted(stageSubmitted:
SparkListenerStageSubmitted): Unit = synchronized {
val rddInfos = stageSubmitted.stageInfo.rddInfos
-rddInfos.foreach { info => _rddInfoMap.getOrElseUpdate(info.id, info) }
+rddInfos.foreach { info =>
--- End diff --
I see. Actually, you can just use one line code `rddInfos.foreach { info
=> _rddInfoMap.getOrElseUpdate(info.id, info).name = info.name }`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user lgieron commented on a diff in the pull request:
https://github.com/apache/spark/pull/13264#discussion_r64365075
--- Diff:
core/src/test/scala/org/apache/spark/ui/storage/StorageTabSuite.scala ---
@@ -179,6 +179,25 @@ class StorageTabSuite extends SparkFunSuite with
BeforeAndAfter {
assert(storageListener.rddInfoList.size === 2)
}
+ test("verify StorageTab still contains a renamed RDD") {
+{
+ val rddInfo = new RDDInfo(0, "original_name", 1, memOnly, Seq(4))
+ val stageInfo0 = new StageInfo(0, 0, "stage0", 1, Seq(rddInfo),
Seq.empty, "details")
+ bus.postToAll(SparkListenerBlockManagerAdded(1L, bm1, 1000L))
+ bus.postToAll(SparkListenerStageSubmitted(stageInfo0))
+ val blockUpdateInfos1 = Seq(BlockUpdatedInfo(bm1, RDDBlockId(0, 1),
memOnly, 100L, 0L))
+ postUpdateBlocks(bus, blockUpdateInfos1)
+ assert(storageListener.rddInfoList.size == 1)
+}
--- End diff --
Nope, it's only there to increase readability (to limit the scope of
`rddInfo`, `stageInfo0` and `blockUpdateInfos1` which aren't needed later in
the test).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user lgieron commented on a diff in the pull request:
https://github.com/apache/spark/pull/13264#discussion_r64364913
--- Diff: core/src/main/scala/org/apache/spark/ui/storage/StorageTab.scala
---
@@ -59,7 +59,15 @@ class StorageListener(storageStatusListener:
StorageStatusListener) extends Bloc
override def onStageSubmitted(stageSubmitted:
SparkListenerStageSubmitted): Unit = synchronized {
val rddInfos = stageSubmitted.stageInfo.rddInfos
-rddInfos.foreach { info => _rddInfoMap.getOrElseUpdate(info.id, info) }
+rddInfos.foreach { info =>
+ _rddInfoMap.get(info.id) match {
+case Some(rddInfo) if info.name != rddInfo.name =>
+ rddInfo.name = info.name
+case None =>
+ _rddInfoMap(info.id) = info
+case _ =>
--- End diff --
I'm not sure I follow, can you elaborate?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user lgieron commented on a diff in the pull request:
https://github.com/apache/spark/pull/13264#discussion_r64354687
--- Diff: core/src/main/scala/org/apache/spark/ui/storage/StorageTab.scala
---
@@ -59,7 +59,15 @@ class StorageListener(storageStatusListener:
StorageStatusListener) extends Bloc
override def onStageSubmitted(stageSubmitted:
SparkListenerStageSubmitted): Unit = synchronized {
val rddInfos = stageSubmitted.stageInfo.rddInfos
-rddInfos.foreach { info => _rddInfoMap.getOrElseUpdate(info.id, info) }
+rddInfos.foreach { info =>
--- End diff --
@zsxwing That was my first attempt at this bug, but I've run into the
problem described by @andrewor14 - there's a bunch of fields that get mutated
in `onBlockUpdated` and, by just substituting `RDDInfo` object, we'll lose
those changes. (The `BlockUpdated` event isn't triggered after name change, so
we need to make sure we preserve these changed values.)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/13264#discussion_r64315046
--- Diff:
core/src/test/scala/org/apache/spark/ui/storage/StorageTabSuite.scala ---
@@ -179,6 +179,25 @@ class StorageTabSuite extends SparkFunSuite with
BeforeAndAfter {
assert(storageListener.rddInfoList.size === 2)
}
+ test("verify StorageTab still contains a renamed RDD") {
+{
+ val rddInfo = new RDDInfo(0, "original_name", 1, memOnly, Seq(4))
+ val stageInfo0 = new StageInfo(0, 0, "stage0", 1, Seq(rddInfo),
Seq.empty, "details")
+ bus.postToAll(SparkListenerBlockManagerAdded(1L, bm1, 1000L))
+ bus.postToAll(SparkListenerStageSubmitted(stageInfo0))
+ val blockUpdateInfos1 = Seq(BlockUpdatedInfo(bm1, RDDBlockId(0, 1),
memOnly, 100L, 0L))
+ postUpdateBlocks(bus, blockUpdateInfos1)
+ assert(storageListener.rddInfoList.size == 1)
+}
--- End diff --
nit: no need to wrap this in `{ }` right?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/13264#discussion_r64315021
--- Diff: core/src/main/scala/org/apache/spark/ui/storage/StorageTab.scala
---
@@ -59,7 +59,15 @@ class StorageListener(storageStatusListener:
StorageStatusListener) extends Bloc
override def onStageSubmitted(stageSubmitted:
SparkListenerStageSubmitted): Unit = synchronized {
val rddInfos = stageSubmitted.stageInfo.rddInfos
-rddInfos.foreach { info => _rddInfoMap.getOrElseUpdate(info.id, info) }
+rddInfos.foreach { info =>
+ _rddInfoMap.get(info.id) match {
+case Some(rddInfo) if info.name != rddInfo.name =>
+ rddInfo.name = info.name
+case None =>
+ _rddInfoMap(info.id) = info
+case _ =>
--- End diff --
we don't need this case. It can be either `Some` or `None`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/13264#discussion_r64314987
--- Diff: core/src/main/scala/org/apache/spark/ui/storage/StorageTab.scala
---
@@ -59,7 +59,15 @@ class StorageListener(storageStatusListener:
StorageStatusListener) extends Bloc
override def onStageSubmitted(stageSubmitted:
SparkListenerStageSubmitted): Unit = synchronized {
val rddInfos = stageSubmitted.stageInfo.rddInfos
-rddInfos.foreach { info => _rddInfoMap.getOrElseUpdate(info.id, info) }
+rddInfos.foreach { info =>
--- End diff --
Hm I'm not so sure if it's safe to just replace it. We mutate the RDD Infos
in `onBlockUpdate`, where the cached information is most update to date. I'm
not sure if there is an ordering between `StageSubmitted` and `BlockUpdated`
that can cause the cached information to be overridden.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/13264#discussion_r64312020
--- Diff: core/src/main/scala/org/apache/spark/ui/storage/StorageTab.scala
---
@@ -59,7 +59,15 @@ class StorageListener(storageStatusListener:
StorageStatusListener) extends Bloc
override def onStageSubmitted(stageSubmitted:
SparkListenerStageSubmitted): Unit = synchronized {
val rddInfos = stageSubmitted.stageInfo.rddInfos
-rddInfos.foreach { info => _rddInfoMap.getOrElseUpdate(info.id, info) }
+rddInfos.foreach { info =>
--- End diff --
@andrewor14 why not just replace the old info with the new one?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/13264#issuecomment-221119529 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59150/ Test PASSed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/13264#issuecomment-221119527 Merged build finished. Test PASSed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/13264#issuecomment-221119370 **[Test build #59150 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59150/consoleFull)** for PR 13264 at commit [`2c0e680`](https://github.com/apache/spark/commit/2c0e680d2ba1e148b0c2d41408b11c391bfcb66a). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/13264#issuecomment-221061309 **[Test build #59150 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59150/consoleFull)** for PR 13264 at commit [`2c0e680`](https://github.com/apache/spark/commit/2c0e680d2ba1e148b0c2d41408b11c391bfcb66a). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user andrewor14 commented on the pull request: https://github.com/apache/spark/pull/13264#issuecomment-221059879 ok to test --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/13264#issuecomment-221046486 Can one of the admins verify this patch? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-9044 Fix "Storage" tab in UI so that it ...
GitHub user lgieron opened a pull request: https://github.com/apache/spark/pull/13264 SPARK-9044 Fix "Storage" tab in UI so that it reflects RDD name change. ## What changes were proposed in this pull request? 1. Making 'name' field of RDDInfo mutable. 2. In StorageListener: catching the fact that RDD's name was changed and updating it in RDDInfo. ## How was this patch tested? 1. Manual verification - the 'Storage' tab now behaves as expected. 2. The commit also contains a new unit test which verifies this. You can merge this pull request into a Git repository by running: $ git pull https://github.com/lgieron/spark SPARK-9044 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/spark/pull/13264.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #13264 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
