[SPARK-23096][SS] Migrate rate source to V2 #20688

jerryshao · 2018-02-28T02:12:41Z

What changes were proposed in this pull request?

This PR migrate micro batch rate source to V2 API and rewrite UTs to suite V2 test.

How was this patch tested?

UTs.

SparkQA · 2018-02-28T04:30:10Z

Test build #87740 has finished for PR 20688 at commit 538223e.

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

SparkQA · 2018-02-28T08:05:02Z

Test build #87751 has finished for PR 20688 at commit 8bfadc3.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun · 2018-02-28T18:14:04Z

Retest this please.

SparkQA · 2018-02-28T21:22:58Z

Test build #87799 has finished for PR 20688 at commit 8bfadc3.

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

jerryshao · 2018-03-05T05:39:56Z

@tdas @jose-torres can you please take a review when you have time, thanks!

SparkQA · 2018-03-05T08:05:01Z

Test build #87951 has finished for PR 20688 at commit 5a29d62.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

tdas · 2018-03-07T03:04:02Z

...ore/src/main/scala/org/apache/spark/sql/execution/streaming/sources/RateSourceProvider.scala

+  override def shortName(): String = "rate"
+}
+
+class RateStreamMicroBatchReader(options: DataSourceOptions, checkpointLocation: String)


split this into a separate file.

tdas · 2018-03-07T03:06:56Z

sql/core/src/test/scala/org/apache/spark/sql/execution/streaming/sources/RateSourceSuite.scala

