Skip to content

Conversation

@koeninger
Copy link
Contributor

i don't think this should be merged until after 1.3.0 is final

@srowen
Copy link
Member

srowen commented Feb 11, 2015

ok to test

@SparkQA
Copy link

SparkQA commented Feb 11, 2015

Test build #27301 has finished for PR 4537 at commit 6953429.

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

@zzcclp
Copy link
Contributor

zzcclp commented Feb 13, 2015

Will this RP be merged into 1.3.0?

@koeninger
Copy link
Contributor Author

I believe that it will not be merged into 1.3
On Feb 12, 2015 8:13 PM, "zzcclp" [email protected] wrote:

Will this RP be merged into 1.3.0?


Reply to this email directly or view it on GitHub
#4537 (comment).

@lausobo
Copy link

lausobo commented Feb 20, 2015

@koeninger what's the reason for not putting this into 1.3.0? Kafka 0.8.2 adds support for scala 2.11. That plus non-experimental scala 2.11 support in spark is an interesting combo for scala 2.11 projects.

Thanks in any case.

@koeninger
Copy link
Contributor Author

Spark 1.3.0 was already supposed to be frozen a while back as far as I know.

My personal gut feeling is that it'd be better to wait for kafka 0.8.2.1
bugfixes and more of a chance to test.

On Thu, Feb 19, 2015 at 10:10 PM, Luis Solano [email protected]
wrote:

@koeninger https://github.com/koeninger what's the reason for not
putting this into 1.3.0? Kafka 0.8.2 adds support for scala 2.11. That plus non-experimental
scala 2.11 support in spark
https://issues.apache.org/jira/browse/SPARK-5850 is an interesting
combo for scala 2.11 projects.

Thanks in any case.


Reply to this email directly or view it on GitHub
#4537 (comment).

@zzcclp
Copy link
Contributor

zzcclp commented Mar 18, 2015

@koeninger , kafka 0.8.2.1 was released, can it be upgrade this version?

@SparkQA
Copy link

SparkQA commented Mar 18, 2015

Test build #28805 has finished for PR 4537 at commit 407382e.

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

@koeninger
Copy link
Contributor Author

Those tests have passed locally for me 3 times in a row... if I get time later I'll try to dig in

@zeitos
Copy link
Contributor

zeitos commented Mar 19, 2015

+1 I just tested it with 2.10 and 2.11 and it also passed looks like a flaky test :s

@srowen
Copy link
Member

srowen commented Mar 19, 2015

Jenkins, retest this please

@SparkQA
Copy link

SparkQA commented Mar 19, 2015

Test build #28872 has finished for PR 4537 at commit 407382e.

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

@zzcclp
Copy link
Contributor

zzcclp commented Apr 1, 2015

@koeninger , no progress yet?

@koeninger
Copy link
Contributor Author

@zzcclp if you want to help try and figure out how reproduce this test failure outside of Jenkins, go for it.

@zzcclp
Copy link
Contributor

zzcclp commented Apr 5, 2015

@koeninger , I can't visit this url , it's 404. ??

@srowen
Copy link
Member

srowen commented Apr 5, 2015

Jenkins, retest this please

@zzcclp
Copy link
Contributor

zzcclp commented Apr 9, 2015

@koeninger , I merge this RP into master and build, it is OK. then run tests as follows:

mvn -Pscala-2.10 -Phadoop-2.3 -Pyarn -Dyarn.version=2.3.0-cdh5.1.2 -Dhadoop.version=2.3.0-cdh5.1.2 -pl core,streaming,external/kafka -DwildcardSuites=org.apache.spark.streaming.kafka.KafkaClusterSuite test

mvn -Pscala-2.10 -Phadoop-2.3 -Pyarn -Dyarn.version=2.3.0-cdh5.1.2 -Dhadoop.version=2.3.0-cdh5.1.2 -pl core,streaming,external/kafka -DwildcardSuites=org.apache.spark.streaming.kafka.KafkaStreamSuite test

all are successful.

I am new to Spark and only do little help.

@koeninger
Copy link
Contributor Author

As far as I can tell from jenkins output, it failed during MiMa checks.

But if I run

sbt '; project streaming-kafka; mima-report-binary-issues'

I get

[info] spark-streaming-kafka: found 0 potential binary incompatibilities (filtered 1)

@srowen
Copy link
Member

srowen commented Apr 15, 2015

Jenkins claims ...

