Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-23565] [SS] New error message for structured streaming sources assertion #20946

Closed
wants to merge 3 commits into from

Conversation

patrickmcgloin
Copy link
Contributor

What changes were proposed in this pull request?

A more informative message to tell you why a structured streaming query cannot continue if you have added more sources, than there are in the existing checkpoint offsets.

How was this patch tested?

I added a Unit Test.

Copy link
Member

@xuanyuanking xuanyuanking left a comment

Choose a reason for hiding this comment

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

Change the title to '[SPARK-23565][SS]...' is more accurate.

@@ -39,7 +39,9 @@ case class OffsetSeq(offsets: Seq[Option[Offset]], metadata: Option[OffsetSeqMet
* cannot be serialized).
*/
def toStreamProgress(sources: Seq[BaseStreamingSource]): StreamProgress = {
assert(sources.size == offsets.size)
assert(sources.size == offsets.size, s"There are [${offsets.size}] sources in the " +
s"checkpoint offsets and now there are [${sources.size}] sources requested by the query. " +
Copy link
Member

Choose a reason for hiding this comment

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

nit: extra blank in the end

@@ -125,6 +125,19 @@ class OffsetSeqLogSuite extends SparkFunSuite with SharedSQLContext {
assert(offsetSeq.metadata === Some(OffsetSeqMetadata(0L, 1480981499528L)))
}

test("assertion that number of checkpoint offsets match number of sources") {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe no need to add UT for the log change.

@patrickmcgloin patrickmcgloin changed the title [SPARK-23565] [SQL] New error message for structured streaming sources assertion [SPARK-23565] [SS] New error message for structured streaming sources assertion Apr 25, 2018
@patrickmcgloin
Copy link
Contributor Author

Hi Xuan,

I have updated the title, removed the extra white space after the period / dot and removed the UT.

@patrickmcgloin
Copy link
Contributor Author

Hi @xuanyuanking, please see comments above.

Copy link
Member

@xuanyuanking xuanyuanking left a comment

Choose a reason for hiding this comment

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

+1 To me, useful in SS debugging, cc @cloud-fan to take a look.

@cloud-fan
Copy link
Contributor

cc @zsxwing

@zsxwing
Copy link
Member

zsxwing commented Apr 26, 2018

okey to test

LGTM

@zsxwing
Copy link
Member

zsxwing commented Apr 26, 2018

ok to test

@SparkQA
Copy link

SparkQA commented Apr 26, 2018

Test build #89881 has finished for PR 20946 at commit b35acf5.

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

@zsxwing
Copy link
Member

zsxwing commented Apr 27, 2018

Thanks! Merging to master.

@asfgit asfgit closed this in 2824f12 Apr 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants