Nested transactions

[codyebberson]

Formalizes our use of database transactions across all database usage.

This is in service of proper FHIR Bundle transaction support, described here: #1369

Once this is shipped, implementing Bundle.type = 'transaction' is this:

Old conventions:

• The only concrete transactions were in the multi-table write operations
• Otherwise, grab a database connection from getClient() as needed

New conventions:

• By default, you should only ever use the server Repository class for database connections
• If you don't need a transaction, use repo.getClient() for database access
  • If in an existing transaction, this will reuse the same connection and be part of the transaction
  • If not, it falls back to the existing behavior and executes a one-off query
• If you need a transaction, wrap with repo.withTransaction((client) => { ... });
  • If already in a transaction, this updates a depth counter and reuses the connection
  • If not, it creates a new transaction
  • This adds the necessary wrappers for commit, rollback and release

Exceptions:

• We have a few cases of database cursors for processing all rows in a table
  • For example, "reindex all resources"
  • Full table cursors are incompatible with this model, and will lock the connection
  • So we still call getClient() (not repo.getClient()) in those cases
• Specialty operations
  • For example, Project/$expunge and CodeSystem/$import
  • These have a high degree of special logic
  • Perhaps someday they should be converted to this same model, but that can be separate work

Known issues:

• There is one serious known issue with Redis cache cleanup in transaction rollback
• As a team, we have discussed a number of possible solutions
  • For example, track resources to add to Redis, and only add on final COMMIT
  • Or, track all affected resource IDs, and flush those from the cache on ROLLBACK
  • Or, revisit our use of Redis cache entirely
• This issue does not affect any of our current uses of withTransaction(), so it does not effect any current runtime implementation
• However, it is a blocker before we can fully implement FHIR transaction bundles
• The next step is to run some experiments and performance tests on the various strategy

Also:

• There were a few unexpected warnings in the postgres logs for a clean run of server tests
• This PR cleans up all of them except for a "unique constraint" error, which is expected and correct

Before:

After:

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

3 Ignored Deployments

Name	Status	Preview	Comments	Updated (UTC)
medplum-app	⬜️ Ignored (Inspect)	Visit Preview		Jan 25, 2024 1:29am
medplum-storybook	⬜️ Ignored (Inspect)	Visit Preview		Jan 25, 2024 1:29am
medplum-www	⬜️ Ignored (Inspect)	Visit Preview		Jan 25, 2024 1:29am

[github-actions]

	Messages
📖	@medplum/core: 153.9 kB
📖	@medplum/react: 337.0 kB

Generated by 🚫 dangerJS against 6a46e40

[sonarcloud]

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
91.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud