Address wait timeout limits in job repository

[adeutscher]

Ever since its inception, some methods in JobRepository have used event WaitAsync calls with timeouts to mitigate the possibility of a race condition leaving events uncaught. They are set in loops with a 250ms timeout whose exit conditions check the data that the awaited event implies a state for. This is functional and it prevents CPU pinning as intended, but it could surely be better.

This issue marks an official effort to address that.

Pain points in JobRepository:

• WaitForEmptyRepositoryAsync waits for _repositoryEmptyEvent. The setting of this this event implies that the number of watched jobs is zero.
• GetNextJobAsync waits for _jobsAvailableEvent if there is currently no next job. The setting of this implies that the number of available jobs is non-zero.

[adeutscher]

Picked this up already, and progress is looking good so far. GetNextJobAsync no longer needs to loop, and existing unit tests pass without adjustment.

[adeutscher]

Not as well as I'd hoped. Original local test with manual printed output was based on observing the worker at rest, not when it processed a job. Oops.

The work continues.

[adeutscher]

The current problem is because the current sync relies on the assumption that the tallies will be updated, but that assumption isn't always correct.

[adeutscher]

Results are looking better.

Test setup:

• Batch-mode loading
• Local build has some temporary print statements to make it crystal-clear when a loop iteration is happening.

Output (job loader noise has been snipped out):

worker-1  | DBG Hosting started
worker-1  | NEXT JOB ITERATION
worker-1  | NEXT JOB ITERATION
worker-1  | VRB Idempotency Monitor: Waiting for watchable events
worker-1  | VRB Worker thread for JobSubscriberManager done. Response: NotEnabled
worker-1  | VRB Worker thread for HeartbeatMonitor done. Response: NotEnabled
worker-1  | INF Health listening on port 8081
worker-1  | WaitForEmptyRepositoryAsync
worker-1  | NEXT JOB ITERATION
worker-1  | NEXT JOB ITERATION
worker-1  | VRB Executor 0 received message UNKNOWN
worker-1  | INF Sleeping for 36 seconds
< Job Loader Noise >
worker-1  | VRB Executor 0 finished processing message UNKNOWN. Result: Success
worker-1  | NEXT JOB ITERATION

Takeaways:

• Iteration in GetNextJobAsync is working as I'd hoped. When a new job arrives and the event is unset, both threads that were waiting on the event are unleashed to try again. One succeeds in a thread-safe manner, while the other one goes back to waiting. This is acceptable.
• WaitForEmptyRepositoryAsync is clearly only being called once and JOBS__USE_LOADER_MODE is very specifically set to false, but the fact that I had to snip out the job loader noise in the section above smells an awful lot like loader mode. Need to investigate what's up with that.

[adeutscher]

Sanity-checked that the problem in the second bullet-point isn't happening in develop, so it's 100% an introduced problem.

[adeutscher]

Confirmed that this wasn't because of an oversight in an invoking method like the state in the idempotency monitor, this is a problem where I chose to set the empty repository event.

The batch-mode loader is pretty straightforward (not a surprise, given that it's a direct adaptation of template's original structuring):

On a clean run:

1. Pull jobs
2. If there are no jobs, throws an exception to mark a reason to wait in the greater loader loop.
3. Submit jobs to intake.
4. Wait until the repository is empty via WaitForEmptyRepositoryAsync

So if WaitForEmptyRepositoryAsync wraps up prematurely, then batch mode loops back to executing steps 1 and 2.

[adeutscher]

New findings:

• Placement of syncing for the empty-repository event wasn't bad, though it wasn't great either.
• Logic within syncing was flipped, a remnant of how it was based off of the jobs-available event.
• The boolean field that prevents redundant get/reset calls to the event (pure paranoia, I know) wasn't set to true by default to mirror the established event.

Testing the fixes to these findings now.

[adeutscher]

Success:

worker-1  | DBG Hosting started
worker-1  | NEXT JOB ITERATION
worker-1  | NEXT JOB ITERATION
worker-1  | VRB Idempotency Monitor: Waiting for watchable events
worker-1  | VRB Worker thread for JobSubscriberManager done. Response: NotEnabled
worker-1  | VRB Worker thread for HeartbeatMonitor done. Response: NotEnabled
worker-1  | INF Health listening on port 8081
< Redacted job loader noise from before I queued a job >
worker-1  | VRB Fetching up to 10 messages from RabbitMQ Queue: RabbitQueue
worker-1  | VRB Fetched 1 jobs in 00:00:00.0025353
worker-1  | NEXT JOB ITERATION
worker-1  | NEXT JOB ITERATION
worker-1  | WaitForEmptyRepositoryAsync
worker-1  | VRB Executor 1 received message UNKNOWN
worker-1  | INF Sleeping for 24 seconds
worker-1  | VRB Executor 1 finished processing message UNKNOWN. Result: Success
worker-1  | NEXT JOB ITERATION
< Job loader noise resumes >

[adeutscher]

Did a quick test to make sure that I didn't break Loader mode by accident by working in a similar area. Loaded 20 not-insignificant jobs at the same time with a maximum backlog size of 10. Successfully had loader mode backing off as expected.

Looking this over again gave me the idea for a minimum batch size for loader mode, but that's firmly out of scope for this branch/issue.

[adeutscher]

Remaining issues:

• Some of these improvements ramped up the cognitive complexity of GetNextJobAsync significantly. Refactoring a bit to make Sonar happy.
• Double-check unit tests locally before publishing PR.

[adeutscher]

Unit tests are in a good state, so I'm publishing the PR and doing a final local test run. Will probably merge later this evening.

[adeutscher]

Acknowledging as a follow-up that this issue doesn't cover the parameterized timeout through JobRepository.WaitForJobDemandAsync. The main method that I wanted to fix with this was always GetNextJobAsync, and I did my best to constrain this branch to just JobRepository.

Revisiting WaitForJobDemandAsync will be a matter for an issue more focused on LoaderModeJobLoader.