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

ExecuteInTx PostgreSQL version #1045

Merged
merged 2 commits into from
Jul 11, 2023
Merged

ExecuteInTx PostgreSQL version #1045

merged 2 commits into from
Jul 11, 2023

Conversation

redbaron
Copy link
Contributor

@redbaron redbaron commented Jul 3, 2023

This PR attempts to improve ExecuteInTx in following ways:

  • Previously it required call-site to provide Tx, but imposed restriction that caller must open, but don't use Tx prior to calling ExecuteInTx. This change now enforces this contract by creating Tx internally, caller provides *sql.DB instead.

  • ExecuteInTx was optimized for CockroachDB. Every transaction in addition to standard BEGIN/COMMIT had SAVEPOINT and RELEASE SAVEPOINT commands as the very first and very last commands in transaction. These are unnecessary in PG and add 2 additional round-trips to the database. For small and quick inserts additional round-trips are comparable to the query itself. There is now separate version of ExecuteInTx for Cockroach and regular PosgreSQL. CockroachDB version is enabled at connection time based on result from version() SQL function.

  • ExecuteInTx used to ignore COMMIT operation result. It wasn't a problem with CockroachDB, because commit actually happened on RELEASE SAVEPOINT and COMMIT couldn't fail. On PostgreSQL COMMIT can fail and ignoring possible errors is undesirable.

Previously ExecuteInTx accepted open transaction, but required
users never to execute any commands on it prior to calling
ExecuteInTx. This API change enforces this contract by making
ExecuteInTx to open transaction internally and pass it to the
callback func.
server/db.go Show resolved Hide resolved
server/db.go Outdated Show resolved Hide resolved
…e Server

PostgreSQL doesn't benefit from SAVEPOINT/ROLLBACK logic like CockroachDB
does. With this change Nakama checks server DB engine and enables CockroachDB
optimization only when necessary.

There are 2 behviour change in the PG version of ExecuteInTx:

- it retries on all "Class 40" (a.k.a retriable) codes, not just
  serialization error:

	40000 	transaction_rollback
	40002 	transaction_integrity_constraint_violation
	40001 	serialization_failure
	40003 	statement_completion_unknown
	40P01 	deadlock_detected

- It doesn't ignore COMMIT result code anymore
@redbaron redbaron requested a review from syhpoon July 11, 2023 12:58
Copy link
Contributor

@syhpoon syhpoon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

server/db.go Show resolved Hide resolved
@redbaron redbaron merged commit 21e3cbd into master Jul 11, 2023
2 checks passed
@redbaron redbaron deleted the execute-in-tx-pg-skip branch July 11, 2023 15:24
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.

None yet

4 participants