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
Improve Multi perf on SQL state stores with 1 op only #3300
Conversation
When Multi is invoked with a single operation, on SQL state stores (Postgres v1/v2, CockroachDB, MySQL, SQLite, MS SQL Server) we can omit starting a transaction, as a single operation is going to be executed atomically regardless. This allows reducing the number of round-trips to the DB from 3 to 1, reduces locks used inside the DB server, and improves overall performance. Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
Signed-off-by: Alessandro (Ale) Segala <43508+ItalyPaleAle@users.noreply.github.com>
Signed-off-by: Alessandro (Ale) Segala <43508+ItalyPaleAle@users.noreply.github.com>
case 1: | ||
return m.execMultiOperation(ctx, request.Operations[0], m.db) | ||
default: | ||
tx, err := m.db.Begin() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably use BeginTx
here like we do in sqlite.
tx, err := m.db.Begin() | |
tx, err := m.db.BeginTx(ctx, nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly a good idea, but I'd like to not make this change in this PR and maybe do a follow-up PR
Begin
and BeginTx
have some difference in behavior, specifically (unlike in the pgx driver for Postgres) the context controls the lifecycle of the entire transaction. See: https://pkg.go.dev/database/sql#DB.BeginTx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Love that we care about saving unnecessary roundtrips.
When Multi is invoked with a single operation, on SQL state stores (Postgres v1/v2, CockroachDB, MySQL, SQLite, MS SQL Server) we can omit starting a transaction, as a single operation is going to be executed atomically regardless.
This should offer some non-insignificant perf benefits thanks to: