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

distributor: fix pool buffer reuse logic when distributor.max-request-pool-buffer-size is set #9666

Merged
merged 1 commit into from
Oct 22, 2024

Conversation

ortuman
Copy link
Contributor

@ortuman ortuman commented Oct 18, 2024

What this PR does

This PR adapts the distributor code to remove the bucketed pool implementation in util.RequestBuffers when a maximum pool buffer size is defined.

Instead, the code for the default util.bufferPool type has been adapted to allow defining a maximum pool capacity.

The reason is that with the current implementatiom, bucketed buffers are used to decompress the incoming request acquiring the buffer based on the specified request size. However, since these buffers eventually end up being resized during the decompression process, they can later end up in a different bucket when returned to the pool, thus preventing subsequent reuse and leading to higher GC pressure.

This behavior has been observed in our dev environments when enabling the config parameter -distributor.max-request-pool-buffer-size.

Which issue(s) this PR fixes or relates to

Fixes n/a

Checklist

  • Tests updated.
  • Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
  • about-versioning.md updated with experimental features.

@ortuman ortuman force-pushed the ortuman/distributor/fix-max-write-buffer-pooling branch 6 times, most recently from 56166b7 to 3b074b1 Compare October 18, 2024 12:10
…t-pool-buffer-size` is set.

Signed-off-by: Miguel Ángel Ortuño <ortuman@gmail.com>
@ortuman ortuman force-pushed the ortuman/distributor/fix-max-write-buffer-pooling branch from 3b074b1 to 0dfb462 Compare October 18, 2024 13:41
@ortuman ortuman marked this pull request as ready for review October 18, 2024 14:08
@ortuman ortuman requested a review from a team as a code owner October 18, 2024 14:08
Copy link
Contributor

@narqo narqo left a comment

Choose a reason for hiding this comment

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

🔥 Works for me. I don't think there is a better way to benchmark the different approaches than against a real workload.

@ortuman ortuman merged commit 8ef8459 into main Oct 22, 2024
29 checks passed
@ortuman ortuman deleted the ortuman/distributor/fix-max-write-buffer-pooling branch October 22, 2024 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants