You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by lw-lin <gi...@git.apache.org> on 2017/02/26 03:56:55 UTC

[GitHub] spark pull request #17070: [WIP][SS] Good error message for version mismatch...

GitHub user lw-lin opened a pull request:

    https://github.com/apache/spark/pull/17070

    [WIP][SS] Good error message for version mismatch in log files

    ## What changes were proposed in this pull request?
    
    (Please fill in changes proposed in this fix)
    
    ## How was this patch tested?
    
    (Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
    (If this patch involves UI changes, please attach a screenshot; otherwise, remove this)
    
    Please review http://spark.apache.org/contributing.html before opening a pull request.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/lw-lin/spark better-msg

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/17070.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #17070
    
----
commit dcae69b351ac71bf1554b760aeea6b763b2bf4c0
Author: Liwei Lin <lw...@gmail.com>
Date:   2017-02-25T03:46:35Z

    Fix

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17070: [SPARK-19721][SS] Good error message for version mismatc...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/17070
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17070: [WIP][SS] Good error message for version mismatch...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17070#discussion_r103098803
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/HDFSMetadataLog.scala ---
    @@ -18,6 +18,7 @@
     package org.apache.spark.sql.execution.streaming
     
     import java.io._
    +import java.lang.IllegalStateException
    --- End diff --
    
    You don't need to import from java.lang


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17070: [WIP][SS] Good error message for version mismatch...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17070#discussion_r103098815
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/HDFSMetadataLog.scala ---
    @@ -268,6 +274,37 @@ class HDFSMetadataLog[T <: AnyRef : ClassTag](sparkSession: SparkSession, path:
             new FileSystemManager(metadataPath, hadoopConf)
         }
       }
    +
    +  /**
    +   * Parse the log version from the given `text` -- will throw exception when the parsed version
    +   * exceeds `maxSupportedVersion`, or when `text` is malformed (such as "xyz", "v", "v-1",
    +   * "v123xyz" etc.)
    +   */
    +  private[sql] def parseVersion(text: String, maxSupportedVersion: Int): Int = {
    +    if (text.length > 0 && text(0) == 'v') {
    +      val version =
    +        try { text.substring(1, text.length).toInt }
    +        catch {
    +          case _: NumberFormatException =>
    +            throw new IllegalStateException(s"Log file was malformed: failed to read correct log " +
    +              s"version from $text.")
    +        }
    +      if (version > 0) {
    +        if (version > maxSupportedVersion) {
    +          throw new IllegalStateException(s"UnsupportedLogVersion: maximum supported log version " +
    +            s"is v${maxSupportedVersion}, but encountered v$version. The log file was produced " +
    +            s"by a newer version of Spark and cannot be read by this version. Please upgrade.")
    +        }
    +        else {
    --- End diff --
    
    Put on previous line; no need to use return


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17070: [SPARK-19721][SS] Good error message for version mismatc...

Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the issue:

    https://github.com/apache/spark/pull/17070
  
    LGTM. Merging to master and 2.1. Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17070: [WIP][SS] Good error message for version mismatch...

Posted by lw-lin <gi...@git.apache.org>.
Github user lw-lin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17070#discussion_r103102420
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/HDFSMetadataLog.scala ---
    @@ -268,6 +274,37 @@ class HDFSMetadataLog[T <: AnyRef : ClassTag](sparkSession: SparkSession, path:
             new FileSystemManager(metadataPath, hadoopConf)
         }
       }
    +
    +  /**
    +   * Parse the log version from the given `text` -- will throw exception when the parsed version
    +   * exceeds `maxSupportedVersion`, or when `text` is malformed (such as "xyz", "v", "v-1",
    +   * "v123xyz" etc.)
    +   */
    +  private[sql] def parseVersion(text: String, maxSupportedVersion: Int): Int = {
    +    if (text.length > 0 && text(0) == 'v') {
    +      val version =
    +        try { text.substring(1, text.length).toInt }
    --- End diff --
    
    done


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17070: [SPARK-19721][SS] Good error message for version mismatc...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/17070
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/73487/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17070: [WIP][SS] Good error message for version mismatch in log...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/17070
  
    **[Test build #73487 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/73487/testReport)** for PR 17070 at commit [`18f77b0`](https://github.com/apache/spark/commit/18f77b0db2ccb9198ac3fd554633fece25e26686).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17070: [SPARK-19721][SS] Good error message for version mismatc...

Posted by lw-lin <gi...@git.apache.org>.
Github user lw-lin commented on the issue:

    https://github.com/apache/spark/pull/17070
  
    @zsxwing would you take a look when you've got a minute? Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17070: [SPARK-19721][SS] Good error message for version mismatc...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/17070
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17070: [WIP][SS] Good error message for version mismatch in log...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/17070
  
    **[Test build #73476 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/73476/testReport)** for PR 17070 at commit [`dcae69b`](https://github.com/apache/spark/commit/dcae69b351ac71bf1554b760aeea6b763b2bf4c0).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17070: [SPARK-19721][SS] Good error message for version mismatc...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/17070
  
    **[Test build #74063 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74063/testReport)** for PR 17070 at commit [`18f77b0`](https://github.com/apache/spark/commit/18f77b0db2ccb9198ac3fd554633fece25e26686).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17070: [SPARK-19721][SS] Good error message for version mismatc...

Posted by lw-lin <gi...@git.apache.org>.
Github user lw-lin commented on the issue:

    https://github.com/apache/spark/pull/17070
  
    @zsxwing would you take a look when you've got a minute? Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17070: [SPARK-19721][SS] Good error message for version ...

Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17070#discussion_r106348166
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/HDFSMetadataLog.scala ---
    @@ -195,6 +195,11 @@ class HDFSMetadataLog[T <: AnyRef : ClassTag](sparkSession: SparkSession, path:
           val input = fileManager.open(batchMetadataFile)
           try {
             Some(deserialize(input))
    +      } catch {
    +        case ise: IllegalStateException =>
    +          // re-throw the exception with the log file path added
    +          throw new IllegalStateException(
    +            s"Failed to read log file $batchMetadataFile. ${ise.getMessage}")
    --- End diff --
    
    nit: please also add `ise` as the cause.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17070: [SPARK-19721][SS] Good error message for version mismatc...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/17070
  
    **[Test build #73487 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/73487/testReport)** for PR 17070 at commit [`18f77b0`](https://github.com/apache/spark/commit/18f77b0db2ccb9198ac3fd554633fece25e26686).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17070: [WIP][SS] Good error message for version mismatch in log...

Posted by lw-lin <gi...@git.apache.org>.
Github user lw-lin commented on the issue:

    https://github.com/apache/spark/pull/17070
  
    @srowen thank you for the comments! I was trying to tackle SPARK-19721, sorry the summary just said "WIP" without a JIRA number; adding JIRA number back.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17070: [SPARK-19721][SS] Good error message for version mismatc...

Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the issue:

    https://github.com/apache/spark/pull/17070
  
    @lw-lin there are conflicts with 2.1. Could you submit a new PR for branch-2.1?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17070: [SPARK-19721][SS] Good error message for version ...

Posted by lw-lin <gi...@git.apache.org>.
Github user lw-lin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17070#discussion_r106352628
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/HDFSMetadataLog.scala ---
    @@ -195,6 +195,11 @@ class HDFSMetadataLog[T <: AnyRef : ClassTag](sparkSession: SparkSession, path:
           val input = fileManager.open(batchMetadataFile)
           try {
             Some(deserialize(input))
    +      } catch {
    +        case ise: IllegalStateException =>
    +          // re-throw the exception with the log file path added
    +          throw new IllegalStateException(
    +            s"Failed to read log file $batchMetadataFile. ${ise.getMessage}")
    --- End diff --
    
    done; thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17070: [WIP][SS] Good error message for version mismatch...

Posted by lw-lin <gi...@git.apache.org>.
Github user lw-lin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17070#discussion_r103102422
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/HDFSMetadataLog.scala ---
    @@ -268,6 +274,37 @@ class HDFSMetadataLog[T <: AnyRef : ClassTag](sparkSession: SparkSession, path:
             new FileSystemManager(metadataPath, hadoopConf)
         }
       }
    +
    +  /**
    +   * Parse the log version from the given `text` -- will throw exception when the parsed version
    +   * exceeds `maxSupportedVersion`, or when `text` is malformed (such as "xyz", "v", "v-1",
    +   * "v123xyz" etc.)
    +   */
    +  private[sql] def parseVersion(text: String, maxSupportedVersion: Int): Int = {
    +    if (text.length > 0 && text(0) == 'v') {
    +      val version =
    +        try { text.substring(1, text.length).toInt }
    +        catch {
    +          case _: NumberFormatException =>
    +            throw new IllegalStateException(s"Log file was malformed: failed to read correct log " +
    +              s"version from $text.")
    +        }
    +      if (version > 0) {
    +        if (version > maxSupportedVersion) {
    +          throw new IllegalStateException(s"UnsupportedLogVersion: maximum supported log version " +
    +            s"is v${maxSupportedVersion}, but encountered v$version. The log file was produced " +
    +            s"by a newer version of Spark and cannot be read by this version. Please upgrade.")
    +        }
    +        else {
    --- End diff --
    
    done


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17070: [WIP][SS] Good error message for version mismatch...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17070#discussion_r103098735
  
    --- Diff: external/kafka-0-10-sql/src/main/scala/org/apache/spark/sql/kafka010/KafkaSource.scala ---
    @@ -100,7 +100,8 @@ private[kafka010] class KafkaSource(
             override def serialize(metadata: KafkaSourceOffset, out: OutputStream): Unit = {
               out.write(0) // A zero byte is written to support Spark 2.1.0 (SPARK-19517)
               val writer = new BufferedWriter(new OutputStreamWriter(out, StandardCharsets.UTF_8))
    -          writer.write(VERSION)
    +          writer.write("v" + VERSION)
    --- End diff --
    
    Write one string, or write this in 3 steps if you're worried about efficiency? rather than 2


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17070: [SPARK-19721][SS] Good error message for version mismatc...

Posted by lw-lin <gi...@git.apache.org>.
Github user lw-lin commented on the issue:

    https://github.com/apache/spark/pull/17070
  
    @zsxwing sure, please see https://github.com/apache/spark/pull/17327


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17070: [SPARK-19721][SS] Good error message for version mismatc...

Posted by lw-lin <gi...@git.apache.org>.
Github user lw-lin commented on the issue:

    https://github.com/apache/spark/pull/17070
  
    Jenkins retest this please


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17070: [WIP][SS] Good error message for version mismatch in log...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/17070
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/73476/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17070: [WIP][SS] Good error message for version mismatch...

Posted by lw-lin <gi...@git.apache.org>.
Github user lw-lin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17070#discussion_r103102394
  
    --- Diff: external/kafka-0-10-sql/src/main/scala/org/apache/spark/sql/kafka010/KafkaSource.scala ---
    @@ -100,7 +100,8 @@ private[kafka010] class KafkaSource(
             override def serialize(metadata: KafkaSourceOffset, out: OutputStream): Unit = {
               out.write(0) // A zero byte is written to support Spark 2.1.0 (SPARK-19517)
               val writer = new BufferedWriter(new OutputStreamWriter(out, StandardCharsets.UTF_8))
    -          writer.write(VERSION)
    +          writer.write("v" + VERSION)
    --- End diff --
    
    done


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17070: [WIP][SS] Good error message for version mismatch...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17070#discussion_r103098721
  
    --- Diff: external/kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaSourceSuite.scala ---
    @@ -226,7 +226,15 @@ class KafkaSourceSuite extends KafkaSourceTest {
             source.getOffset.get // Read initial offset
           }
     
    -      assert(e.getMessage.contains("Please upgrade your Spark"))
    +      Seq(
    --- End diff --
    
    I don't think it's useful to assert about the exact message. Assert that it has key substrings.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17070: [WIP][SS] Good error message for version mismatch in log...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/17070
  
    **[Test build #73476 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/73476/testReport)** for PR 17070 at commit [`dcae69b`](https://github.com/apache/spark/commit/dcae69b351ac71bf1554b760aeea6b763b2bf4c0).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17070: [WIP][SS] Good error message for version mismatch...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17070#discussion_r103098816
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/HDFSMetadataLog.scala ---
    @@ -268,6 +274,37 @@ class HDFSMetadataLog[T <: AnyRef : ClassTag](sparkSession: SparkSession, path:
             new FileSystemManager(metadataPath, hadoopConf)
         }
       }
    +
    +  /**
    +   * Parse the log version from the given `text` -- will throw exception when the parsed version
    +   * exceeds `maxSupportedVersion`, or when `text` is malformed (such as "xyz", "v", "v-1",
    +   * "v123xyz" etc.)
    +   */
    +  private[sql] def parseVersion(text: String, maxSupportedVersion: Int): Int = {
    +    if (text.length > 0 && text(0) == 'v') {
    +      val version =
    +        try { text.substring(1, text.length).toInt }
    --- End diff --
    
    Brace style is wrong


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17070: [WIP][SS] Good error message for version mismatch...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17070#discussion_r103098789
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/HDFSMetadataLog.scala ---
    @@ -195,6 +196,11 @@ class HDFSMetadataLog[T <: AnyRef : ClassTag](sparkSession: SparkSession, path:
           val input = fileManager.open(batchMetadataFile)
           try {
             Some(deserialize(input))
    +      } catch {
    +        case ise: IllegalStateException =>
    --- End diff --
    
    Why not just let the exception go?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17070: [WIP][SS] Good error message for version mismatch in log...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:

    https://github.com/apache/spark/pull/17070
  
    This is changing a lot of stuff to barely improve an error, and the PR has problems. I don't think this is worthwhile


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17070: [WIP][SS] Good error message for version mismatch in log...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/17070
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17070: [SPARK-19721][SS] Good error message for version ...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/17070


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17070: [WIP][SS] Good error message for version mismatch...

Posted by lw-lin <gi...@git.apache.org>.
Github user lw-lin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17070#discussion_r103102416
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/HDFSMetadataLog.scala ---
    @@ -18,6 +18,7 @@
     package org.apache.spark.sql.execution.streaming
     
     import java.io._
    +import java.lang.IllegalStateException
    --- End diff --
    
    ah, what a simple mistake :-)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17070: [SPARK-19721][SS] Good error message for version mismatc...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/17070
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/74648/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17070: [SPARK-19721][SS] Good error message for version mismatc...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/17070
  
    **[Test build #74648 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74648/testReport)** for PR 17070 at commit [`32c0017`](https://github.com/apache/spark/commit/32c00179feb730b5f30ec5282a41d60357ad322f).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17070: [WIP][SS] Good error message for version mismatch...

Posted by lw-lin <gi...@git.apache.org>.
Github user lw-lin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17070#discussion_r103102389
  
    --- Diff: external/kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaSourceSuite.scala ---
    @@ -226,7 +226,15 @@ class KafkaSourceSuite extends KafkaSourceTest {
             source.getOffset.get // Read initial offset
           }
     
    -      assert(e.getMessage.contains("Please upgrade your Spark"))
    +      Seq(
    --- End diff --
    
    done; thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17070: [SPARK-19721][SS] Good error message for version mismatc...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/17070
  
    **[Test build #74063 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74063/testReport)** for PR 17070 at commit [`18f77b0`](https://github.com/apache/spark/commit/18f77b0db2ccb9198ac3fd554633fece25e26686).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17070: [SPARK-19721][SS] Good error message for version mismatc...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/17070
  
    **[Test build #74648 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74648/testReport)** for PR 17070 at commit [`32c0017`](https://github.com/apache/spark/commit/32c00179feb730b5f30ec5282a41d60357ad322f).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17070: [SPARK-19721][SS] Good error message for version mismatc...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/17070
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17070: [WIP][SS] Good error message for version mismatch...

Posted by lw-lin <gi...@git.apache.org>.
Github user lw-lin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17070#discussion_r103102481
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/HDFSMetadataLog.scala ---
    @@ -195,6 +196,11 @@ class HDFSMetadataLog[T <: AnyRef : ClassTag](sparkSession: SparkSession, path:
           val input = fileManager.open(batchMetadataFile)
           try {
             Some(deserialize(input))
    +      } catch {
    +        case ise: IllegalStateException =>
    --- End diff --
    
    the low-level exception does not know about the log file's path, and I'm trying to put it into the error message to give users very explicit information


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17070: [SPARK-19721][SS] Good error message for version mismatc...

Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the issue:

    https://github.com/apache/spark/pull/17070
  
    /cc @zsxwing 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org