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.

Sign up for GitHub

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

Jump to bottom

Wrong comparison of server bootstrap addresses in KafkaTemplate.afterSingletonsInstantiated #3466

Closed
bleipold1 opened this issue Aug 29, 2024 · 2 comments · Fixed by #3471
Closed

Wrong comparison of server bootstrap addresses in KafkaTemplate.afterSingletonsInstantiated #3466

bleipold1 opened this issue Aug 29, 2024 · 2 comments · Fixed by #3471
Labels
for: backport-to-3.1.x for: backport-to-3.2.x status: backported type: enhancement
Milestone
3.3.0-M3

Comments

@bleipold1
Copy link

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

3.2.3

Describe the bug

In line 490 of KafkaTemplate are the bootstrap server addresses of the producer factory and the bootstrap server addresses of the kafka admin bean compared with each other. In most cases this will fail, because producerServers is a list of strings and adminServers always string. Even if the string represents the same list.

To Reproduce

Expected behavior

If producerServers and adminServers represent the same bootstrap servers, the if branch should not be entered.
@sobychacko
Copy link
Contributor

@bleipold1 Thanks for the report. We will look into this soon.

@sobychacko
Copy link
Contributor

sobychacko commented Aug 29, 2024 •
edited
Loading

@bleipold1 While it looks like an issue, we are curious how you encountered this issue. If they don't match (even by type), then we get into the conditional for creating a new KafkaAdmin.

Also, this only happens if you have a Boot application because you always get the bootstrap server config from Boot as a List.

In summary, this doesn't look like a bug, but an optimization on KafkaTempate, i.e. if they are equal, then we don't need to create a new KafkaAdmin.

sobychacko added a commit to sobychacko/spring-kafka that referenced this issue Aug 29, 2024
@sobychacko
spring-projectsGH-3466: Optimize KafkaAdmin creation in KafkaTemplate
650d09b 
Fixes: spring-projects#3466

spring-projects#3466

Improve bootstrap-server config comparison to avoid unnecessary
KafkaAdmin recreation. This addresses inconsistencies between
List<String> and String configurations for bootstrap servers.

The change ensures that List versions of bootstrap-server configs are
converted to regular Strings by removing brackets. This allows for
consistent comparison between producer and admin configurations.

This optimization is particularly relevant for Spring Boot scenarios
where configs may be provided in different formats but represent the
same underlying values.
sobychacko added a commit to sobychacko/spring-kafka that referenced this issue Aug 29, 2024
@sobychacko
spring-projectsGH-3466: Optimize KafkaAdmin creation in KafkaTemplate
5fe8cae 
Fixes: spring-projects#3466

spring-projects#3466

Improve bootstrap-server config comparison to avoid unnecessary
KafkaAdmin recreation. This addresses inconsistencies between
List<String> and String configurations for bootstrap servers.

The change ensures that List versions of bootstrap-server configs are
converted to regular Strings by removing brackets. This allows for
consistent comparison between producer and admin configurations.

This optimization is particularly relevant for Spring Boot scenarios
where configs may be provided in different formats but represent the
same underlying values.
sobychacko added a commit to sobychacko/spring-kafka that referenced this issue Aug 29, 2024
@sobychacko
spring-projectsGH-3466: Optimize KafkaAdmin creation in KafkaTemplate
cc99c9a 
Fixes: spring-projects#3466

spring-projects#3466

Improve bootstrap-server config comparison to avoid unnecessary
KafkaAdmin recreation. This addresses inconsistencies between
List<String> and String configurations for bootstrap servers.

The change ensures that List versions of bootstrap-server configs are
converted to regular Strings by removing brackets. This allows for
consistent comparison between producer and admin configurations.

This optimization is particularly relevant for Spring Boot scenarios
where configs may be provided in different formats but represent the
same underlying values.
@sobychacko sobychacko mentioned this issue Aug 29, 2024
Merged
@sobychacko sobychacko added this to the 3.3.0-M3 milestone Aug 29, 2024
spring-builds pushed a commit that referenced this issue Aug 30, 2024
@sobychacko @spring-builds
GH-3466: Optimize KafkaAdmin creation in KafkaTemplate (#3471)
ba179aa 
* GH-3466: Optimize KafkaAdmin creation in KafkaTemplate

Fixes: #3466

#3466

Improve bootstrap-server config comparison to avoid unnecessary
KafkaAdmin recreation. This addresses inconsistencies between
List<String> and String configurations for bootstrap servers.

The change ensures that List versions of bootstrap-server configs are
converted to regular Strings by removing brackets. This allows for
consistent comparison between producer and admin configurations.

This optimization is particularly relevant for Spring Boot scenarios
where configs may be provided in different formats but represent the
same underlying values.

* Addressing PR review

(cherry picked from commit 1d4f7f9)
spring-builds pushed a commit that referenced this issue Aug 30, 2024
@sobychacko @spring-builds
GH-3466: Optimize KafkaAdmin creation in KafkaTemplate (#3471)
552d456 
* GH-3466: Optimize KafkaAdmin creation in KafkaTemplate

Fixes: #3466

#3466

Improve bootstrap-server config comparison to avoid unnecessary
KafkaAdmin recreation. This addresses inconsistencies between
List<String> and String configurations for bootstrap servers.

The change ensures that List versions of bootstrap-server configs are
converted to regular Strings by removing brackets. This allows for
consistent comparison between producer and admin configurations.

This optimization is particularly relevant for Spring Boot scenarios
where configs may be provided in different formats but represent the
same underlying values.

* Addressing PR review

(cherry picked from commit 1d4f7f9)
This was referenced Aug 30, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
for: backport-to-3.1.x for: backport-to-3.2.x status: backported type: enhancement
Projects
None yet
Milestone
3.3.0-M3
Development

Successfully merging a pull request may close this issue.

GH-3466: Optimize KafkaAdmin creation in KafkaTemplate sobychacko/spring-kafka
4 participants
@sobychacko @artembilan @spring-builds @bleipold1