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.

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

[SPARK-17782][STREAMING][BUILD] Add Kafka 0.10 project to build modules #15355

Closed
wants to merge 4 commits into from

Conversation

hvanhovell
Copy link
Contributor

@hvanhovell hvanhovell commented Oct 5, 2016

What changes were proposed in this pull request?

This PR adds the Kafka 0.10 subproject to the build infrastructure. This makes sure Kafka 0.10 tests are only triggers when it or of its dependencies change.

@hvanhovell
Copy link
Contributor Author

cc @koeninger any idea why this flaky?

@hvanhovell hvanhovell changed the title [SPARK-17782][STREAMING]Disable Kafka 010 pattern based subscription test. [SPARK-17782][STREAMING] Disable Kafka 010 pattern based subscription test. Oct 5, 2016
@koeninger
Copy link
Contributor

I have generally been unable to reproduce these kinds of test failures on my local environment, and don't have access to the build server, so trying fix without repro is pretty much shooting randomly in the dark. It does seem unfortunate to me that we're effectively doing full integration tests on every PR, even if a patch has changed something (e.g. MLLib) that couldn't possibly affect the modules in /external

@SparkQA
Copy link

SparkQA commented Oct 5, 2016

Test build #66361 has finished for PR 15355 at commit b7074d4.

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

@rxin
Copy link
Contributor

rxin commented Oct 5, 2016

Actually we do have the infra to not run these tests if it is just an unrelated module change. Was those not setup for Kafka?

@srowen
Copy link
Member

srowen commented Oct 5, 2016

Good point, I don't see separate config for the 0.10 module in dev/sparktestsupport/modules.py though there is for 0.8 module. That could be a better way forward.

@SparkQA
Copy link

SparkQA commented Oct 6, 2016

Test build #66461 has finished for PR 15355 at commit 0bac9ff.

  • This patch fails executing the dev/run-tests script.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Oct 6, 2016

Test build #66463 has finished for PR 15355 at commit 5ba78b6.

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

@zsxwing
Copy link
Member

zsxwing commented Oct 6, 2016

@koeninger you can download the unit test logs from https://amplab.cs.berkeley.edu/jenkins/job/spark-master-test-sbt-hadoop-2.6/1756/artifact/

I saw the offsets of the first batch (pat1 0 16 16 should be pat1 0 0 16) was wrong in the unit test logs:

16/10/05 20:42:46.313 streaming-start INFO JobGenerator: Started JobGenerator at 1475725367000 ms
16/10/05 20:42:46.313 streaming-start INFO JobScheduler: Started JobScheduler
16/10/05 20:42:46.313 pool-1-thread-1-ScalaTest-running-DirectKafkaStreamSuite INFO StreamingContext: StreamingContext started
16/10/05 20:42:47.015 JobGenerator INFO JobScheduler: Added jobs for time 1475725367000 ms
16/10/05 20:42:47.016 JobScheduler INFO JobScheduler: Starting job streaming job 1475725367000 ms.0 from job set of time 1475725367000 ms
16/10/05 20:42:47.017 streaming-job-executor-0 INFO DirectKafkaStreamSuite: pat2 0 3 16
16/10/05 20:42:47.017 streaming-job-executor-0 INFO DirectKafkaStreamSuite: pat1 0 16 16

Seems the earliest config does not work.

@koeninger
Copy link
Contributor

@zsxwing good eye, thanks. It's not that auto.offset.reset.earliest doesn't work, it's that there's a potential race condition that poll gets called twice slowly enough for consumer position to be modified before topicpartitions are paused.

#15387

should address that.

It's something that whoever works on the duplicated equivalent code in the structured streaming module is going to have to address, also.

@hvanhovell
Copy link
Contributor Author

I have re-enables the kafka test. This PR now only contains a change to build infrastructure.

@hvanhovell hvanhovell changed the title [SPARK-17782][STREAMING] Disable Kafka 010 pattern based subscription test. [SPARK-17782][STREAMING][Build] Add Kafka 0.10 project to build modules Oct 7, 2016
@hvanhovell hvanhovell changed the title [SPARK-17782][STREAMING][Build] Add Kafka 0.10 project to build modules [SPARK-17782][STREAMING][BUILD] Add Kafka 0.10 project to build modules Oct 7, 2016
Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

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

This looks good and will merge as soon as it passes, though the actual fix will be in the other PR.

@SparkQA
Copy link

SparkQA commented Oct 7, 2016

Test build #66487 has finished for PR 15355 at commit a2ba414.

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

@srowen
Copy link
Member

srowen commented Oct 7, 2016

Merged to master

@asfgit asfgit closed this in 18bf9d2 Oct 7, 2016
asfgit pushed a commit that referenced this pull request Oct 7, 2016
## What changes were proposed in this pull request?
This PR adds the Kafka 0.10 subproject to the build infrastructure. This makes sure Kafka 0.10 tests are only triggers when it or of its dependencies change.

Author: Herman van Hovell <hvanhovell@databricks.com>

Closes #15355 from hvanhovell/SPARK-17782.
@zsxwing
Copy link
Member

zsxwing commented Oct 7, 2016

Also check-picked this one into branch 2.0 since it's also helpful for 2.0 backport PRs.

uzadude pushed a commit to uzadude/spark that referenced this pull request Jan 27, 2017
## What changes were proposed in this pull request?
This PR adds the Kafka 0.10 subproject to the build infrastructure. This makes sure Kafka 0.10 tests are only triggers when it or of its dependencies change.

Author: Herman van Hovell <hvanhovell@databricks.com>

Closes apache#15355 from hvanhovell/SPARK-17782.
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.

6 participants