Skip to content

MINOR: reduce commit interval and cache size for integration test#2789

Closed
mjsax wants to merge 1 commit intoapache:trunkfrom
mjsax:minor-improve-integration-test
Closed

MINOR: reduce commit interval and cache size for integration test#2789
mjsax wants to merge 1 commit intoapache:trunkfrom
mjsax:minor-improve-integration-test

Conversation

@mjsax
Copy link
Copy Markdown
Member

@mjsax mjsax commented Apr 2, 2017

No description provided.

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Apr 2, 2017

Call for review @enothereska @dguy

@asfbot
Copy link
Copy Markdown

asfbot commented Apr 2, 2017

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

@asfbot
Copy link
Copy Markdown

asfbot commented Apr 2, 2017

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

@asfbot
Copy link
Copy Markdown

asfbot commented Apr 2, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/2615/
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

@enothereska
Copy link
Copy Markdown
Contributor

@mjsax is there an error that we're trying to fix by changing the commit interval. Note that with a non-0 cache size, even with a commit interval of 0, there is still a chance that the cache dedups. So if there is a dedup error, changing the commit interval won't be sufficient.

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Apr 3, 2017

Yes -- I am aware of that. This is only about timeouts -- by default, we commit every 30 seconds, and ofter tests wait for 30 seconds to receive output -- reducing commit interval should stabilize (and maybe speed up) the tests by avoiding race conditions.

@mjsax mjsax force-pushed the minor-improve-integration-test branch from 3693b84 to d65b847 Compare April 3, 2017 17:16
@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Apr 3, 2017

Rebased to get latest test stabilizations from trunk and to trigger new build.

@asfbot
Copy link
Copy Markdown

asfbot commented Apr 3, 2017

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

@asfbot
Copy link
Copy Markdown

asfbot commented Apr 3, 2017

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

@asfbot
Copy link
Copy Markdown

asfbot commented Apr 3, 2017

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

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Apr 3, 2017

@enothereska Doesn't seem to solve it for all cases... one build fails with

java.lang.AssertionError: Condition not met within timeout 30000. Expecting 1 records from topic output- while only received 0: []
	at org.apache.kafka.test.TestUtils.waitForCondition(TestUtils.java:265)
	at org.apache.kafka.streams.integration.utils.IntegrationTestUtils.waitUntilMinKeyValueRecordsReceived(IntegrationTestUtils.java:206)
	at org.apache.kafka.streams.integration.utils.IntegrationTestUtils.waitUntilMinKeyValueRecordsReceived(IntegrationTestUtils.java:175)
	at org.apache.kafka.streams.integration.KTableKTableJoinIntegrationTest.verifyKTableKTableJoin(KTableKTableJoinIntegrationTest.java:221)
	at org.apache.kafka.streams.integration.KTableKTableJoinIntegrationTest.shouldInnerLeftJoin(KTableKTableJoinIntegrationTest.java:142)

@enothereska
Copy link
Copy Markdown
Contributor

Yeah, let me have a look.

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Apr 4, 2017

@enothereska I think it makes still sense to merge this PR -- can you have a look?

@enothereska
Copy link
Copy Markdown
Contributor

Ok LGTM. This can probably wait for @guozhangwang to be checked in since other committers might be busy with 0.10.2 bug fix release and this is not urgent.

@mjsax mjsax force-pushed the minor-improve-integration-test branch from d65b847 to 7c5ccc1 Compare April 19, 2017 21:45
@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Apr 19, 2017

Rebases this.

Call for final review and merging @guozhangwang

@guozhangwang
Copy link
Copy Markdown
Contributor

@mjsax should this go to 0.10.2 as well?

@asfbot
Copy link
Copy Markdown

asfbot commented Apr 19, 2017

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

@asfbot
Copy link
Copy Markdown

asfbot commented Apr 19, 2017

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

@asfbot
Copy link
Copy Markdown

asfbot commented Apr 19, 2017

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

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Apr 19, 2017

@guozhangwang Did not think about it -- might make sense to stabilize 0.10.2 builds, too.

@asfgit asfgit closed this in 14ab3dc Apr 20, 2017
asfgit pushed a commit that referenced this pull request Apr 20, 2017
Author: Matthias J. Sax <matthias@confluent.io>

Reviewers: Damian Guy, Eno Thereska, Guozhang Wang

Closes #2789 from mjsax/minor-improve-integration-test

(cherry picked from commit 14ab3dc)
Signed-off-by: Guozhang Wang <wangguoz@gmail.com>
@guozhangwang
Copy link
Copy Markdown
Contributor

LGTM and merged to trunk and 0.10.2.

@mjsax mjsax deleted the minor-improve-integration-test branch April 21, 2017 05:40
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.

5 participants