[error]  * method setConsumerOffsetMetadata(java.lang.String,scala.collection.immutable.Map)scala.util.Either in class org.apache.spark.streaming.kafka.KafkaCluster does not have a correspondent in new version
[error]    filter with: ProblemFilters.exclude[MissingMethodProblem]("org.apache.spark.streaming.kafka.KafkaCluster.setConsumerOffsetMetadata")
[error]  * method setConsumerOffsets(java.lang.String,scala.collection.immutable.Map)scala.util.Either in class org.apache.spark.streaming.kafka.KafkaCluster does not have a correspondent in new version
[error]    filter with: ProblemFilters.exclude[MissingMethodProblem]("org.apache.spark.streaming.kafka.KafkaCluster.setConsumerOffsets")
[error]  * method getConsumerOffsetMetadata(java.lang.String,scala.collection.immutable.Set)scala.util.Either in class org.apache.spark.streaming.kafka.KafkaCluster does not have a correspondent in new version
[error]    filter with: ProblemFilters.exclude[MissingMethodProblem]("org.apache.spark.streaming.kafka.KafkaCluster.getConsumerOffsetMetadata")
[error]  * method getConsumerOffsets(java.lang.String,scala.collection.immutable.Set)scala.util.Either in class org.apache.spark.streaming.kafka.KafkaCluster does not have a correspondent in new version
[error]    filter with: ProblemFilters.exclude[MissingMethodProblem]("org.apache.spark.streaming.kafka.KafkaCluster.getConsumerOffsets")

@koeninger
Copy link
Contributor Author

That maybe makes a certain amount of sense... I'll try replacing the default arguments with multiple overloaded methods, see if that passes mima.

@zzcclp
Copy link
Contributor

zzcclp commented Apr 22, 2015

@koeninger , I test streaming + kakfa 0.8.2.1 with this RP, it works well, 😄

@koeninger
Copy link
Contributor Author

Glad to hear it works for you. Fixing the default arguments for mima was straightforward, but there's a lot that has changed in the test code. Work's been busy, haven't had time to fix the merge conflicts yet

@SparkQA
Copy link

SparkQA commented Apr 30, 2015

Test build #31448 has finished for PR 4537 at commit 1d896e2.

  • This patch fails PySpark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

@tdas
Copy link
Contributor

tdas commented May 1, 2015

Hey @koeninger , here is a trick I am trying out to to do targeted debugging in Jenkins. I disabled all the other tests and running only streaming tests. The whole thing runs in less than 20 minutes.

#5824

Try this on your PR to see whether you can pin point the issue?

@SparkQA
Copy link

SparkQA commented May 1, 2015

Test build #31555 has finished for PR 4537 at commit 30d991d.

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

@SparkQA
Copy link

SparkQA commented May 1, 2015

Test build #31558 has finished for PR 4537 at commit d4267e9.

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

Copy link
Contributor

Choose a reason for hiding this comment

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

If the waitUntilLeaderOffset has been added to Scala, shouldnt they be added to Python as well? Maybe that then this arbitrary sleep wont be needed.

@SparkQA
Copy link

SparkQA commented May 1, 2015

Test build #31587 has finished for PR 4537 at commit 1770abc.

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

@koeninger
Copy link
Contributor Author

scalastyle passes locally for me...

@koeninger
Copy link
Contributor Author

At any rate, I tested it out locally against 0.8.1.1 server install, with the IdempotentExample job from my blogpost. Seemed to work ok.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you put this big piece of code in a separate def (can be inside waitUntilMetadata..) outside the eventually, and then the assert just calls that function? Much easier to read.

Copy link
Contributor

Choose a reason for hiding this comment

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

And also explain in a comment what this code does? For example, I am not sure what is isr in leaderAndIsr. And what are all these conditions &&-ed together.

@SparkQA
Copy link

SparkQA commented May 1, 2015

Test build #31602 has finished for PR 4537 at commit e6dfaf6.

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

@tdas
Copy link
Contributor

tdas commented May 1, 2015

Running this couple of times to test for flakiness.

@SparkQA
Copy link

SparkQA commented May 1, 2015

Test build #31608 has finished for PR 4537 at commit 803aa2c.

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

@SparkQA
Copy link

SparkQA commented May 1, 2015

Test build #756 has finished for PR 4537 at commit 803aa2c.

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

@SparkQA
Copy link

SparkQA commented May 1, 2015

Test build #757 has finished for PR 4537 at commit 803aa2c.

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

@tdas
Copy link
Contributor

tdas commented May 2, 2015

Great! Merging this! Thanks @koeninger @helena !

@asfgit asfgit closed this in 4786484 May 2, 2015
jeanlyn pushed a commit to jeanlyn/spark that referenced this pull request May 28, 2015
i don't think this should be merged until after 1.3.0 is final

