[FLINK-26387][connector/kafka] Use individual multi-broker cluster in broker failure tests #18965
Conversation
There was a problem hiding this comment.
Thanks @PatrickRen for the contribution, I left several comments
...r-kafka/src/test/java/org/apache/flink/streaming/connectors/kafka/KafkaConsumerTestBase.java
Outdated
Show resolved
Hide resolved
| hasBeenCheckpointedBeforeFailure = hasBeenCheckpointed; | ||
| killedLeaderBefore = true; |
There was a problem hiding this comment.
This should be a bug in your last PR and now you correct this IIUC, aha?
There was a problem hiding this comment.
Yep exactly 👍 Sorry for being so careless
...r-kafka/src/test/java/org/apache/flink/streaming/connectors/kafka/KafkaConsumerTestBase.java
Outdated
Show resolved
Hide resolved
|
|
||
| private KafkaConsumer<Void, Void> createTempConsumer() { | ||
| Properties consumerProps = new Properties(); | ||
| consumerProps.putAll(getStandardProperties()); | ||
| consumerProps.setProperty( | ||
| ConsumerConfig.KEY_DESERIALIZER_CLASS_CONFIG, | ||
| VoidDeserializer.class.getCanonicalName()); | ||
| consumerProps.setProperty( | ||
| ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG, | ||
| VoidDeserializer.class.getCanonicalName()); | ||
| consumerProps.setProperty(ConsumerConfig.ALLOW_AUTO_CREATE_TOPICS_CONFIG, "false"); | ||
| return new KafkaConsumer<>(consumerProps); | ||
| } |
There was a problem hiding this comment.
plz check the IDE warning before open a PR thus we can avoid this kind of minor issue.
There was a problem hiding this comment.
Thanks for the reminder! This class has too much warnings (technical debt) so I didn't notice this one 😞 Maybe we need a giant refactor on Kafka test utils in the future. They are deeply bound with the legacy FlinkKafkaProducer and FlinkKafkaConsumer now so a lots of horrible warnings in IDE.
...r-kafka/src/test/java/org/apache/flink/streaming/connectors/kafka/KafkaConsumerTestBase.java
Outdated
Show resolved
Hide resolved
… broker failure tests
|
Thanks @leonardBang for the review! I made another push just now. Please have a look when you are available. |
…er failure tests This closes apache#18965.
…er failure tests This closes apache#18965.
…er failure tests This closes apache#18965.
What is the purpose of the change
This pull request fixes a flaky broker failure test that should use multi-broker cluster for replicating partitions during the test run.
Brief change log
Verifying this change
This change is already covered by existing broker failure tests.
Does this pull request potentially affect one of the following parts:
@Public(Evolving): (yes / no)Documentation