[SPARK-6197][CORE] handle json exception when hisotry file not finished writing #4927

liyezhang556520 · 2015-03-06T05:36:36Z

For details, please refer to SPARK-6197

SparkQA · 2015-03-06T05:37:45Z

Test build #28328 has started for PR 4927 at commit 2973024.

This patch merges cleanly.

liyezhang556520 · 2015-03-06T05:42:19Z

cc @srowen, @andrewor14, @viirya

viirya · 2015-03-06T05:49:53Z

@liyezhang556520 do you really encounter this problem? Isn't the writing operation atomic?

liyezhang556520 · 2015-03-06T05:57:56Z

Yes, I do encounter the problem, if you use Ctrl+C during tasks finishing, it's easy to happen.

zsxwing · 2015-03-06T06:02:45Z

@liyezhang556520 do you really encounter this problem? Isn't the writing operation atomic?

The writing operation is not atomic.

liyezhang556520 · 2015-03-06T06:07:58Z

And I don't think writing operation is atomic, signals still can iterrupt

liyezhang556520 · 2015-03-06T06:09:51Z

ok, thanks @zsxwing answer the second question.

SparkQA · 2015-03-06T06:57:29Z

Test build #28328 has finished for PR 4927 at commit 2973024.

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

AmplabJenkins · 2015-03-06T06:57:32Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28328/
Test PASSed.

srowen · 2015-03-06T11:38:37Z

core/src/main/scala/org/apache/spark/scheduler/ReplayListenerBus.scala

+  def replay(
+      logData: InputStream,
+      sourceName: String,
+      sourceTruncated: Boolean = false): Unit = {


Nit: let's call this maybeTruncated? it's not certain that the file is truncated.

SparkQA · 2015-03-06T15:23:04Z

Test build #28344 has started for PR 4927 at commit 2b48831.

This patch merges cleanly.

liyezhang556520 · 2015-03-06T15:27:17Z

@srowen , thanks for your comments, and I have updated the code.

srowen · 2015-03-06T15:28:40Z

core/src/main/scala/org/apache/spark/deploy/master/Master.scala

      try {
-        replayBus.replay(logInput, eventLogFile)
+        replayBus.replay(logInput, eventLogFile, 


OK by me except this line no longer needs to wrap. Maybe see if others have a comment before bothering to update again.

SparkQA · 2015-03-06T15:42:44Z

Test build #28345 has started for PR 4927 at commit 5cbdc82.

This patch merges cleanly.

SparkQA · 2015-03-06T16:45:09Z

Test build #28344 has finished for PR 4927 at commit 2b48831.

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

AmplabJenkins · 2015-03-06T16:45:14Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28344/
Test PASSed.

SparkQA · 2015-03-06T17:02:17Z

Test build #28345 has finished for PR 4927 at commit 5cbdc82.

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

AmplabJenkins · 2015-03-06T17:02:21Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28345/
Test PASSed.

srowen · 2015-03-06T17:05:21Z

Thoughts, @viirya @zsxwing ?

viirya · 2015-03-06T17:21:04Z

Hmm, just being curious, replay originally has a try...catch block outside the log reading, and this pr adds another one in the reading. Is it better to combine two try...catch blocks together for readability?

liyezhang556520 · 2015-03-06T17:37:44Z

@viirya , that is because we need to check whether lines iterator is exhausted (by checking lines.hasNext) to decide whether we can ignore the exception, and the lines is only available in the try block.

viirya · 2015-03-06T17:43:32Z

Ah, not notice that. :-)

zsxwing · 2015-03-13T08:09:27Z

core/src/main/scala/org/apache/spark/scheduler/ReplayListenerBus.scala

@@ -17,6 +17,7 @@

 package org.apache.spark.scheduler

+import com.fasterxml.jackson.core.JsonParseException


The import order is wrong. See https://cwiki.apache.org/confluence/display/SPARK/Spark+Code+Style+Guide#SparkCodeStyleGuide-Imports

zsxwing · 2015-03-13T08:09:43Z

LGTM except the minor import order problem.

srowen · 2015-03-13T13:59:34Z

I can fix the import on merge.

…ed writing For details, please refer to [SPARK-6197](https://issues.apache.org/jira/browse/SPARK-6197) Author: Zhang, Liye <liye.zhang@intel.com> Closes apache#4927 from liyezhang556520/jsonParseError and squashes the following commits: 5cbdc82 [Zhang, Liye] without unnecessary wrap 2b48831 [Zhang, Liye] small changes with sean owen's comments 2973024 [Zhang, Liye] handle json exception when file not finished writing Conflicts: core/src/main/scala/org/apache/spark/deploy/master/Master.scala

…ed writing For details, please refer to [SPARK-6197](https://issues.apache.org/jira/browse/SPARK-6197) Author: Zhang, Liye <liye.zhang@intel.com> Closes #4927 from liyezhang556520/jsonParseError and squashes the following commits: 5cbdc82 [Zhang, Liye] without unnecessary wrap 2b48831 [Zhang, Liye] small changes with sean owen's comments 2973024 [Zhang, Liye] handle json exception when file not finished writing

handle json exception when file not finished writing

2973024

srowen reviewed Mar 6, 2015
View reviewed changes

small changes with sean owen's comments

2b48831

srowen reviewed Mar 6, 2015
View reviewed changes

without unnecessary wrap

5cbdc82

zsxwing reviewed Mar 13, 2015
View reviewed changes

asfgit closed this in 9048e81 Mar 13, 2015

liyezhang556520 deleted the jsonParseError branch March 20, 2015 05:18

		@@ -17,6 +17,7 @@

		package org.apache.spark.scheduler

		import com.fasterxml.jackson.core.JsonParseException

[SPARK-6197][CORE] handle json exception when hisotry file not finished writing #4927

[SPARK-6197][CORE] handle json exception when hisotry file not finished writing #4927

Uh oh!

Conversation

liyezhang556520 commented Mar 6, 2015

Uh oh!

SparkQA commented Mar 6, 2015

Uh oh!

liyezhang556520 commented Mar 6, 2015

Uh oh!

viirya commented Mar 6, 2015

Uh oh!

liyezhang556520 commented Mar 6, 2015

Uh oh!

zsxwing commented Mar 6, 2015

Uh oh!

liyezhang556520 commented Mar 6, 2015

Uh oh!

liyezhang556520 commented Mar 6, 2015

Uh oh!

SparkQA commented Mar 6, 2015

Uh oh!

AmplabJenkins commented Mar 6, 2015

Uh oh!

srowen Mar 6, 2015

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Mar 6, 2015

Uh oh!

liyezhang556520 commented Mar 6, 2015

Uh oh!

srowen Mar 6, 2015

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Mar 6, 2015

Uh oh!

SparkQA commented Mar 6, 2015

Uh oh!

AmplabJenkins commented Mar 6, 2015

Uh oh!

SparkQA commented Mar 6, 2015

Uh oh!

AmplabJenkins commented Mar 6, 2015

Uh oh!

srowen commented Mar 6, 2015

Uh oh!

viirya commented Mar 6, 2015

Uh oh!

liyezhang556520 commented Mar 6, 2015

Uh oh!

viirya commented Mar 6, 2015

Uh oh!

zsxwing Mar 13, 2015

Choose a reason for hiding this comment

Uh oh!

zsxwing commented Mar 13, 2015

Uh oh!

srowen commented Mar 13, 2015

Uh oh!

Uh oh!