Skip to content

KAFKA-9748: Extend Streams integration tests for EOS beta#8441

Merged
mjsax merged 2 commits intoapache:trunkfrom
mjsax:kafka-9748-eos-integration-tests
Apr 9, 2020
Merged

KAFKA-9748: Extend Streams integration tests for EOS beta#8441
mjsax merged 2 commits intoapache:trunkfrom
mjsax:kafka-9748-eos-integration-tests

Conversation

@mjsax
Copy link
Copy Markdown
Member

@mjsax mjsax commented Apr 7, 2020

The original PR was only considering EosIntegrationTest. However, there are actually a few more integration test with eos-alpha enabled that we missed to update.

Call for review @abbccdda @guozhangwang

@mjsax mjsax added streams tests Test fixes (including flaky tests) labels Apr 7, 2020
produceInitialGlobalTableValues(true);
}

private void produceInitialGlobalTableValues(final boolean enableTransactions) throws Exception {
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.

We always set it to true -- removing unnecessary code.

taskId,
Task.TaskType.ACTIVE,
EXACTLY_ONCE.equals(streamsConfig.getString(StreamsConfig.PROCESSING_GUARANTEE_CONFIG)),
StreamThread.eosEnabled(streamsConfig),
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.

Side cleanup

{StreamsConfig.EXACTLY_ONCE},
{StreamsConfig.AT_LEAST_ONCE}
{StreamsConfig.EXACTLY_ONCE_BETA}
});
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.

General question: we record the same metric for all three processing modes. Why do you need to parametrize this test? \cc @cadonna

Copy link
Copy Markdown

@abbccdda abbccdda left a comment

Choose a reason for hiding this comment

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

LGTM, with a comment


@Test
public void shouldRestoreAndProgressWhenTopicWrittenToDuringRestorationWithEosEnabled() throws Exception {
public void shouldRestoreAndProgressWhenTopicWrittenToDuringRestorationWithEosAlphaEnabled() throws Exception {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Could we extract the test portion as a common function?

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Apr 8, 2020

Both Jenkins runs passed. Magic!

Copy link
Copy Markdown

@abbccdda abbccdda left a comment

Choose a reason for hiding this comment

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

LGTM! Merge after jenkins passed

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Apr 8, 2020

Build failed with

23:01:46 [ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/kafka-pr-jdk11-scala2.13@2/clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerCoordinator.java:332:5: NPath Complexity is 588 (max allowed is 500). [NPathComplexity]

Seems to be fixed via #8447

Can retest after more reviews.

Copy link
Copy Markdown
Contributor

@guozhangwang guozhangwang left a comment

Choose a reason for hiding this comment

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

LGTM. Please feel free after green builds (maybe also run locally multiple times to capture any flakiness regression earlier) and comments addressed.

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Apr 8, 2020

Retest this please.

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Apr 9, 2020

Java 8 passed.
Java 11: kafka.api.ConsumerBounceTest.testRollingBrokerRestartsWithSmallerMaxGroupSizeConfigDisruptsBigGroup

Retest this please.

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Apr 9, 2020

Java 8 passed.
Java 11: kafka.api.SaslGssapiSslEndToEndAuthorizationTest.testNoDescribeProduceOrConsumeWithoutTopicDescribeAcl

Different test as before. Also run the tests locally multiple times without issues. Merging this.

@mjsax mjsax merged commit 73ec730 into apache:trunk Apr 9, 2020
@mjsax mjsax deleted the kafka-9748-eos-integration-tests branch April 9, 2020 17:57
@mjsax mjsax added the kip Requires or implements a KIP label Jun 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kip Requires or implements a KIP streams tests Test fixes (including flaky tests)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants