Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EmbeddedKafkaCustomizer ignores adminTimeout in EmbeddedKafka annotation #3658

Closed
markxj opened this issue Dec 3, 2024 · 1 comment · Fixed by #3664
Closed

EmbeddedKafkaCustomizer ignores adminTimeout in EmbeddedKafka annotation #3658

markxj opened this issue Dec 3, 2024 · 1 comment · Fixed by #3664

Comments

@markxj
Copy link

markxj commented Dec 3, 2024

In what version(s) of Spring for Apache Kafka are you seeing this issue? 3.2.5

When using the @embeddedkafka annotation with a list of topics, it will occasionally timeout on a slow build pipeline with the following exception

ERROR org.springframework.boot.SpringApplication : Application run failed
org.apache.kafka.common.KafkaException: java.util.concurrent.TimeoutException
	at org.springframework.kafka.test.EmbeddedKafkaZKBroker.createTopics(EmbeddedKafkaZKBroker.java:429) ~[spring-kafka-test-3.2.5.jar:3.2.5]
	at org.springframework.kafka.test.EmbeddedKafkaZKBroker.lambda$createKafkaTopics$6(EmbeddedKafkaZKBroker.java:416) ~[spring-kafka-test-3.2.5.jar:3.2.5]
	at org.springframework.kafka.test.EmbeddedKafkaZKBroker.doWithAdmin(EmbeddedKafkaZKBroker.java:514) ~[spring-kafka-test-3.2.5.jar:3.2.5]
	at org.springframework.kafka.test.EmbeddedKafkaZKBroker.createKafkaTopics(EmbeddedKafkaZKBroker.java:415) ~[spring-kafka-test-3.2.5.jar:3.2.5]
	at org.springframework.kafka.test.EmbeddedKafkaZKBroker.afterPropertiesSet(EmbeddedKafkaZKBroker.java:325) ~[spring-kafka-test-3.2.5.jar:3.2.5]
	at org.springframework.kafka.test.context.EmbeddedKafkaContextCustomizer.customizeContext(EmbeddedKafkaContextCustomizer.java:130) ~[spring-kafka-test-3.2.5.jar:3.2.5]
	at org.springframework.boot.test.context.SpringBootContextLoader$ContextCustomizerAdapter.initialize(SpringBootContextLoader.java:443) ~[spring-boot-test-3.3.6.jar:3.3.6]

Checking in the debugger, I see that the admin timeout is set to the default 10 seconds.

Setting the adminTimeout attribute in the EmbeddedKafka annotation has no effect.

To Reproduce

Use the EmbeddedKafka annotation with the adminTimeout attribute set to something other than the default 10s.
Use a debugger and set a breakpoint in EmbeddedKafkaZKBroker.createTopics
Look at the adminTimeout property - it will be 10 seconds

Expected behavior

I believe EmbeddedKafkaContextCustomizer should call

embeddedKafkaBroker.setAdminTimeout(embeddedKafka.adminTimeout())

before calling embeddedKafkaBroker.afterPropertiesSet()

@artembilan
Copy link
Member

Confirmed as a bug.
Unlike EmbeddedKafkaCondition, the EmbeddedKafkaContextCustomizer, does not handle the mentioned adminTimeout.

It feels like the EmbeddedKafkaContextCustomizer could just delegate to that EmbeddedKafkaCondition via new static methods with optional PropertyResolver.
This way we would have all the logic in one place and it would be more stable from bugs like this.

@artembilan artembilan self-assigned this Dec 12, 2024
artembilan added a commit to artembilan/spring-kafka that referenced this issue Dec 12, 2024
…ution

Fixes: spring-projects#3658
Issue link: spring-projects#3658

When `@EmbeddedKafka` is used with Spring context, the `adminTimeout` is not resolved.
Apparently when `adminTimeout` was introduced, it was covered only by the `EmbeddedKafkaCondition`.

* Extract `EmbeddedKafkaBrokerFactory` to encapsulate an `EmbeddedKafkaBroker` creation logic
(including the mentioned `adminTimeout`)
* Replace the logic in the `EmbeddedKafkaCondition` and `EmbeddedKafkaContextCustomizer`
with that new `EmbeddedKafkaBrokerFactory`, essentially, introducing a single place of truth.
* Pull `adminTimeout(int)` property to the `EmbeddedKafkaBroker` interface,
making the logic in the `EmbeddedKafkaBrokerFactory` simpler
* Add `adminTimeout` attribute verification into tests for condition, as well as Spring-based

**Auto-cherry-pick to `3.2.x`**
artembilan added a commit that referenced this issue Dec 12, 2024
Fixes: #3658
Issue link: #3658

When `@EmbeddedKafka` is used with Spring context, the `adminTimeout` is not resolved.
Apparently when `adminTimeout` was introduced, it was covered only by the `EmbeddedKafkaCondition`.

* Extract `EmbeddedKafkaBrokerFactory` to encapsulate an `EmbeddedKafkaBroker` creation logic
(including the mentioned `adminTimeout`)
* Replace the logic in the `EmbeddedKafkaCondition` and `EmbeddedKafkaContextCustomizer`
with that new `EmbeddedKafkaBrokerFactory`, essentially, introducing a single place of truth.
* Pull `adminTimeout(int)` property to the `EmbeddedKafkaBroker` interface,
making the logic in the `EmbeddedKafkaBrokerFactory` simpler
* Add `adminTimeout` attribute verification into tests for condition, as well as Spring-based

# Conflicts:
#	spring-kafka-test/src/main/java/org/springframework/kafka/test/context/EmbeddedKafkaContextCustomizer.java
#	spring-kafka-test/src/test/java/org/springframework/kafka/test/context/EmbeddedKafkaContextCustomizerTests.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants