Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BatchCompleter: batch all ops, not just completed #617

Merged
merged 1 commit into from
Sep 25, 2024

Conversation

bgentry
Copy link
Contributor

@bgentry bgentry commented Sep 24, 2024

This adds a JobSetStateIfRunningMany query and corresponding driver API, with implementations for both pgxv5 and database/sql.

The BatchCompleter was updated to use this new query and to batch all operations, not only those moving to a complete state. This means the AsyncCompleter (as well as the InlineCompleter) are both now unused and could be deleted, along with their underlying queries.

The intention of this is not just to facilitate improved performance even on snoozes, retries, errors, cancellations, etc., but also to get down to a single path for completions (similar to now having a single path for insertions).

Benchmarks

These changes show a slight decrease in performance for the case of 100% completed jobs, but with the benefit of a significant improvement for a mix of jobs. With a normal workload including snoozes, cancellations, etc, we would expect this change to reduce overall contention due to fewer concurrent transactions and fewer conns active on a given database pool.

Before

Running tool: /opt/homebrew/bin/go test -benchmem -run=^$ -bench ^(BenchmarkAsyncCompleter_Concurrency10|BenchmarkAsyncCompleter_Concurrency100|BenchmarkBatchCompleter|BenchmarkInlineCompleter)$ github.com/riverqueue/river/internal/jobcompleter

goos: darwin
goarch: arm64
pkg: github.com/riverqueue/river/internal/jobcompleter
cpu: Apple M1 Max
BenchmarkBatchCompleter/Completion-10                       	   79447	     16670 ns/op	    1535 B/op	      18 allocs/op
--- BENCH: BenchmarkBatchCompleter/Completion-10
    logger.go:257: time=2024-09-24T21:01:07.323-05:00 level=WARN msg="BatchCompleter: Hit maximum backlog; completions will wait until below threshold" max_backlog=20000
    logger.go:257: time=2024-09-24T21:01:07.756-05:00 level=WARN msg="BatchCompleter: Hit maximum backlog; completions will wait until below threshold" max_backlog=20000
    logger.go:257: time=2024-09-24T21:01:08.083-05:00 level=WARN msg="BatchCompleter: Hit maximum backlog; completions will wait until below threshold" max_backlog=20000
BenchmarkBatchCompleter/RotatingStates-10                   	   22052	     51476 ns/op	    3816 B/op	      54 allocs/op
PASS
ok  	github.com/riverqueue/river/internal/jobcompleter	19.210s

After

Running tool: /opt/homebrew/bin/go test -benchmem -run=^$ -bench ^(BenchmarkAsyncCompleter_Concurrency10|BenchmarkAsyncCompleter_Concurrency100|BenchmarkBatchCompleter|BenchmarkInlineCompleter)$ github.com/riverqueue/river/internal/jobcompleter

goos: darwin
goarch: arm64
pkg: github.com/riverqueue/river/internal/jobcompleter
cpu: Apple M1 Max
BenchmarkBatchCompleter/Completion-10                       	   58850	     19416 ns/op	    2462 B/op	      26 allocs/op
--- BENCH: BenchmarkBatchCompleter/Completion-10
    logger.go:257: time=2024-09-24T21:07:05.374-05:00 level=WARN msg="BatchCompleter: Hit maximum backlog; completions will wait until below threshold" max_backlog=20000
    logger.go:257: time=2024-09-24T21:07:05.816-05:00 level=WARN msg="BatchCompleter: Hit maximum backlog; completions will wait until below threshold" max_backlog=20000
BenchmarkBatchCompleter/RotatingStates-10                   	   70927	     19355 ns/op	    2625 B/op	      30 allocs/op
--- BENCH: BenchmarkBatchCompleter/RotatingStates-10
    logger.go:257: time=2024-09-24T21:07:08.036-05:00 level=WARN msg="BatchCompleter: Hit maximum backlog; completions will wait until below threshold" max_backlog=20000
    logger.go:257: time=2024-09-24T21:07:08.505-05:00 level=WARN msg="BatchCompleter: Hit maximum backlog; completions will wait until below threshold" max_backlog=20000
PASS
ok  	github.com/riverqueue/river/internal/jobcompleter	18.182s

@bgentry bgentry force-pushed the bg-job-set-state-if-running-many branch 2 times, most recently from 528d1b0 to 8ba3a61 Compare September 25, 2024 02:00
@bgentry bgentry marked this pull request as ready for review September 25, 2024 02:12
This adds a `JobSetStateIfRunningMany` query and corresponding driver
API, with implementations for both pgxv5 and `database/sql`.

The `BatchCompleter` was updated to use this new query and to batch
_all_ operations, not only those moving to a `complete` state. This
means the `AsyncCompleter` (as well as the `InlineCompleter`) are both
now unused and could be deleted, along with their underlying queries.

The intention of this is not just to facilitate improved performance
even on snoozes, retries, errors, cancellations, etc., but also to get
down to a single path for completions (similar to now having a single
path for insertions).
@bgentry bgentry force-pushed the bg-job-set-state-if-running-many branch from 8ba3a61 to 2eb03e2 Compare September 25, 2024 02:20
@bgentry
Copy link
Contributor Author

bgentry commented Sep 25, 2024

@brandur alright, this is ready to go, please take a look if you can. Also let me know if you think we should nuke the inline or async completers altogether at this point (along with their DB queries) or if you think we should keep them around for now.

@bgentry
Copy link
Contributor Author

bgentry commented Sep 25, 2024

The InlineCompleter is still used from some tests. Removing just the AsyncCompleter won't save many LoC because the underlying query is shared with InlineCompleter anyway, so I think it makes sense to just leave those around for now.

Going to move forward on shipping the rest of this to unblock upcoming stuff.

@bgentry bgentry merged commit bd43738 into master Sep 25, 2024
14 checks passed
@bgentry bgentry deleted the bg-job-set-state-if-running-many branch September 25, 2024 20:41
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.

1 participant