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

Support for setting KafkaAdmin#operationTimeout via externalised configuration #2430

Closed
hari-mani opened this issue Oct 8, 2022 · 2 comments · Fixed by #2431
Closed

Support for setting KafkaAdmin#operationTimeout via externalised configuration #2430

hari-mani opened this issue Oct 8, 2022 · 2 comments · Fixed by #2431
Milestone

Comments

@hari-mani
Copy link

hari-mani commented Oct 8, 2022

Expected Behavior

Setting the property spring.kafka.admin.operationTimeout should set the value for KafkaAdmin#operationTimeout

Current Behavior

To set the mentioned property the application is required to create a bean of type KafkaAdmin and set value for operationTimeout using the setter as show below

 @Bean
  public KafkaAdmin kafkaAdmin() {
    KafkaAdmin kafkaAdmin = new KafkaAdmin(this.properties.buildAdminProperties());
    kafkaAdmin.setFatalIfBrokerNotAvailable(this.properties.getAdmin().isFailFast());
    kafkaAdmin.setModifyTopicConfigs(this.properties.getAdmin().isModifyTopicConfigs());
    kafkaAdmin.setOperationTimeout(1000);
    return kafkaAdmin;
  }

Context

While trying to run an application using spring-kafka I got the below exception where the operation was getting timeout due to it's inability to complete within the default value set for operationTimeout.

java.util.concurrent.TimeoutException: null
        at java.base/java.util.concurrent.CompletableFuture.timedGet(CompletableFuture.java:1960) ~[na:na]
        at java.base/java.util.concurrent.CompletableFuture.get(CompletableFuture.java:2095) ~[na:na]
        at org.apache.kafka.common.internals.KafkaFutureImpl.get(KafkaFutureImpl.java:180) ~[kafka-clients-3.3.1.jar:na]
        at org.springframework.kafka.core.KafkaAdmin.clusterId(KafkaAdmin.java:233) ~[spring-kafka-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
        at org.springframework.kafka.listener.KafkaMessageListenerContainer$ListenerConsumer.obtainClusterId(KafkaMessageListenerContainer.java:941) ~[spring-kafka-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
        at org.springframework.kafka.listener.KafkaMessageListenerContainer$ListenerConsumer.<init>(KafkaMessageListenerContainer.java:919) ~[spring-kafka-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
        at org.springframework.kafka.listener.KafkaMessageListenerContainer.doStart(KafkaMessageListenerContainer.java:376) ~[spring-kafka-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
        at org.springframework.kafka.listener.AbstractMessageListenerContainer.start(AbstractMessageListenerContainer.java:462) ~[spring-kafka-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
        at org.springframework.kafka.listener.ConcurrentMessageListenerContainer.doStart(ConcurrentMessageListenerContainer.java:226) ~[spring-kafka-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
        at org.springframework.kafka.listener.AbstractMessageListenerContainer.start(AbstractMessageListenerContainer.java:462) ~[spring-kafka-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
        at org.springframework.kafka.config.KafkaListenerEndpointRegistry.startIfNecessary(KafkaListenerEndpointRegistry.java:383) ~[spring-kafka-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
        at org.springframework.kafka.config.KafkaListenerEndpointRegistry.start(KafkaListenerEndpointRegistry.java:328) ~[spring-kafka-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
        at org.springframework.context.support.DefaultLifecycleProcessor.doStart(DefaultLifecycleProcessor.java:178) ~[spring-context-6.0.0-SNAPSHOT.jar:6.0.0-SNAPSHOT]
        at org.springframework.context.support.DefaultLifecycleProcessor$LifecycleGroup.start(DefaultLifecycleProcessor.java:356) ~[spring-context-6.0.0-SNAPSHOT.jar:6.0.0-SNAPSHOT]
        at java.base/java.lang.Iterable.forEach(Iterable.java:75) ~[na:na]
        at org.springframework.context.support.DefaultLifecycleProcessor.startBeans(DefaultLifecycleProcessor.java:155) ~[spring-context-6.0.0-SNAPSHOT.jar:6.0.0-SNAPSHOT]
        at org.springframework.context.support.DefaultLifecycleProcessor.onRefresh(DefaultLifecycleProcessor.java:123) ~[spring-context-6.0.0-SNAPSHOT.jar:6.0.0-SNAPSHOT]
        at org.springframework.context.support.AbstractApplicationContext.finishRefresh(AbstractApplicationContext.java:943) ~[spring-context-6.0.0-SNAPSHOT.jar:6.0.0-SNAPSHOT]
        at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:595) ~[spring-context-6.0.0-SNAPSHOT.jar:6.0.0-SNAPSHOT]
        at org.springframework.boot.SpringApplication.refresh(SpringApplication.java:742) ~[spring-boot-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
        at org.springframework.boot.SpringApplication.refreshContext(SpringApplication.java:433) ~[spring-boot-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
        at org.springframework.boot.SpringApplication.run(SpringApplication.java:313) ~[spring-boot-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
        at org.springframework.boot.SpringApplication.run(SpringApplication.java:1314) ~[spring-boot-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
        at org.springframework.boot.SpringApplication.run(SpringApplication.java:1303) ~[spring-boot-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]

Also the javadoc mentions that the value for operationTimeout denotes the value in seconds but it is used with unit as TimeUnit.MILLISECONDS when passed to KafkaFuture.get().

this.clusterId = client.describeCluster().clusterId().get(this.operationTimeout, TimeUnit.MILLISECONDS);
@garyrussell
Copy link
Contributor

The describeCluster() call is new, and is wrong; all the other uses use TimeUnit.SECONDS; thanks for reporting.

This code is not in any release, so I assume you are using SNAPSHOTs.

I will fix those calls to use seconds.

In any case, requests for new spring-boot auto configuration properties must be opened against that project, not here.

In the meantime, you don't need to declare your own bean, you can set the property on the auto configured bean in some other bean definition.

@Bean
NewTopic topic(KafkaAdmin admin) {
	admin.setOperationTimeout(5);
	return TopicBuilder.name("topic1").partitions(1).replicas(1).build();
}

@garyrussell
Copy link
Contributor

We also shouldn't be fetching the clusterId unless observation is enabled.

@garyrussell garyrussell reopened this Oct 8, 2022
garyrussell added a commit to garyrussell/spring-kafka that referenced this issue Oct 8, 2022
Resolves spring-projects#2430

Do not obtain the clusterId unless explicitly needed for observation.
artembilan pushed a commit that referenced this issue Oct 10, 2022
Resolves #2430

Do not obtain the clusterId unless explicitly needed for observation.
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.

2 participants