feat(compactor HS): add job queue and deletion job builder - #17843
Conversation
| // Dequeue retrieves the next job from the queue | ||
| rpc Dequeue(DequeueRequest) returns (DequeueResponse) {} | ||
| // ReportJobResponse reports the result of executing a job | ||
| rpc ReportJobResponse(ReportJobResultResponse) returns (ReportJobResultRequest) {} |
There was a problem hiding this comment.
This feels backward? Shouldn't it take a request and return a response?
There was a problem hiding this comment.
good catch. I don't know how I made that mistake 🤦
| // Queue implements the job queue service | ||
| type Queue struct { | ||
| queue chan *Job | ||
| closed bool |
There was a problem hiding this comment.
This feels like it might lead to a race condition? I couldn't create one, but this boolean is accessed multiple places without any synchronization.
| stop: make(chan struct{}), | ||
| checkTimedOutJobsInterval: checkTimedOutJobsInterval, | ||
| processingJobs: make(map[string]*processingJob), | ||
| jobTimeout: 15 * time.Minute, |
There was a problem hiding this comment.
Do we want to make the queue size, the timeout, or the retry max configurable?
There was a problem hiding this comment.
I didn't want to make the queue a buffered channel to build the jobs as they are picked up for processes, but I missed making the change. When any jobs fail, we want to ensure we have as few jobs in flight as possible because we stop processing manifests and abandon all the running jobs for that manifest.
We can certainly make the other two configurable, but since the code is not wired in, I have just left a TODO for now.
What this PR does / why we need it:
As a follow-up work on making Compactor horizontally scalable for processing of delete requests, this PR adds the following features:
Special notes for your reviewer:
retention.Chunkfrom byte slice to a string to simplify the new code. The byte slice was added initially to optimise memory usage with boltdb index. However, since we recommend tsdb and byte slice optimisation doesn't help much with it, I have changed the type.Checklist