Author: cody koeninger <[email protected]>
Author: Helena Edelson <[email protected]>

Closes apache#4537 from koeninger/wip-2808-kafka-0.8.2-upgrade and squashes the following commits:

803aa2c [cody koeninger] [SPARK-2808][Streaming][Kafka] code cleanup per TD
e6dfaf6 [cody koeninger] [SPARK-2808][Streaming][Kafka] pointless whitespace change to trigger jenkins again
1770abc [cody koeninger] [SPARK-2808][Streaming][Kafka] make waitUntilLeaderOffset easier to call, call it from python tests as well
d4267e9 [cody koeninger] [SPARK-2808][Streaming][Kafka] fix stderr redirect in python test script
30d991d [cody koeninger] [SPARK-2808][Streaming][Kafka] remove stderr prints since it breaks python 3 syntax
1d896e2 [cody koeninger] [SPARK-2808][Streaming][Kafka] add even even more logging to python test
4c4557f [cody koeninger] [SPARK-2808][Streaming][Kafka] add even more logging to python test
115aeee [cody koeninger] Merge branch 'master' into wip-2808-kafka-0.8.2-upgrade
2712649 [cody koeninger] [SPARK-2808][Streaming][Kafka] add more logging to python test, see why its timing out in jenkins
2b92d3f [cody koeninger] [SPARK-2808][Streaming][Kafka] wait for leader offsets in the java test as well
3824ce3 [cody koeninger] [SPARK-2808][Streaming][Kafka] naming / comments per tdas
61b3464 [cody koeninger] [SPARK-2808][Streaming][Kafka] delay for second send in boundary condition test
af6f3ec [cody koeninger] [SPARK-2808][Streaming][Kafka] delay test until latest leader offset matches expected value
9edab4c [cody koeninger] [SPARK-2808][Streaming][Kafka] more shots in the dark on jenkins failing test
c70ee43 [cody koeninger] [SPARK-2808][Streaming][Kafka] add more asserts to test, try to figure out why it fails on jenkins but not locally
1d10751 [cody koeninger] Merge branch 'master' into wip-2808-kafka-0.8.2-upgrade
ed02d2c [cody koeninger] [SPARK-2808][Streaming][Kafka] move default argument for api version to overloaded method, for binary compat
407382e [cody koeninger] [SPARK-2808][Streaming][Kafka] update kafka to 0.8.2.1
77de6c2 [cody koeninger] Merge branch 'master' into wip-2808-kafka-0.8.2-upgrade
6953429 [cody koeninger] [SPARK-2808][Streaming][Kafka] update kafka to 0.8.2
2e67c66 [Helena Edelson] #SPARK-2808 Update to Kafka 0.8.2.0 GA from beta.
d9dc2bc [Helena Edelson] Merge remote-tracking branch 'upstream/master' into wip-2808-kafka-0.8.2-upgrade
e768164 [Helena Edelson] apache#2808 update kafka to version 0.8.2
jeanlyn pushed a commit to jeanlyn/spark that referenced this pull request Jun 12, 2015
i don't think this should be merged until after 1.3.0 is final

Author: cody koeninger <[email protected]>
Author: Helena Edelson <[email protected]>

Closes apache#4537 from koeninger/wip-2808-kafka-0.8.2-upgrade and squashes the following commits:

