Skip to content

CQ shared store: Delete from index on remove or roll over (backport #13959) #13987

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

Merged
merged 1 commit into from
May 30, 2025

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented May 30, 2025

It is expensive to delete files because we must clean up the index and to get the messages in the file we have to scan it.

Instead of cleaning up the index on file delete this commit deletes from the index as soon as possible. There are two scenarios: messages that are removed from the current write file, and messages that are removed from other files. In the latter case, we
can just delete the index entry on remove. For messages in the current write file, we want to keep the entry in case fanout is used, because we don't want to write the fanout message multiple times if we can avoid it. So we keep track of removes in the current write file and do a cleanup of these entries on file roll over.

Compared to the previous implementation we will no longer increase the ref_count of messages that are not in the current write file, meaning we may do more writes in fanout scenarios. But at the same time the file delete operation is much cheaper.

@mkuratczyk


This is an automatic backport of pull request #13959 done by [Mergify](https://mergify.com).

It was expensive to delete files because we had clean up
the index and to get the messages in the file we have to
scan it.

Instead of cleaning up the index on file delete this
commit deletes from the index as soon as possible.
There are two scenarios: messages that are removed
from the current write file, and messages that are
removed from other files. In the latter case, we
can just delete the index entry on remove. For messages
in the current write file, we want to keep the entry
in case fanout is used, because we don't want to write
the fanout message multiple times if we can avoid it.
So we keep track of removes in the current write file
and do a cleanup of these entries on file roll over.

Compared to the previous implementation we will no
longer increase the ref_count of messages that are
not in the current write file, meaning we may do more
writes in fanout scenarios. But at the same time the
file delete operation is much cheaper.

Additionally, we prioritise delete calls in rabbit_msg_store_gc.
Without that change, if the compaction was lagging behind,
we could have file deletion requests queued behind many compaction
requests, leading to many unnecessary compactions of files
that could already be deleted.

Co-authored-by: Michal Kuratczyk <michal.kuratczyk@broadcom.com>
(cherry picked from commit 0278980)
@michaelklishin michaelklishin added this to the 4.1.1 milestone May 30, 2025
@michaelklishin michaelklishin merged commit ee6ffd9 into v4.1.x May 30, 2025
266 of 273 checks passed
@michaelklishin michaelklishin deleted the mergify/bp/v4.1.x/pr-13959 branch May 30, 2025 16:06
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