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

rangefeed: add memory adjuster before publishing events #120348

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

wenyihu6
Copy link
Contributor

@wenyihu6 wenyihu6 commented Mar 12, 2024

We over-accounts at the beginning to account for everything that could have happened. If the memory turns out to be not needed, we de-allocates the memory right away to prevent registrations from holding onto unnecessary memory for too long.

Copy link

blathers-crl bot commented Mar 12, 2024

Your pull request contains more than 1000 changes. It is strongly encouraged to split big PRs into smaller chunks.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@wenyihu6 wenyihu6 force-pushed the finalpr branch 8 times, most recently from fe37f9b to a7586d1 Compare March 14, 2024 06:56
@wenyihu6 wenyihu6 changed the title [wip] rangefeed: add memory adjuster before publishing events rangefeed: add memory adjuster before publishing events Mar 14, 2024
To prevent OOMs, we previously introduced memory budget to limit event buffering
to processor.eventC channel. These events could point to large underlying data
structure and keeping them in the channel prevent them from being garbage
collected. However, the current memory accounting did not have a good way to
predict the memory footprint of events. It uses calculateDateSize which 1. does
not account for memory of the new RangeFeedEvents that could be spawned (such as
checkpoint events) 2. inaccurately uses protobuf-encoded wire size for data 3.
does not account for memory of the base structs.

This patch improves the memory estimation by resolving the three points above.
It tries its best effort to account for base struct's memory, predict generation
of new events, and use actual data size rather than compressed protobuf size.
Since it is challenging to predict whether there would be a new checkpoint
event, our strategy in this PR is over-accounting >> under-accounting. GitHub
issue (todo) tracks the remaining work to improve accuracy.

Resolves: cockroachdb#114190
Release note: None
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