803aa2c [cody koeninger] [SPARK-2808][Streaming][Kafka] code cleanup per TD
e6dfaf6 [cody koeninger] [SPARK-2808][Streaming][Kafka] pointless whitespace change to trigger jenkins again
1770abc [cody koeninger] [SPARK-2808][Streaming][Kafka] make waitUntilLeaderOffset easier to call, call it from python tests as well
d4267e9 [cody koeninger] [SPARK-2808][Streaming][Kafka] fix stderr redirect in python test script
30d991d [cody koeninger] [SPARK-2808][Streaming][Kafka] remove stderr prints since it breaks python 3 syntax
1d896e2 [cody koeninger] [SPARK-2808][Streaming][Kafka] add even even more logging to python test
4c4557f [cody koeninger] [SPARK-2808][Streaming][Kafka] add even more logging to python test
115aeee [cody koeninger] Merge branch 'master' into wip-2808-kafka-0.8.2-upgrade
2712649 [cody koeninger] [SPARK-2808][Streaming][Kafka] add more logging to python test, see why its timing out in jenkins
2b92d3f [cody koeninger] [SPARK-2808][Streaming][Kafka] wait for leader offsets in the java test as well
3824ce3 [cody koeninger] [SPARK-2808][Streaming][Kafka] naming / comments per tdas
61b3464 [cody koeninger] [SPARK-2808][Streaming][Kafka] delay for second send in boundary condition test
af6f3ec [cody koeninger] [SPARK-2808][Streaming][Kafka] delay test until latest leader offset matches expected value
9edab4c [cody koeninger] [SPARK-2808][Streaming][Kafka] more shots in the dark on jenkins failing test
c70ee43 [cody koeninger] [SPARK-2808][Streaming][Kafka] add more asserts to test, try to figure out why it fails on jenkins but not locally
1d10751 [cody koeninger] Merge branch 'master' into wip-2808-kafka-0.8.2-upgrade
ed02d2c [cody koeninger] [SPARK-2808][Streaming][Kafka] move default argument for api version to overloaded method, for binary compat
407382e [cody koeninger] [SPARK-2808][Streaming][Kafka] update kafka to 0.8.2.1
77de6c2 [cody koeninger] Merge branch 'master' into wip-2808-kafka-0.8.2-upgrade
6953429 [cody koeninger] [SPARK-2808][Streaming][Kafka] update kafka to 0.8.2
2e67c66 [Helena Edelson] #SPARK-2808 Update to Kafka 0.8.2.0 GA from beta.
d9dc2bc [Helena Edelson] Merge remote-tracking branch 'upstream/master' into wip-2808-kafka-0.8.2-upgrade
e768164 [Helena Edelson] apache#2808 update kafka to version 0.8.2
nemccarthy pushed a commit to nemccarthy/spark that referenced this pull request Jun 19, 2015
i don't think this should be merged until after 1.3.0 is final

Author: cody koeninger <[email protected]>
Author: Helena Edelson <[email protected]>

Closes apache#4537 from koeninger/wip-2808-kafka-0.8.2-upgrade and squashes the following commits:

803aa2c [cody koeninger] [SPARK-2808][Streaming][Kafka] code cleanup per TD
e6dfaf6 [cody koeninger] [SPARK-2808][Streaming][Kafka] pointless whitespace change to trigger jenkins again
1770abc [cody koeninger] [SPARK-2808][Streaming][Kafka] make waitUntilLeaderOffset easier to call, call it from python tests as well
d4267e9 [cody koeninger] [SPARK-2808][Streaming][Kafka] fix stderr redirect in python test script
30d991d [cody koeninger] [SPARK-2808][Streaming][Kafka] remove stderr prints since it breaks python 3 syntax
1d896e2 [cody koeninger] [SPARK-2808][Streaming][Kafka] add even even more logging to python test
4c4557f [cody koeninger] [SPARK-2808][Streaming][Kafka] add even more logging to python test
115aeee [cody koeninger] Merge branch 'master' into wip-2808-kafka-0.8.2-upgrade
2712649 [cody koeninger] [SPARK-2808][Streaming][Kafka] add more logging to python test, see why its timing out in jenkins
2b92d3f [cody koeninger] [SPARK-2808][Streaming][Kafka] wait for leader offsets in the java test as well
3824ce3 [cody koeninger] [SPARK-2808][Streaming][Kafka] naming / comments per tdas
61b3464 [cody koeninger] [SPARK-2808][Streaming][Kafka] delay for second send in boundary condition test
af6f3ec [cody koeninger] [SPARK-2808][Streaming][Kafka] delay test until latest leader offset matches expected value
9edab4c [cody koeninger] [SPARK-2808][Streaming][Kafka] more shots in the dark on jenkins failing test
c70ee43 [cody koeninger] [SPARK-2808][Streaming][Kafka] add more asserts to test, try to figure out why it fails on jenkins but not locally
1d10751 [cody koeninger] Merge branch 'master' into wip-2808-kafka-0.8.2-upgrade
ed02d2c [cody koeninger] [SPARK-2808][Streaming][Kafka] move default argument for api version to overloaded method, for binary compat
407382e [cody koeninger] [SPARK-2808][Streaming][Kafka] update kafka to 0.8.2.1
77de6c2 [cody koeninger] Merge branch 'master' into wip-2808-kafka-0.8.2-upgrade
6953429 [cody koeninger] [SPARK-2808][Streaming][Kafka] update kafka to 0.8.2
2e67c66 [Helena Edelson] #SPARK-2808 Update to Kafka 0.8.2.0 GA from beta.
d9dc2bc [Helena Edelson] Merge remote-tracking branch 'upstream/master' into wip-2808-kafka-0.8.2-upgrade
e768164 [Helena Edelson] apache#2808 update kafka to version 0.8.2
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.

7 participants