You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by lw-lin <gi...@git.apache.org> on 2016/03/29 11:42:26 UTC
[GitHub] spark pull request: [SPARK-12857][STREAMING] Streaming tab in web ...
GitHub user lw-lin opened a pull request:
https://github.com/apache/spark/pull/12032
[SPARK-12857][STREAMING] Streaming tab in web UI uses records and events interchangeably
## What changes were proposed in this pull request?
Currently the Streaming tab in web UI uses records and events interchangeably; this PR tries to standardize them on "records".
"records" is chosen over "events" because:
- "records" is used extensively throughout the streaming documents, codes, and comments
- "events" is used only in Streaming UI related codes and comments
## How was this patch tested?
- existing test suites
- manually checking on the Streaming UI tab
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/lw-lin/spark streaming-events-to-records
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/12032.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 #12032
----
commit 43e3032f08af6a918ea40d01a731ff5f4cd7d913
Author: Liwei Lin <lw...@gmail.com>
Date: 2016-03-29T02:06:27Z
Standardize records and events to records
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12857][STREAMING] Standardize "records"...
Posted by lw-lin <gi...@git.apache.org>.
Github user lw-lin commented on the pull request:
https://github.com/apache/spark/pull/12032#issuecomment-203259640
@zsxwing please be aware of the changes we made here :-)
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12857][STREAMING] Standardize "records"...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:
https://github.com/apache/spark/pull/12032#issuecomment-203726152
LGTM
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12857][STREAMING] Streaming tab in web ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12032#issuecomment-202809094
**[Test build #54429 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54429/consoleFull)** for PR 12032 at commit [`43e3032`](https://github.com/apache/spark/commit/43e3032f08af6a918ea40d01a731ff5f4cd7d913).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12857][STREAMING] Standardize "records"...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/12032#discussion_r57729876
--- Diff: streaming/src/main/scala/org/apache/spark/streaming/ui/StreamingPage.scala ---
@@ -241,24 +241,24 @@ private[ui] class StreamingPage(parent: StreamingTab)
// Use the max input rate for all InputDStreams' graphs to make the Y axis ranges same.
// If it's not an integral number, just use its ceil integral number.
- val maxEventRate = eventRateForAllStreams.max.map(_.ceil.toLong).getOrElse(0L)
- val minEventRate = 0L
+ val maxRecordRate = recordRateForAllStreams.max.map(_.ceil.toLong).getOrElse(0L)
+ val minRecordRate = 0L
val batchInterval = UIUtils.convertToTimeUnit(listener.batchDuration, normalizedUnit)
val jsCollector = new JsCollector
- val graphUIDataForEventRateOfAllStreams =
+ val graphUIDataForRecordRateOfAllStreams =
new GraphUIData(
- "all-stream-events-timeline",
- "all-stream-events-histogram",
- eventRateForAllStreams.data,
+ "all-stream-records-timeline",
--- End diff --
The other changes seem OK as they're to labels and private classes and internal code; this changes a div ID and I'm not clear if something else has to change in the code to match? like will something else fail because it doesn't find all-stream-events-timeline? I don't see any other references though.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12857][STREAMING] Streaming tab in web ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12032#issuecomment-202828513
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12857][STREAMING] Streaming tab in web ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12032#issuecomment-202828370
**[Test build #54429 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54429/consoleFull)** for PR 12032 at commit [`43e3032`](https://github.com/apache/spark/commit/43e3032f08af6a918ea40d01a731ff5f4cd7d913).
* 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12857][STREAMING] Standardize "records"...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/12032#issuecomment-204592222
Merged to master
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12857][STREAMING] Standardize "records"...
Posted by lw-lin <gi...@git.apache.org>.
Github user lw-lin commented on a diff in the pull request:
https://github.com/apache/spark/pull/12032#discussion_r57835381
--- Diff: streaming/src/main/scala/org/apache/spark/streaming/ui/StreamingPage.scala ---
@@ -241,24 +241,24 @@ private[ui] class StreamingPage(parent: StreamingTab)
// Use the max input rate for all InputDStreams' graphs to make the Y axis ranges same.
// If it's not an integral number, just use its ceil integral number.
- val maxEventRate = eventRateForAllStreams.max.map(_.ceil.toLong).getOrElse(0L)
- val minEventRate = 0L
+ val maxRecordRate = recordRateForAllStreams.max.map(_.ceil.toLong).getOrElse(0L)
+ val minRecordRate = 0L
val batchInterval = UIUtils.convertToTimeUnit(listener.batchDuration, normalizedUnit)
val jsCollector = new JsCollector
- val graphUIDataForEventRateOfAllStreams =
+ val graphUIDataForRecordRateOfAllStreams =
new GraphUIData(
- "all-stream-events-timeline",
- "all-stream-events-histogram",
- eventRateForAllStreams.data,
+ "all-stream-records-timeline",
--- End diff --
@srowen thank you for the detailed review!
Yeah normally this requires something else to change, but `GraphUIData` has been implemented well enough to save the trouble. We only need to change this `"all-stream-events-timeline"` here, then the div ID as well as any other place referring to this div will change accordingly; please see the snapshots of the HTML source:
div as a placeholder:
then fill in that div with data:
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12857][STREAMING] Streaming tab in web ...
Posted by lw-lin <gi...@git.apache.org>.
Github user lw-lin commented on the pull request:
https://github.com/apache/spark/pull/12032#issuecomment-202808056
No public documents need to change, since we have been using the term "records" consistently.
Strings on the Streaming tab has been changed from "events" to "records".
There will be very minimum change if we also clean up the codes a little -- only one private class, one public method in another private class, and some local variable require renaming -- so I've also done it along within this PR.
@srowen @zsxwing would you mind taking a look when you have time? 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12857][STREAMING] Streaming tab in web ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12032#issuecomment-202828518
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/54429/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12857][STREAMING] Standardize "records"...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/12032
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org