Repository navigation
grpc ensure FIFO order of jobs stored by the server - #2209
Conversation
use a monotonically increasing ticket number associated with jobs to ensure that workers solve jobs in FIFO order
tmckayus
added
bug
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes.
 |
| Layer / File(s) | Summary |
|---|---|
Queue state and sequence contract cpp/src/grpc/server/grpc_job_queue.hpp, cpp/src/grpc/server/grpc_server_types.hpp, cpp/src/grpc/server/grpc_server_main.cpp |
The shared-memory slot definition includes enqueue sequence state. Server state initializes the next sequence to 1, and queue initialization sets each slot sequence to 0. |
Publish and claim jobs cpp/src/grpc/server/grpc_job_management.cpp, cpp/src/grpc/server/grpc_job_queue.hpp, cpp/src/grpc/server/grpc_worker.cpp, cpp/tests/linear_programming/grpc/* |
Both asynchronous submission paths publish through a helper that assigns a sequence and sets the slot ready. Workers claim the eligible slot with the lowest nonzero sequence. Submission logs include the assigned sequence. Tests cover ordered claims, worker assignment, and concurrent claims. |
Reset and release slot state cpp/src/grpc/server/grpc_job_queue.hpp, cpp/src/grpc/server/grpc_worker.cpp, cpp/src/grpc/server/grpc_worker_infra.cpp, cpp/tests/linear_programming/grpc/grpc_job_queue_test.cpp |
Slot reset clears readiness before resetting the enqueue sequence and remaining slot metadata. Failed-worker cleanup uses the shared reset helper. Tests check reset behavior. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix
Possibly related issues
- Issue 2210 describes slot scanning that can select jobs out of arrival order and starve waiting jobs. This PR assigns sequences and claims the lowest-sequence ready job.
Merge Risk: 
512e0
-
The queue changes are ready to merge after normal checks; no concrete ordering or slot-reuse issue remains identified.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 56.25% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 7 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title "grpc ensure FIFO order of jobs stored by the server" matches the main change. The PR adds a monotonically increasing enqueue sequence (ticket number) to job slots, and workers claim the rea… Description check The description explains the problem (client starvation when early clients keep resubmitting), the fix (monotonic ticket numbers, workers select the lowest ticket), and the testing harness with 100 cl… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title "grpc ensure FIFO order of jobs stored by the server" matches the main change. The PR adds a monotonically increasing enqueue sequence (ticket number) to job slots, and workers claim the ready job with the lowest sequence, which gives FIFO ordering. The title is specific and short, and it names the affected area (grpc server).
Full details: Description check
-
Explanation
The description explains the problem (client starvation when early clients keep resubmitting), the fix (monotonic ticket numbers, workers select the lowest ticket), and the testing harness with 100 clients and roughly 30,000 jobs. These points match the raw summary, which covers the enqueue sequence, the claim logic in grpc_job_queue.hpp, and the new concurrent test.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 56.25% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 7 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
|
/ok to test 5803e45 |
CI Test Summary1 failed · 20 passed · 0 skipped
|
|
/merge |
The grpc server algorithm to read the job data structure could result in client starvation if multiple clients submitted to the server at the same time, and the clients submitting the first jobs to run continuously submitted new jobs whenever it's current job completed.
Add a monotonically increasing ticket number associated with jobs to ensure that workers solve jobs in FIFO order. Workers now scan the job structure to find the lowest ticket number and run that job.
This was tested and verified on a harness that identified the original problem: run 100 clients in separate processes for 5 minutes and check the job completion total of each client. Without the fix, most gRPC clients would starve under this scenario. With this fix, none starve and over roughly 30,000 jobs the totals are roughly equal.