Skip to content

[#3686] Fix getting Kafka consumer positions; fix integration tests.#3692

Merged
sophokles73 merged 1 commit intoeclipse-hono:masterfrom
calohmn:PR/fix_kafka_integration_tests
Sep 18, 2025
Merged

[#3686] Fix getting Kafka consumer positions; fix integration tests.#3692
sophokles73 merged 1 commit intoeclipse-hono:masterfrom
calohmn:PR/fix_kafka_integration_tests

Conversation

@calohmn
Copy link
Contributor

@calohmn calohmn commented Sep 6, 2025

This fixes #3686.

The changes here address 2 causes for test-timeout issues:

  • long offset-commit retries on already deleted topics due to premature topic-deletion
  • long consumer.position(topicPartition) requests on deleted topics

The first cause is mitigated by first making sure that a topic has been assigned to the consumer before deleting it (and by the change in "isOffsetsCommitNeededForTopic()" as a safeguard).
For the 2nd cause, a dedicated getPositionIfTopicExists() method has been added.

@calohmn calohmn added the bug label Sep 6, 2025
@calohmn calohmn requested a review from sophokles73 as a code owner September 6, 2025 07:36
@calohmn calohmn force-pushed the PR/fix_kafka_integration_tests branch from e06b600 to 81874ad Compare September 6, 2025 08:41
@calohmn calohmn force-pushed the PR/fix_kafka_integration_tests branch from 81874ad to 270a5a5 Compare September 6, 2025 09:07
public boolean isOffsetsCommitNeededForTopic(final String topic) {
public synchronized boolean isOffsetsCommitNeededForTopic(final String topic) {
if (offsetsMap.keySet().stream().noneMatch(tp -> tp.topic().equals(topic))) {
LOG.info("isOffsetsCommitNeededForTopic: no offsets stored for topic {} (yet); offset commit may be needed after having fetched positions",
Copy link
Contributor

Choose a reason for hiding this comment

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

will this happen regularly and often in production? Or is this just a safeguard against the peculiarities of our integration tests where we permanently create and delete tenants/topics? If it is the former, then we should probably use DEBUG level ...

Copy link
Contributor

@sophokles73 sophokles73 left a comment

Choose a reason for hiding this comment

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

LGTM

@sophokles73 sophokles73 merged commit cfe860a into eclipse-hono:master Sep 18, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Failing Kafka-related integration tests

2 participants