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

Do not check consumer_groups if the offset is invalid #15237

Merged
merged 1 commit into from
Jul 12, 2023

Conversation

FlorentClarret
Copy link
Member

@FlorentClarret FlorentClarret commented Jul 11, 2023

What does this PR do?

Do not check consumer_groups if the offset is invalid

Motivation

Additional Notes

  • This will improve performance for this check in some cases with a very large regex and a high number of groups
  • This is a refactor, existing tests are enough to validate everything is still working

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have changelog/ and integration/ labels attached
  • If the PR doesn't need to be tested during QA, please add a qa/skip-qa label.

@FlorentClarret FlorentClarret requested a review from a team as a code owner July 11, 2023 06:37
@FlorentClarret FlorentClarret changed the title Simplify OFFSET_INVALID condition Do not check consumer_groups if the offset is invalid Jul 11, 2023
@codecov
Copy link

codecov bot commented Jul 11, 2023

Codecov Report

Merging #15237 (9593f2b) into master (f2bcac8) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Flag Coverage Δ
kafka_consumer 93.30% <100.00%> (+0.15%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@github-actions
Copy link

Test Results

    8 files      8 suites   6m 33s ⏱️
  48 tests   48 ✔️ 0 💤 0
196 runs  192 ✔️ 4 💤 0

Results for commit 9593f2b.

Copy link
Contributor

@yzhan289 yzhan289 left a comment

Choose a reason for hiding this comment

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

🔥

@FlorentClarret FlorentClarret merged commit 2771cb5 into master Jul 12, 2023
26 of 28 checks passed
@FlorentClarret FlorentClarret deleted the florentclarret/kafka_consumer/invalid_offset branch July 12, 2023 15:00
iliakur pushed a commit that referenced this pull request Jul 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants