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

Stephan/revert polling removal #18090

Merged
merged 5 commits into from
Apr 30, 2024
Merged

Conversation

StephanDollberg
Copy link
Member

@StephanDollberg StephanDollberg commented Apr 26, 2024

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v24.1.x
  • v23.3.x
  • v23.2.x

Release Notes

Features

  • Re-adds the fetch_read_strategy cluster config property to select between polling and non-polling fetch implementations. Uses the non-polling fetch implementation by default.

StephanDollberg and others added 5 commits April 26, 2024 11:44
…as deprecated"

This reverts commit c735156.

There is scenarios where polling with high debounce can be used to limit
fetch perf impact (at the cost of high latency).
This reverts commit dd4676a.

Will be replaced with 23.3 commit
This reverts commit 44bde8d.

Will be replaced with 23.3 commit
The `kafka_latency_fetch_latency` metric originally measured the time
it'd take to complete one fetch poll. A fetch poll would create a fetch
plan then execute it in parallel on every shard. On a given shard
`fetch_ntps_in_parallel` would account for the majority of the execution time
of the plan.

Since fetches are no longer implemented by polling there isn't an
exactly equivalent measurement that can be assigned to the metric.

This commit instead records the duration of the first call to
`fetch_ntps_in_parallel` on every shard to the metric. This first call takes
as long as it would during a fetch poll. Hence the resulting measurement
should be close to the duration of a fetch poll.

(cherry picked from commit 44bde8d)
@StephanDollberg StephanDollberg requested a review from a team as a code owner April 26, 2024 10:44
, fetch_read_strategy(
*this,
"fetch_read_strategy",
"The strategy used to fulfill fetch requests",

Choose a reason for hiding this comment

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

Suggested change
"The strategy used to fulfill fetch requests",
"The strategy used to fulfill fetch requests.",

@ballard26 ballard26 self-requested a review April 30, 2024 20:47
Copy link
Contributor

@ballard26 ballard26 left a comment

Choose a reason for hiding this comment

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

LGTM. Looks like you just need to fix the PR body.

@piyushredpanda piyushredpanda merged commit 4675397 into dev Apr 30, 2024
18 of 22 checks passed
@piyushredpanda piyushredpanda deleted the stephan/revert-polling-removal branch April 30, 2024 21:11
@vbotbuildovich
Copy link
Collaborator

/backport v24.1.x

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.

5 participants