Skip to content

KAFKA-4980: testReprocessingFromScratch unit test failure#2757

Closed
mjsax wants to merge 2 commits intoapache:trunkfrom
mjsax:minor-fix-resetintegrationtest
Closed

KAFKA-4980: testReprocessingFromScratch unit test failure#2757
mjsax wants to merge 2 commits intoapache:trunkfrom
mjsax:minor-fix-resetintegrationtest

Conversation

@mjsax
Copy link
Copy Markdown
Member

@mjsax mjsax commented Mar 29, 2017

We got test error org.apache.kafka.common.errors.TopicExistsException: Topic 'inputTopic' already exists. in some builds. Can reproduce reliably at local machine. Root cause it async "topic delete" that might not be finished before topic gets re-created.

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Mar 29, 2017

Call for review @enothereska @dguy

@asfbot
Copy link
Copy Markdown

asfbot commented Mar 29, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/2487/
Test PASSed (JDK 7 and Scala 2.10).

@asfbot
Copy link
Copy Markdown

asfbot commented Mar 29, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/2491/
Test PASSed (JDK 8 and Scala 2.11).

@asfbot
Copy link
Copy Markdown

asfbot commented Mar 29, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/2487/
Test PASSed (JDK 8 and Scala 2.12).

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Mar 29, 2017

// ignore
}

waitUntilUserTopicsGotDeleted();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: waitUntilTopicsAreDeleted

Assert.assertEquals(0, exitCode);
}

private void waitUntilUserTopicsGotDeleted() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I find this a bit difficult to follow. Could it be something like:

 final Set<String> expectedMissingTopics = Utils.mkSet(INPUT_TOPIC,
                                                              OUTPUT_TOPIC,
                                                              OUTPUT_TOPIC_2,
                                                              OUTPUT_TOPIC_2_RERUN);

        ZkUtils zkUtils = null;
        try {
            zkUtils = ZkUtils.apply(CLUSTER.zKConnectString(),
                30000,
                30000,
                JaasUtils.isZkSecurityEnabled());

            while(expectedMissingTopics.removeAll(scala.collection.JavaConversions.seqAsJavaList
                (zkUtils.getAllTopics()))) {
                Utils.sleep(100);
            }

        } finally {
            if (zkUtils != null) {
                zkUtils.close();
            }
        }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1 on what Damian said. Currently it seems to me we're adding the same topic several times to expectedMissingTopics?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That wound do IMHO. (1) assume "topic1" was not deleted. On the first check, "topic1" gets removed from expectedMissingTopics -- in the second check it does not get removed (because it's not in the set anymore) and thus the loop would exit -- even if the topic got not deleted. Thus, we need to add all topics in each loop, because we don't know if a topic was removed or not. (2) adding the same topic several times is not a problem because we use a Set

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah - you are correct. Which further highlights that i think it is currently too difficult to follow.

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Mar 29, 2017

@ijuma The error says ERROR: Could not install GRADLE_2_4_RC_2_HOME -- seems to be a build environment problem. Not related to the test.

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Mar 29, 2017

Updated.

@asfbot
Copy link
Copy Markdown

asfbot commented Mar 29, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/2517/
Test PASSed (JDK 8 and Scala 2.11).

@asfbot
Copy link
Copy Markdown

asfbot commented Mar 29, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/2513/
Test PASSed (JDK 7 and Scala 2.10).

@asfbot
Copy link
Copy Markdown

asfbot commented Mar 30, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/2513/
Test PASSed (JDK 8 and Scala 2.12).

Copy link
Copy Markdown
Contributor

@dguy dguy left a comment

Choose a reason for hiding this comment

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

LGTM

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Mar 30, 2017

@guozhangwang Call for final review and merging.

@guozhangwang
Copy link
Copy Markdown
Contributor

Is this for KAFKA-4980? If yes please update the title.

@mjsax mjsax changed the title MINOR: stabelize flaky ResetIntegrationTest KAFKA-4980: testReprocessingFromScratch unit test failure Mar 30, 2017
@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Mar 30, 2017

@guozhangwang Done. Did not know about the JIRA...

@guozhangwang
Copy link
Copy Markdown
Contributor

LGTM and merged to trunk.

@asfgit asfgit closed this in 92b7d75 Mar 30, 2017
@mjsax mjsax deleted the minor-fix-resetintegrationtest branch March 30, 2017 20:35
@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Mar 31, 2017

Should we cherry-pick this to 0.10.2 too ?

@guozhangwang
Copy link
Copy Markdown
Contributor

I have been thinking about it, but I feel it is not a blocking issue for 0.10.2 so did not cherry-pick when merging.

asfgit pushed a commit that referenced this pull request Apr 20, 2017
We got test error `org.apache.kafka.common.errors.TopicExistsException: Topic 'inputTopic' already exists.` in some builds. Can reproduce reliably at local machine. Root cause it async "topic delete" that might not be finished before topic gets re-created.

Author: Matthias J. Sax <matthias@confluent.io>

Reviewers: Ismael Juma, Damian Guy, Guozhang Wang

Closes #2757 from mjsax/minor-fix-resetintegrationtest
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