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

Optimize highwater offset collection #15285

Merged
merged 2 commits into from
Jul 27, 2023
Merged

Conversation

vivek-datadog
Copy link
Contributor

@vivek-datadog vivek-datadog commented Jul 17, 2023

What does this PR do?

  • Highwater offsets were collected in a loop with one call made to the broker for every topic partition. This was causing a large number of calls (tens of thousands as per the debug flare from AGENT-9940) to the broker and the highwater offset collection was taking more than 2min
  • This change introduces the ability to query the highwater offsets of all topic partitions within a kafka cluster in a single call
  • Also multiple early exits have been configured while fetching highwater offsets as performance optimization
  • Improved debug logs

Motivation

AGENT-9940

Additional Notes

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.

@vivek-datadog vivek-datadog added changelog/no-changelog qa/skip-qa Automatically skip this PR for the next QA labels Jul 17, 2023
@github-actions
Copy link

github-actions bot commented Jul 17, 2023

Test Results

    8 files      8 suites   6m 1s ⏱️
  47 tests   47 ✔️ 0 💤 0
192 runs  188 ✔️ 4 💤 0

Results for commit 181aa64.

♻️ This comment has been updated with latest results.

@vivek-datadog vivek-datadog force-pushed the vivek-datadog/AGENT-9940 branch 3 times, most recently from 4a16365 to 63642b5 Compare July 17, 2023 15:27
@codecov
Copy link

codecov bot commented Jul 17, 2023

Codecov Report

Merging #15285 (763b166) into master (cbe645a) will decrease coverage by 0.92%.
The diff coverage is 89.83%.

❗ Current head 763b166 differs from pull request most recent head 181aa64. Consider uploading reports for the commit 181aa64 to get more accurate results

Flag Coverage Δ
kafka_consumer 92.82% <89.83%> (-0.43%) ⬇️

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

@vivek-datadog vivek-datadog force-pushed the vivek-datadog/AGENT-9940 branch 2 times, most recently from db8ab1e to d41e638 Compare July 18, 2023 10:13
@github-actions
Copy link

The validations job has failed; please review the Files changed tab for possible suggestions to resolve.

@vivek-datadog vivek-datadog force-pushed the vivek-datadog/AGENT-9940 branch from d41e638 to 9c8be38 Compare July 19, 2023 09:14
@vivek-datadog vivek-datadog marked this pull request as ready for review July 19, 2023 09:16
@vivek-datadog vivek-datadog requested a review from a team as a code owner July 19, 2023 09:16
@vivek-datadog vivek-datadog force-pushed the vivek-datadog/AGENT-9940 branch 6 times, most recently from 972f76f to 2408083 Compare July 24, 2023 14:32
@vivek-datadog vivek-datadog marked this pull request as draft July 24, 2023 14:33
@vivek-datadog vivek-datadog force-pushed the vivek-datadog/AGENT-9940 branch from 2408083 to 514317a Compare July 24, 2023 14:53
@github-actions
Copy link

The validations job has failed; please review the Files changed tab for possible suggestions to resolve.

@vivek-datadog vivek-datadog force-pushed the vivek-datadog/AGENT-9940 branch from 61966d8 to 7ce8eb9 Compare July 24, 2023 15:28
@github-actions
Copy link

The validations job has failed; please review the Files changed tab for possible suggestions to resolve.

@vivek-datadog vivek-datadog force-pushed the vivek-datadog/AGENT-9940 branch from 7ce8eb9 to 3bef6c5 Compare July 24, 2023 15:32
@github-actions
Copy link

The validations job has failed; please review the Files changed tab for possible suggestions to resolve.

@vivek-datadog vivek-datadog force-pushed the vivek-datadog/AGENT-9940 branch from 3bef6c5 to 5980e78 Compare July 24, 2023 15:36
@github-actions
Copy link

The validations job has failed; please review the Files changed tab for possible suggestions to resolve.

@vivek-datadog vivek-datadog force-pushed the vivek-datadog/AGENT-9940 branch from 5980e78 to 89ed175 Compare July 24, 2023 18:24
@github-actions
Copy link

The validations job has failed; please review the Files changed tab for possible suggestions to resolve.

@vivek-datadog vivek-datadog force-pushed the vivek-datadog/AGENT-9940 branch from 89ed175 to 0a15bfb Compare July 24, 2023 18:29
@ghost ghost added the release label Jul 24, 2023
@vivek-datadog vivek-datadog force-pushed the vivek-datadog/AGENT-9940 branch 4 times, most recently from 02c74ba to a6017f3 Compare July 24, 2023 19:01
@ghost ghost added the documentation label Jul 24, 2023
@vivek-datadog vivek-datadog force-pushed the vivek-datadog/AGENT-9940 branch 3 times, most recently from ce6e826 to 763b166 Compare July 25, 2023 13:07
@vivek-datadog vivek-datadog changed the title [AGENT-9940] Add debug logs [AGENT-9940] Optimize highwater offset collection Jul 26, 2023
@vivek-datadog vivek-datadog force-pushed the vivek-datadog/AGENT-9940 branch from 763b166 to 181aa64 Compare July 26, 2023 18:06
@vivek-datadog vivek-datadog marked this pull request as ready for review July 26, 2023 18:07
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.

LGTM! Just some non-blocking comments

@vivek-datadog vivek-datadog merged commit cb9bd2d into master Jul 27, 2023
@vivek-datadog vivek-datadog deleted the vivek-datadog/AGENT-9940 branch July 27, 2023 13:56
vivek-datadog added a commit that referenced this pull request Jul 31, 2023
Query the highwater offsets of all topic partitions within a kafka cluster in a single call
Configure early exits while fetching highwater offsets as performance optimization
vivek-datadog added a commit that referenced this pull request Aug 4, 2023
Query the highwater offsets of all topic partitions within a kafka cluster in a single call
Configure early exits while fetching highwater offsets as performance optimization
vivek-datadog added a commit that referenced this pull request Aug 14, 2023
Query the highwater offsets of all topic partitions within a kafka cluster in a single call
Configure early exits while fetching highwater offsets as performance optimization
@vivek-datadog vivek-datadog changed the title [AGENT-9940] Optimize highwater offset collection Optimize highwater offset collection Aug 14, 2023
vivek-datadog added a commit that referenced this pull request Aug 14, 2023
* Optimize highwater offset collection (#15285)

Query the highwater offsets of all topic partitions within a kafka cluster in a single call
Configure early exits while fetching highwater offsets as performance optimization

* Reduce number of consumer creations for highwater offset collection (#15476)

- Remove optimization based on consume group and cluster id
- Explicitly close consumer instance

Context: Optimization based on consumer group and cluster id helped to reduce
number of broker calls, but it still created multiple consumer instances

As the consumer groups are collected based on kafka_connect_str and the
same is used for creating consumer at the time of highwater offset
collection, they both are bound to be part of the same cluster. This
means that highwater offset for (topic, partition) will be the same as
the check runs in the context of a single cluster. Therefore it is
beneficial to optimize based on (topic, partition) instead of
consumer group or cluster id.
@vivek-datadog vivek-datadog removed the qa/skip-qa Automatically skip this PR for the next QA label Aug 18, 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.

2 participants