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

[SPARK-17346][SQL][Tests]Fix the flaky topic deletion in KafkaSourceStressSuite #15384

Closed
wants to merge 3 commits into from
Closed

[SPARK-17346][SQL][Tests]Fix the flaky topic deletion in KafkaSourceStressSuite #15384

wants to merge 3 commits into from

Conversation

zsxwing
Copy link
Member

@zsxwing zsxwing commented Oct 6, 2016

What changes were proposed in this pull request?

A follow up Pr for SPARK-17346 to fix flaky org.apache.spark.sql.kafka010.KafkaSourceStressSuite.

Test log: https://amplab.cs.berkeley.edu/jenkins/job/spark-master-test-sbt-hadoop-2.4/1855/testReport/junit/org.apache.spark.sql.kafka010/KafkaSourceStressSuite/_It_is_not_a_test_/

Looks like deleting the Kafka internal topic __consumer_offsets is flaky. This PR just simply ignores internal topics.

How was this patch tested?

Existing tests.

@zsxwing zsxwing changed the title [SPARK-17346][SQL]Fix the flaky topic deletion in tests [SPARK-17346][SQL][Tests]Fix the flaky topic deletion in tests Oct 6, 2016
@zsxwing
Copy link
Member Author

zsxwing commented Oct 6, 2016

/cc @tdas

@zsxwing zsxwing changed the title [SPARK-17346][SQL][Tests]Fix the flaky topic deletion in tests [SPARK-17346][SQL][Tests]Fix the flaky topic deletion in KafkaSourceStressSuite Oct 6, 2016
srowen
srowen approved these changes Oct 6, 2016
View reviewed changes
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.

Yes we shouldn't be deleting that topic

@SparkQA
Copy link

SparkQA commented Oct 6, 2016

Test build #66466 has finished for PR 15384 at commit 5158d5d.

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

@tdas
Copy link
Contributor

tdas commented Oct 6, 2016

Are you sure we are ignoring all internal topics with that filter?
Because, it maybe better to not make assumption, and rather prefix all topics we create with something, and deleted only those.

@zsxwing
Copy link
Member Author

zsxwing commented Oct 6, 2016

Are you sure we are ignoring all internal topics with that filter?
Because, it maybe better to not make assumption, and rather prefix all topics we create with something, and deleted only those.

Good point. Just use stress to filter instead.

@SparkQA
Copy link

SparkQA commented Oct 7, 2016

Test build #66471 has finished for PR 15384 at commit c7950f0.

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

@tdas
Copy link
Contributor

tdas commented Oct 7, 2016

LGTM. Merging it to master and 2.0

@tdas
Copy link
Contributor

tdas commented Oct 7, 2016

Correction. Merging to master only. Can you fix the 2.0 PR with this issue since that is still open?

@zsxwing
Copy link
Member Author

zsxwing commented Oct 7, 2016

Since deleting a normal topic may be timeout as well: https://amplab.cs.berkeley.edu/jenkins/job/spark-master-test-sbt-hadoop-2.6/1763/testReport/junit/org.apache.spark.sql.kafka010/KafkaSourceStressSuite/_It_is_not_a_test_/

I just removed the topic deletion codes. It's not necessary since we are going to shut down the kafka cluster.

@SparkQA
Copy link

SparkQA commented Oct 7, 2016

Test build #66520 has finished for PR 15384 at commit 0addb26.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class KafkaSourceStressSuite extends KafkaSourceTest

@zsxwing
Copy link
Member Author

zsxwing commented Oct 7, 2016

@tdas since this one has not yet been merged, I just updated the PR to remove the topic cleanup.

@tdas
Copy link
Contributor

tdas commented Oct 11, 2016

Merging it to master, and branch 2.0

asfgit pushed a commit that referenced this pull request Oct 11, 2016
@zsxwing @tdas
[SPARK-17346][SQL][TESTS] Fix the flaky topic deletion in KafkaSource…
a6b5e1d 
…StressSuite

## What changes were proposed in this pull request?

A follow up Pr for SPARK-17346 to fix flaky `org.apache.spark.sql.kafka010.KafkaSourceStressSuite`.

Test log: https://amplab.cs.berkeley.edu/jenkins/job/spark-master-test-sbt-hadoop-2.4/1855/testReport/junit/org.apache.spark.sql.kafka010/KafkaSourceStressSuite/_It_is_not_a_test_/

Looks like deleting the Kafka internal topic `__consumer_offsets` is flaky. This PR just simply ignores internal topics.

## How was this patch tested?

Existing tests.

Author: Shixiong Zhu <shixiong@databricks.com>

Closes #15384 from zsxwing/SPARK-17346-flaky-test.

(cherry picked from commit 75b9e35)
Signed-off-by: Tathagata Das <tathagata.das1565@gmail.com>
@asfgit asfgit closed this in 75b9e35 Oct 11, 2016
@zsxwing zsxwing deleted the SPARK-17346-flaky-test branch October 11, 2016 17:57
uzadude pushed a commit to uzadude/spark that referenced this pull request Jan 27, 2017
@zsxwing @uzadude
[SPARK-17346][SQL][TESTS] Fix the flaky topic deletion in KafkaSource…
0ce0a5d 
…StressSuite

## What changes were proposed in this pull request?

A follow up Pr for SPARK-17346 to fix flaky `org.apache.spark.sql.kafka010.KafkaSourceStressSuite`.

Test log: https://amplab.cs.berkeley.edu/jenkins/job/spark-master-test-sbt-hadoop-2.4/1855/testReport/junit/org.apache.spark.sql.kafka010/KafkaSourceStressSuite/_It_is_not_a_test_/

Looks like deleting the Kafka internal topic `__consumer_offsets` is flaky. This PR just simply ignores internal topics.

## How was this patch tested?

Existing tests.

Author: Shixiong Zhu <shixiong@databricks.com>

Closes apache#15384 from zsxwing/SPARK-17346-flaky-test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@srowen srowen srowen approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@zsxwing @SparkQA @tdas @srowen