Skip to content

Conversation

@tdas
Copy link
Contributor

@tdas tdas commented Jun 22, 2017

What changes were proposed in this pull request?

Copy of PR #18355 for testing the unidoc issue.

How was this patch tested?

@tdas tdas changed the title [SPARK-21145][SS] Added StateStoreProviderId with queryRunId to reload StateStoreProviders when query is restarted [DO-NOT-MERGE][SPARK-21145][SS] Added StateStoreProviderId with queryRunId to reload StateStoreProviders when query is restarted Jun 22, 2017
@SparkQA
Copy link

SparkQA commented Jun 22, 2017

Test build #78484 has finished for PR 18396 at commit 0ad5a5c.

  • This patch fails to generate documentation.
  • This patch merges cleanly.
  • This patch adds no public classes.

@HyukjinKwon
Copy link
Member

This looks weird. Many documentation goes mismatched after afterAll and BeforeAndAfterAll which do not look related with the documentation failure.

I just tested before this PR - https://gist.github.com/HyukjinKwon/ca77ffb7848af00c8a67e6e050b716a3

after this PR - https://gist.github.com/HyukjinKwon/481ca9e8aad918f4757eb0c8efb06ee4

To unblock the PR, (just based on my wild guess) I tried and found that a weird workaround was relocating.

--- a/sql/core/src/test/scala/org/apache/spark/sql/streaming/StreamTest.scala
+++ b/sql/core/src/test/scala/org/apache/spark/sql/streaming/StreamTest.scala
@@ -216,6 +216,11 @@ trait StreamTest extends QueryTest with SharedSQLContext with Timeouts with Befo
       AssertOnQuery(query => { func(query); true })
   }

+  override def afterAll(): Unit = {
+    super.afterAll()
+    StateStore.stop() // stop the state store maintenance thread and unload store providers
+  }
+
   /**
    * Executes the specified actions on the given streaming DataFrame and provides helpful
    * error messages in the case of failures or incorrect answers.
@@ -661,11 +666,6 @@ trait StreamTest extends QueryTest with SharedSQLContext with Timeouts with Befo
     testStream(ds)(actions: _*)
   }

-  override def afterAll(): Unit = {
-    super.afterAll()
-    StateStore.stop() // stop the state store maintenance thread and unload store providers
-  }
-
   object AwaitTerminationTester {

Let me try to prepare minimal reproduction and open an issue to GenJavaDoc.

@tdas
Copy link
Contributor Author

tdas commented Jun 23, 2017

Thanks I will try that in my main PR - #18355

@SparkQA
Copy link

SparkQA commented Jun 23, 2017

Test build #78491 has finished for PR 18396 at commit 622c6a7.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • trait StreamTest extends QueryTest with SharedSQLContext with Timeouts

@tdas
Copy link
Contributor Author

tdas commented Jun 23, 2017

@HyukjinKwon But why did this change in StreamTests change all the warnings to errors?

@HyukjinKwon
Copy link
Member

HyukjinKwon commented Jun 23, 2017

For warnings becoming errors, I described this in https://issues.apache.org/jira/browse/SPARK-20840 with my tests and observations. Given my investigation, the root cause is, Javadoc8 produces both warnings and errors into standard error and SBT gets confused of which one is warning. One possible approach (which was done in SBT I believe) is to manually parse the log output (in a similar way I proposed in the JIRA) but it looks there is limitation. There are more information in the JIRA I investigated for a couple of weeks.

+The basic rule for SBT with Javadoc is, up to my knowledge, produces both errors and warnings as errors if there is any error and produces warnings if there is no error.

For mismatching docs, I don't know the exact reason. My wild guess was it got confused by nested ones so I just tried to put afterAll to where I guess GenJavaDoc gets less confused.

For not throwing errors for missing documentations in StreamTest, I guess because we disabled the doclint for miing documentation - https://github.com/apache/spark/blob/master/project/SparkBuild.scala#L233. For unknown reason, it appears the documentation went mixed with some functions that have no documentation and this threw errors for parameter mismatching that we check up to my knowledge.

@tdas tdas closed this Jun 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants