Skip to content

KAFKA-7584: StreamsConfig throws ClassCastException if max.in.flight.request.per.connect is specified as String#5874

Merged
guozhangwang merged 3 commits intoapache:trunkfrom
mjsax:kafka-7584-stream-config
Nov 15, 2018
Merged

KAFKA-7584: StreamsConfig throws ClassCastException if max.in.flight.request.per.connect is specified as String#5874
guozhangwang merged 3 commits intoapache:trunkfrom
mjsax:kafka-7584-stream-config

Conversation

@mjsax
Copy link
Copy Markdown
Member

@mjsax mjsax commented Nov 2, 2018

No description provided.

@mjsax mjsax added the streams label Nov 2, 2018
@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Nov 2, 2018

Call for review @guozhangwang @bbejeck @vvcephei

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Nov 2, 2018

Java11 issues again. Failing Streams tests timed out. Additionally:

java.lang.AssertionError: 
Expected: <[KeyValue(0, 0), KeyValue(0, 1), KeyValue(0, 3), KeyValue(0, 6), KeyValue(0, 10), KeyValue(0, 15), KeyValue(0, 21), KeyValue(0, 28), KeyValue(0, 36), KeyValue(0, 45), KeyValue(0, 55), KeyValue(0, 66), KeyValue(0, 78), KeyValue(0, 91), KeyValue(0, 105), KeyValue(0, 120), KeyValue(0, 136), KeyValue(0, 153), KeyValue(0, 171), KeyValue(0, 190)]>
     but: was <[KeyValue(0, 0), KeyValue(0, 1), KeyValue(0, 3), KeyValue(0, 6), KeyValue(0, 10), KeyValue(0, 15), KeyValue(0, 21), KeyValue(0, 28), KeyValue(0, 36), KeyValue(0, 45), KeyValue(0, 10), KeyValue(0, 21), KeyValue(0, 33), KeyValue(0, 46), KeyValue(0, 60), KeyValue(0, 75), KeyValue(0, 91), KeyValue(0, 108), KeyValue(0, 126), KeyValue(0, 145)]>
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:8)
	at org.apache.kafka.streams.integration.EosIntegrationTest.checkResultPerKey(EosIntegrationTest.java:218)
	at org.apache.kafka.streams.integration.EosIntegrationTest.shouldNotViolateEosIfOneTaskFailsWithState(EosIntegrationTest.java:467)

Also some non-Streams test failures.

bbejeck
bbejeck approved these changes Nov 3, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@bbejeck bbejeck left a comment

Choose a reason for hiding this comment

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

Thanks @mjsax just one minor nit otherwise LGTM

Comment thread streams/src/main/java/org/apache/kafka/streams/StreamsConfig.java Outdated
throw new ConfigException("Config value (with type " + maxInFlightRequests.getClass().getName() + " for " + ProducerConfig.MAX_IN_FLIGHT_REQUESTS_PER_CONNECTION + " could not be converted into an int.");
}

if (5 < maxInFlightRequestsAsInteger) {
Copy link
Copy Markdown
Member

@bbejeck bbejeck Nov 3, 2018

Choose a reason for hiding this comment

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

super nit: maybe reverse statement to maxInFlightRequestsAsInteger > 5 IMHO easier to grok, but this is highly opinionated, so feel free to ignore.

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Nov 4, 2018

Failing Java11 tests unrelated:

kafka.api.ClientIdQuotaTest.testThrottledProducerConsumer
kafka.api.ClientIdQuotaTest.testQuotaOverrideDelete
kafka.api.UserClientIdQuotaTest.testThrottledProducerConsumer
kafka.server.DynamicBrokerReconfigurationTest.testUncleanLeaderElectionEnable

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Nov 4, 2018

Updated this.

@bbejeck
Copy link
Copy Markdown
Member

bbejeck commented Nov 4, 2018

failures unrelated

retest this please

kamalcph
kamalcph reviewed Nov 5, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@kamalcph kamalcph 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 Nov 5, 2018

Retest this please

@vvcephei
Copy link
Copy Markdown
Contributor

vvcephei commented Nov 6, 2018

LGTM. Thanks @mjsax !

@guozhangwang guozhangwang merged commit d2f3794 into apache:trunk Nov 15, 2018
@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Nov 15, 2018

Did you cherry-pick this to 2.0 and 2.1? (earlier version are not affected).

@guozhangwang
Copy link
Copy Markdown
Contributor

guozhangwang commented Nov 15, 2018

Have not, thanks for reminding!

guozhangwang pushed a commit that referenced this pull request Nov 15, 2018
@mjsax @guozhangwang
KAFKA-7584: StreamsConfig throws ClassCastException if max.in.flight.…
d8e0f43 
…request.per.connect is specified as String (#5874)

Reviewers: Bill Bejeck <bill@confluent.io>, John Roesler <john@confluent.io>, Guozhang Wang <wangguoz@gmail.com>
guozhangwang pushed a commit that referenced this pull request Nov 15, 2018
@mjsax @guozhangwang
KAFKA-7584: StreamsConfig throws ClassCastException if max.in.flight.…
40cc817 
…request.per.connect is specified as String (#5874)

Reviewers: Bill Bejeck <bill@confluent.io>, John Roesler <john@confluent.io>, Guozhang Wang <wangguoz@gmail.com>
@guozhangwang
Copy link
Copy Markdown
Contributor

guozhangwang commented Nov 15, 2018

Cherry-picked to 2.0 and 2.1 as well.

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Nov 15, 2018

Thank you!

@mjsax mjsax deleted the kafka-7584-stream-config branch November 15, 2018 23:25
pengxiaolong pushed a commit to pengxiaolong/kafka that referenced this pull request Jun 14, 2019
@mjsax @pengxiaolong
KAFKA-7584: StreamsConfig throws ClassCastException if max.in.flight.…
8c9bbdc 
…request.per.connect is specified as String (apache#5874)

Reviewers: Bill Bejeck <bill@confluent.io>, John Roesler <john@confluent.io>, Guozhang Wang <wangguoz@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kamalcph kamalcph kamalcph left review comments

@bbejeck bbejeck bbejeck approved these changes

@guozhangwang guozhangwang guozhangwang approved these changes

Assignees

No one assigned

Labels

streams

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@mjsax @bbejeck @vvcephei @guozhangwang @kamalcph