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

[weekly-r316] Cherry-pick: MemPostings.Commit() from prometheus#15374 #749

Draft
wants to merge 4 commits into
base: weekly-r316
Choose a base branch
from

Conversation

colega
Copy link
Contributor

@colega colega commented Nov 13, 2024

Cherry pick of: prometheus/prometheus#15374

On top of weekly-r316

Copy link
Member

@jesusvazquez jesusvazquez left a comment

Choose a reason for hiding this comment

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

LGTM

I'm also doing a follow up on the upstream PR. And we want to test this with a bit more traffic before we merge it there.

The current approach is underperforming because all appenders try to
Commit(), regardless they actually created series or not. And even if
they created series, and those series were already committed, it will
still try to grab both locks just to realize there's nothing to commit.

We'll solve that by keeping a watermark of what's already committed, and
keeping track of the series id's created by that appender.
We'll only do the commit operation if there's something to commit.

Signed-off-by: Oleg Zaytsev <[email protected]>
There's a lot of things to be done in HeadAppender.Commit().
There's no required particular order for many of them.

Sometimes multiple appenders will arrive on the same
MemPostings.Commit() call, both having created series in the past.
Both of them will wait, one of them will just waste that time, because
the first one will most likely commit all the postings needed to be
committed by the second one.

So this adds a MemPostings.TryCommit(), that will do best effort to try
to commit now, and if it's not possible right now (either because
someone is Committing, or someone is Adding), it will retry committing
later, doing other pending work in the meantime (committing samples,
etc.)

Signed-off-by: Oleg Zaytsev <[email protected]>
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