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.

Sign up for GitHub

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

Jump to bottom

[HOTFIX][Streaming][MLlib] use temp folder for checkpoint #2046

Closed
wants to merge 3 commits into from
Closed

[HOTFIX][Streaming][MLlib] use temp folder for checkpoint #2046

wants to merge 3 commits into from

Conversation

mengxr
Copy link
Contributor

@mengxr mengxr commented Aug 20, 2014

or Jenkins will complain about no Apache header in checkpoint files. @tdas @rxin

@mengxr mengxr changed the title [HOTFIX] use temp folder for checkpoint [HOTFIX][Streaming][MLlib] use temp folder for checkpoint Aug 20, 2014
@SparkQA
Copy link

SparkQA commented Aug 20, 2014

QA tests have started for PR 2046 at commit 89964ab.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Aug 20, 2014

QA tests have finished for PR 2046 at commit 89964ab.

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

@mengxr
Copy link
Contributor Author

mengxr commented Aug 20, 2014

@rxin Could you help clean the Jenkins working directory?

@SparkQA
Copy link

SparkQA commented Aug 20, 2014

QA tests have started for PR 2046 at commit 9797843.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Aug 20, 2014

QA tests have finished for PR 2046 at commit 9797843.

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

@davies
Copy link
Contributor

davies commented Aug 20, 2014

Jenkins, retest this please

@mengxr
Copy link
Contributor Author

mengxr commented Aug 20, 2014

Jenkins, retest this please.

@SparkQA
Copy link

SparkQA commented Aug 20, 2014

QA tests have started for PR 2046 at commit 0d3ec73.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Aug 20, 2014

QA tests have finished for PR 2046 at commit 0d3ec73.

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

@rxin
Copy link
Contributor

rxin commented Aug 20, 2014

Jenkins, retest this please.

@SparkQA
Copy link

SparkQA commented Aug 20, 2014

QA tests have started for PR 2046 at commit 0d3ec73.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Aug 20, 2014

QA tests have finished for PR 2046 at commit 0d3ec73.

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

@mengxr
Copy link
Contributor Author

mengxr commented Aug 20, 2014

Jenkins, retest this please.

@SparkQA
Copy link

SparkQA commented Aug 20, 2014

QA tests have started for PR 2046 at commit 0d3ec73.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Aug 20, 2014

QA tests have finished for PR 2046 at commit 0d3ec73.

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

@mengxr
Copy link
Contributor Author

mengxr commented Aug 20, 2014

Jenkins, retest this please.

@SparkQA
Copy link

SparkQA commented Aug 20, 2014

QA tests have started for PR 2046 at commit 0d3ec73.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Aug 20, 2014

QA tests have finished for PR 2046 at commit 0d3ec73.

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

@mengxr
Copy link
Contributor Author

mengxr commented Aug 20, 2014

Jenkins, retest this please.

@SparkQA
Copy link

SparkQA commented Aug 20, 2014

QA tests have started for PR 2046 at commit 0d3ec73.

  • This patch merges cleanly.

@pwendell
Copy link
Contributor

@mengxr when something like this happens - can you please log into jenkins manually and clear the bad files on all nodes? otherwise this will fail other tests indefinitely until someone does that.

@SparkQA
Copy link

SparkQA commented Aug 20, 2014

QA tests have finished for PR 2046 at commit 0d3ec73.

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

@asfgit asfgit closed this in fce5c0f Aug 20, 2014
asfgit pushed a commit that referenced this pull request Aug 20, 2014
@mengxr
[HOTFIX][Streaming][MLlib] use temp folder for checkpoint
d5db95b 
or Jenkins will complain about no Apache header in checkpoint files. tdas rxin

Author: Xiangrui Meng <meng@databricks.com>

Closes #2046 from mengxr/tmp-checkpoint and squashes the following commits:

0d3ec73 [Xiangrui Meng] remove ssc.stop
9797843 [Xiangrui Meng] change checkpointDir to lazy val
89964ab [Xiangrui Meng] use temp folder for checkpoint

(cherry picked from commit fce5c0f)
Signed-off-by: Xiangrui Meng <meng@databricks.com>
@rxin
Copy link
Contributor

rxin commented Aug 20, 2014

We can just wipe the workspace on the web.

@rxin
Copy link
Contributor

rxin commented Aug 20, 2014

I did that.

@mengxr
Copy link
Contributor Author

mengxr commented Aug 20, 2014

The folder is created by a unit test. So we have to change the checkpointDir setting first and then wipe the workspace.

srowen
srowen reviewed Aug 20, 2014
View reviewed changes
streaming/src/test/scala/org/apache/spark/streaming/TestSuiteBase.scala
@@ -119,7 +119,12 @@ trait TestSuiteBase extends FunSuite with BeforeAndAfter with Logging {
def batchDuration = Seconds(1)

// Directory where the checkpoint data will be saved
def checkpointDir = "checkpoint"
lazy val checkpointDir = {
val dir = Files.createTempDir()
Copy link
Member

Choose a reason for hiding this comment

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

Ideally, delete this explicitly and recursively in the 'after' function. It could fail to be deleted if the JVM crashes later on. Minor though.

@JoshRosen
Copy link
Contributor

Why not just add the checkpoint directory to .rat_excludes? Or run git clean in the Jenkins scripts? See my comment at #1978 (comment)

@srowen
Copy link
Member

srowen commented Aug 20, 2014

Probably also a good idea, but tests ought to try to behave well -- write to temp dirs, not the source tree, and clean up reliably. I suppose it helps ensure test state isn't accidentally picked up by subsequent runs too. It's not just an issue for RAT and Jenkins, but for devs. For a while tests were not cleaning up a "work" directory, and I ended up with hundreds of thousands of files under it. (That's fixed now.) My IDE was trying to index them. Stuff like that is a minor problem.

xiliu82 pushed a commit to xiliu82/spark that referenced this pull request Sep 4, 2014
@mengxr @conviva-zz
[HOTFIX][Streaming][MLlib] use temp folder for checkpoint
a0c898e 
or Jenkins will complain about no Apache header in checkpoint files. tdas rxin

Author: Xiangrui Meng <meng@databricks.com>

Closes apache#2046 from mengxr/tmp-checkpoint and squashes the following commits:

0d3ec73 [Xiangrui Meng] remove ssc.stop
9797843 [Xiangrui Meng] change checkpointDir to lazy val
89964ab [Xiangrui Meng] use temp folder for checkpoint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@mengxr @SparkQA @davies @rxin @pwendell @JoshRosen @srowen