Skip to content

feat(compactor HS): add a job runner for processing of deletion jobs #18058

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 2 commits into from
Jun 11, 2025

Conversation

sandeepsukhani
Copy link
Contributor

What this PR does / why we need it:
As a follow-up to work on making Compactor horizontally scalable for processing delete requests, this PR adds a job runner that executes the deletion jobs. The Deletion Job runner would download the chunks from the object storage, apply the filters from the given delete requests, upload any newly built chunks with deleted lines filtered out and build a job response which includes:

  1. List of chunk IDs to delete from storage and the index.
  2. List of chunk IDs to de-index from the index.
  3. List of new chunks to be added to the index.

Checklist

  • Tests updated

@sandeepsukhani sandeepsukhani requested a review from a team as a code owner June 11, 2025 08:12
}

// get the chunk from storage
chks, err := chunkClient.GetChunks(context.Background(), []chunk.Chunk{chk})
Copy link
Collaborator

Choose a reason for hiding this comment

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

use the ctx parameter here instead of context.Background?

var filterFuncs []filter.Func

tableInterval := retention.ExtractIntervalFromTableName(deletionJob.TableName)
for _, chunkID := range deletionJob.ChunkIDs {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we have a lot of chunks, this would be a lot of loop iterations. Should we consider using a worker pool of some sort here? Can be in a future code iteration, of course

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was going to introduce a concurrency after some testing. I have left a comment for now to avoid missing it.
Thanks!

@sandeepsukhani sandeepsukhani enabled auto-merge (squash) June 11, 2025 12:29
@sandeepsukhani sandeepsukhani merged commit 58f4fca into main Jun 11, 2025
65 checks passed
@sandeepsukhani sandeepsukhani deleted the deletion-job-runner branch June 11, 2025 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants