riverqueue / river

Fast and reliable background jobs in Go
https://riverqueue.com
Mozilla Public License 2.0
3.34k stars 89 forks source link

Have client's `TestOnly` activate `AsyncCompleter` instead of `BatchCompleter` #441

Open brandur opened 2 months ago

brandur commented 2 months ago

I was looking into why River's example tests were so incredibly slow. By far the biggest reason was that maintenance services were jittering on start up, but by far the second biggest reason is that the batch completer pauses before carrying out any completion (waiting to see if anymore jobs will come in to add to the completion batch), which delays subscribers from receiving a confirmation that a job's been finished.

Here, I propose that we have the TestOnly flag activate the async completer instead of the normal batch completer. This removes the pause mentioned above, and especially in test cases where job volume is expected to be low, will have no other performance disadvantage. The batch completer is generally more complicated, so this will probably also lower memory overhead.

There is some risk in removing the batch completer from standard test usage in that it means less load on the batch completer, but I think is probably okay because the batch completer's test suite in isolation is very comprehensive.

An alternative to this is might be to make the client's completer configurable through Config. I thought about that approach, but it has the downside of exposing more API, and the async completer will be more appropriate for use in 99.9% of test cases anyway, so it's an extra complication with no real advantage.

brandur commented 2 months ago

@bgentry This one's a little more invasive, but wanted to get your thoughts on it. It'll make any integration-level tests that needs to wait for a job to round trip 100 ms or so faster.

brandur commented 2 months ago

Thanks!

Yeah, thinking a little more on this one, I think I'm going to put it on ice for a while. If the test suite improvement was like 20% or some big number like that, it'd be totally worth it IMO, but it does seem to be only a few seconds right now, putting the cost/benefit more into question.