+import org.apache.spark.sql.streaming.StreamTest
+import org.apache.spark.util.ManualClock
+
+class RateSourceSuite extends StreamTest {


Why did you not move this file using "git mv" and then change? Then we would have been able to diff it properly.
This was a pain in the text socket v2 PR as well :(

Hi @tdas , I think I used "git mv", the thing is that when the diff is larger then x%, it will treat as "git rm" and "git add" (https://makandracards.com/makandra/30957-git-how-to-get-a-useful-diff-when-renaming-files).

Sorry about the inconvenience, but I'm not sure if there's other approaches.

tdas · 2018-03-07T03:12:44Z

...n/scala/org/apache/spark/sql/execution/streaming/continuous/ContinuousRateStreamSource.scala

-import org.apache.spark.sql.execution.streaming.{RateSourceProvider, RateStreamOffset, ValueRunTimeMsPair}
-import org.apache.spark.sql.execution.streaming.sources.RateStreamSourceV2
+import org.apache.spark.sql.execution.streaming.{RateStreamOffset, ValueRunTimeMsPair}
+import org.apache.spark.sql.execution.streaming.sources.RateSourceProvider
 import org.apache.spark.sql.sources.v2.DataSourceOptions
 import org.apache.spark.sql.sources.v2.reader._
 import org.apache.spark.sql.sources.v2.reader.streaming.{ContinuousDataReader, ContinuousReader, Offset, PartitionOffset}


Could you make the class names and file names of the different readers consistent with each other? Similar to Kafka?

RateStreamProvider
RateStreamMicroBatchReader, RateStreamMicroBatchDataReaderFactory ....
RateStreamContinuousReader, ....

jerryshao · 2018-03-08T11:40:19Z

@tdas I tried different ways to keep history of rename, but seems git always treat as "rm and add" for big changes (https://coderwall.com/p/_csouq/renaming-and-changing-files-in-git-without-losing-history). Sorry to bring inconvenience. Do you have any better solutions?

jerryshao · 2018-03-08T11:46:57Z

Jenkins, retest this please.

SparkQA · 2018-03-08T14:48:35Z

Test build #88085 has finished for PR 20688 at commit 63ed0b4.

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

SparkQA · 2018-03-08T15:04:53Z

Test build #88086 has finished for PR 20688 at commit 63ed0b4.

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

jose-torres · 2018-03-14T16:27:46Z

Sorry it took me so long to get to this. LGTM

jerryshao · 2018-03-28T01:15:08Z

Thanks @tdas and @jose-torres .

gatorsmile · 2018-03-28T16:12:57Z

Sorry, I need to revert this PR. It breaks our build. Could you resubmit the PR after fixing the tests?

https://amplab.cs.berkeley.edu/jenkins/view/Spark%20QA%20Test/job/spark-master-test-maven-hadoop-2.6/

gatorsmile · 2018-03-28T16:59:42Z

Next time, we should re-trigger the tests before merging the code. The most recent tests ran 20 days ago

## What changes were proposed in this pull request? This PR migrate micro batch rate source to V2 API and rewrite UTs to suite V2 test. ## How was this patch tested? UTs. Author: jerryshao <sshao@hortonworks.com> Closes apache#20688 from jerryshao/SPARK-23096.

## What changes were proposed in this pull request? Roll forward c68ec4e (#20688). There are two minor test changes required: * An error which used to be TreeNodeException[ArithmeticException] is no longer wrapped and is now just ArithmeticException. * The test framework simply does not set the active Spark session. (Or rather, it doesn't do so early enough - I think it only happens when a query is analyzed.) I've added the required logic to SQLTestUtils. ## How was this patch tested? existing tests Author: Jose Torres <torres.joseph.f+github@gmail.com> Author: jerryshao <sshao@hortonworks.com> Closes #20922 from jose-torres/ratefix.

## What changes were proposed in this pull request? This PR migrate micro batch rate source to V2 API and rewrite UTs to suite V2 test. ## How was this patch tested? UTs. Author: jerryshao <sshao@hortonworks.com> Closes apache#20688 from jerryshao/SPARK-23096.

## What changes were proposed in this pull request? Roll forward c68ec4e (apache#20688). There are two minor test changes required: * An error which used to be TreeNodeException[ArithmeticException] is no longer wrapped and is now just ArithmeticException. * The test framework simply does not set the active Spark session. (Or rather, it doesn't do so early enough - I think it only happens when a query is analyzed.) I've added the required logic to SQLTestUtils. ## How was this patch tested? existing tests Author: Jose Torres <torres.joseph.f+github@gmail.com> Author: jerryshao <sshao@hortonworks.com> Closes apache#20922 from jose-torres/ratefix.

Roll forward c68ec4e (apache#20688). There are two minor test changes required: * An error which used to be TreeNodeException[ArithmeticException] is no longer wrapped and is now just ArithmeticException. * The test framework simply does not set the active Spark session. (Or rather, it doesn't do so early enough - I think it only happens when a query is analyzed.) I've added the required logic to SQLTestUtils. existing tests Author: Jose Torres <torres.joseph.f+github@gmail.com> Author: jerryshao <sshao@hortonworks.com> Closes apache#20922 from jose-torres/ratefix. Ref: LIHADOOP-48531

jerryshao force-pushed the SPARK-23096 branch from 8bfadc3 to 5a29d62 Compare March 5, 2018 05:39

tdas reviewed Mar 7, 2018

View reviewed changes

Migrate rate source to V2

63ed0b4

jerryshao force-pushed the SPARK-23096 branch from 5a29d62 to 63ed0b4 Compare March 8, 2018 11:37

asfgit closed this in c68ec4e Mar 27, 2018

mn-mikke mentioned this pull request Mar 28, 2018

[SPARK-23736][SQL] Extending the concat function to support array columns #20858

Closed

jose-torres mentioned this pull request Mar 28, 2018

Roll forward "[SPARK-23096][SS] Migrate rate source to V2" #20921

Closed

jose-torres mentioned this pull request Mar 28, 2018

Roll forward "[SPARK-23096][SS] Migrate rate source to V2" #20922

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-23096][SS] Migrate rate source to V2 #20688

[SPARK-23096][SS] Migrate rate source to V2 #20688

jerryshao commented Feb 28, 2018

SparkQA commented Feb 28, 2018

SparkQA commented Feb 28, 2018

dongjoon-hyun commented Feb 28, 2018

SparkQA commented Feb 28, 2018

jerryshao commented Mar 5, 2018

SparkQA commented Mar 5, 2018

tdas Mar 7, 2018

tdas Mar 7, 2018

jerryshao Mar 7, 2018 •

edited

tdas Mar 7, 2018 •

edited

jerryshao commented Mar 8, 2018

jerryshao commented Mar 8, 2018

SparkQA commented Mar 8, 2018

SparkQA commented Mar 8, 2018

jose-torres commented Mar 14, 2018

jerryshao commented Mar 28, 2018

gatorsmile commented Mar 28, 2018

gatorsmile commented Mar 28, 2018

[SPARK-23096][SS] Migrate rate source to V2 #20688

[SPARK-23096][SS] Migrate rate source to V2 #20688

Conversation

jerryshao commented Feb 28, 2018

What changes were proposed in this pull request?

How was this patch tested?

SparkQA commented Feb 28, 2018

SparkQA commented Feb 28, 2018

dongjoon-hyun commented Feb 28, 2018

SparkQA commented Feb 28, 2018

jerryshao commented Mar 5, 2018

SparkQA commented Mar 5, 2018

tdas Mar 7, 2018

Choose a reason for hiding this comment

tdas Mar 7, 2018

Choose a reason for hiding this comment

jerryshao Mar 7, 2018 • edited

Choose a reason for hiding this comment

tdas Mar 7, 2018 • edited

Choose a reason for hiding this comment

jerryshao commented Mar 8, 2018

jerryshao commented Mar 8, 2018

SparkQA commented Mar 8, 2018

SparkQA commented Mar 8, 2018

jose-torres commented Mar 14, 2018

jerryshao commented Mar 28, 2018

gatorsmile commented Mar 28, 2018

gatorsmile commented Mar 28, 2018

jerryshao Mar 7, 2018 •

edited

tdas Mar 7, 2018 •

edited