Skip to content

Conversation

@Jamesbarford
Copy link
Contributor

I see a problem with this;

If a benchmark request (req1) is marked as complete while it still has unfinished optional jobs, and a subsequent benchmark request (req2) is marked as in_progress, then any optional jobs belonging to req1 will be skipped.

If this were implemented using a generic is_optional flag, those jobs would also be skipped, because the request would be considered complete before the optional jobs were ever picked up.

The way I see to rectify this is to change how the dequeue query logic works for the ordering. Moreover is_optional is perhaps misleading as a term though I'm struggling to think of a better name. However I'd be interested to know your thoughts before doing so.


For the ui should we just filter optional jobs out?

@Jamesbarford Jamesbarford force-pushed the feat/optional-benchmark-jobs branch from 74185e7 to 3e6b6cb Compare December 8, 2025 11:14
@Jamesbarford Jamesbarford requested a review from Kobzol December 8, 2025 11:14
@Kobzol
Copy link
Member

Kobzol commented Dec 8, 2025

then any optional jobs belonging to req1 will be skipped.

I didn't understand this part. Once we enqueue a job into the job queue, it will always become benchmarked, right? We don't even need to modify the dequeue logic or anything. All that has to change is create some jobs with the optional flag, and change the "is request completed" logic to completely ignore optional jobs.

@Jamesbarford
Copy link
Contributor Author

I could be mistaken or over complicating what we want here.

If a job is optional then I was thinking we don't want to take an optional job into consideration when marking a benchmark request as complete.

Thus if a benchmark request is marked as success or failure but it had 2 optional jobs remaining they, I think, would be skipped as we would not look to dequeue a job from a benchmark request who's status is success or failure?

@Kobzol
Copy link
Member

Kobzol commented Dec 8, 2025

When we dequeue a job, we don't look at its benchmark request, right? Except for loading some metadata from it. So the collector doesn't really care if the request is completed/failed or not, it will just dequeue the job regardless.

@Jamesbarford
Copy link
Contributor Author

Yeah you're right 👍 we don't, so this should just work 😄

@Jamesbarford Jamesbarford force-pushed the feat/optional-benchmark-jobs branch from 3e6b6cb to 084675f Compare December 8, 2025 15:25
@Kobzol
Copy link
Member

Kobzol commented Dec 8, 2025

(it's still draft, is it ready for a review? :) )

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