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.

Sign up for GitHub

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

Jump to bottom

feat: add support for nested transaction rollbacks via savepoints in sql #4637

Closed
wants to merge 2 commits into from
Closed

feat: add support for nested transaction rollbacks via savepoints in sql #4637

wants to merge 2 commits into from

Conversation

janpio
Copy link
Contributor

@janpio janpio commented Jan 10, 2024

Integration for #4375

@LucianBuzzo
feat: add support for nested transaction rollbacks via savepoints in sql
29c424a 
This is my first OSS contribution for a Rust project, so I'm sure I've
made some stupid mistakes, but I think it should mostly work :)

This change adds a mutable depth counter, that can track how many levels
deep a transaction is, and uses savepoints to implement correct rollback
behaviour. Previously, once a nested transaction was complete, it would
be saved with `COMMIT`, meaning that even if the outer transaction was
rolled back, the operations in the inner transaction would persist. With
this change, if the outer transaction gets rolled back, then all inner
transactions will also be rolled back.

Different flavours of SQL servers have different syntax for handling
savepoints, so I've had to add new methods to the `Queryable` trait for
getting the commit and rollback statements. These are both parameterized
by the current depth.

I've additionally had to modify the `begin_statement` method to accept a depth
parameter, as it will need to conditionally create a savepoint.

When opening a transaction via the transaction server, you can now pass
the prior transaction ID to re-use the existing transaction,
incrementing the depth.

Signed-off-by: Lucian Buzzo <[email protected]>
@janpio janpio mentioned this pull request Jan 10, 2024
Open
@github-actions GitHub Actions
Copy link
Contributor

WASM Size

Engine This PR Base branch Diff
WASM 2.756MiB 2.744MiB 12.821KiB
WASM (gzip) 1.010MiB 1.006MiB 3.844KiB

@codspeed-hq CodSpeed HQ
Copy link

codspeed-hq bot commented Jan 10, 2024 •
edited
Loading

CodSpeed Performance Report

Merging #4637 will not alter performance

Comparing integration/sql-nested-transactions-3 (cc1af55) with main (0a83d85)

Summary

✅ 11 untouched benchmarks

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 10, 2024 •
edited
Loading

🚨 WASM query-engine: 1 benchmark(s) have regressed at least 2%

Full benchmark report 
DATABASE_URL="postgresql://postgres:postgres@localhost:5432/bench?schema=imdb_bench&sslmode=disable" \
node --experimental-wasm-modules query-engine/driver-adapters/executor/dist/bench.mjs
cpu: AMD EPYC 7763 64-Core Processor
runtime: node v18.19.0 (x64-linux)

benchmark                   time (avg)             (min … max)       p75       p99      p995
-------------------------------------------------------------- -----------------------------
• movies.findMany() (all - 25000)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline  313.76 ms/iter (307.93 ms … 330.59 ms) 314.48 ms 330.59 ms 330.59 ms
Web Assembly: Latest    313.89 ms/iter (308.18 ms … 329.58 ms)  314.4 ms 329.58 ms 329.58 ms
Web Assembly: Current    308.9 ms/iter (305.95 ms … 316.14 ms) 310.01 ms 316.14 ms 316.14 ms
Node API: Current       232.42 ms/iter    (223.36 ms … 244 ms) 233.81 ms    244 ms    244 ms

summary for movies.findMany() (all - 25000)
  Web Assembly: Current
   1.33x slower than Node API: Current
   1.02x faster than Web Assembly: Baseline
   1.02x faster than Web Assembly: Latest

• movies.findMany({ take: 2000 })
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   12.69 ms/iter      (12 ms … 17.34 ms)  12.64 ms  17.34 ms  17.34 ms
Web Assembly: Latest     13.49 ms/iter   (12.23 ms … 22.67 ms)  13.07 ms  22.67 ms  22.67 ms
Web Assembly: Current    12.75 ms/iter   (11.95 ms … 21.73 ms)  12.51 ms  21.73 ms  21.73 ms
Node API: Current         9.59 ms/iter    (9.25 ms … 15.11 ms)   9.61 ms  15.11 ms  15.11 ms

summary for movies.findMany({ take: 2000 })
  Web Assembly: Current
   1.33x slower than Node API: Current
   1x faster than Web Assembly: Baseline
   1.06x faster than Web Assembly: Latest

• movies.findMany({ where: {...}, take: 2000 })
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline    2.07 ms/iter     (1.84 ms … 3.72 ms)   2.01 ms   3.51 ms   3.53 ms
Web Assembly: Latest      2.09 ms/iter      (1.85 ms … 3.4 ms)   2.07 ms   3.36 ms   3.36 ms
Web Assembly: Current     2.12 ms/iter     (1.84 ms … 3.57 ms)    2.1 ms   3.48 ms    3.5 ms
Node API: Current          1.6 ms/iter     (1.48 ms … 2.39 ms)    1.6 ms   2.07 ms   2.22 ms

summary for movies.findMany({ where: {...}, take: 2000 })
  Web Assembly: Current
   1.32x slower than Node API: Current
   1.02x slower than Web Assembly: Baseline
   1.01x slower than Web Assembly: Latest

• movies.findMany({ include: { cast: true } take: 2000 }) (m2m)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   12.37 ms/iter   (11.87 ms … 14.46 ms)  12.44 ms  14.46 ms  14.46 ms
Web Assembly: Latest     12.46 ms/iter    (12.1 ms … 12.98 ms)  12.61 ms  12.98 ms  12.98 ms
Web Assembly: Current    12.45 ms/iter      (12 ms … 13.95 ms)  12.55 ms  13.95 ms  13.95 ms
Node API: Current         9.44 ms/iter    (9.04 ms … 10.06 ms)   9.53 ms  10.06 ms  10.06 ms

summary for movies.findMany({ include: { cast: true } take: 2000 }) (m2m)
  Web Assembly: Current
   1.32x slower than Node API: Current
   1.01x slower than Web Assembly: Baseline
   1x faster than Web Assembly: Latest

• movies.findMany({ where: {...}, include: { cast: true } take: 2000 }) (m2m)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline    1.96 ms/iter     (1.84 ms … 5.07 ms)   1.92 ms   2.92 ms   3.17 ms
Web Assembly: Latest      1.95 ms/iter     (1.83 ms … 3.17 ms)   1.93 ms   2.97 ms   2.99 ms
Web Assembly: Current     1.92 ms/iter     (1.82 ms … 5.41 ms)    1.9 ms   2.89 ms    2.9 ms
Node API: Current         1.63 ms/iter     (1.51 ms … 1.98 ms)   1.64 ms   1.93 ms   1.95 ms

summary for movies.findMany({ where: {...}, include: { cast: true } take: 2000 }) (m2m)
  Web Assembly: Current
   1.17x slower than Node API: Current
   1.02x faster than Web Assembly: Latest
   1.02x faster than Web Assembly: Baseline

• movies.findMany({ take: 2000, include: { cast: { include: { person: true } } } })
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline   12.76 ms/iter    (12.18 ms … 15.9 ms)  12.86 ms   15.9 ms   15.9 ms
Web Assembly: Latest     12.57 ms/iter   (12.14 ms … 13.65 ms)   12.7 ms  13.65 ms  13.65 ms
Web Assembly: Current    12.41 ms/iter   (11.88 ms … 15.82 ms)  12.54 ms  15.82 ms  15.82 ms
Node API: Current         9.65 ms/iter    (9.33 ms … 10.23 ms)   9.77 ms  10.23 ms  10.23 ms

summary for movies.findMany({ take: 2000, include: { cast: { include: { person: true } } } })
  Web Assembly: Current
   1.29x slower than Node API: Current
   1.01x faster than Web Assembly: Latest
   1.03x faster than Web Assembly: Baseline

• movie.findMany({ where: { ... }, take: 2000, include: { cast: { include: { person: true } } } })
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline    1.94 ms/iter     (1.84 ms … 2.84 ms)   1.94 ms   2.48 ms   2.61 ms
Web Assembly: Latest       1.9 ms/iter     (1.81 ms … 3.01 ms)   1.89 ms    2.4 ms   2.41 ms
Web Assembly: Current     1.89 ms/iter     (1.81 ms … 2.79 ms)   1.88 ms   2.61 ms   2.75 ms
Node API: Current         1.62 ms/iter     (1.51 ms … 1.97 ms)   1.62 ms   1.86 ms   1.87 ms

summary for movie.findMany({ where: { ... }, take: 2000, include: { cast: { include: { person: true } } } })
  Web Assembly: Current
   1.17x slower than Node API: Current
   1.01x faster than Web Assembly: Latest
   1.03x faster than Web Assembly: Baseline

• movie.findMany({ where: { reviews: { author: { ... } }, take: 100 }) (to-many -> to-one)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline  965.64 µs/iter    (861.4 µs … 1.65 ms) 939.85 µs   1.54 ms   1.57 ms
Web Assembly: Latest    944.86 µs/iter   (854.59 µs … 1.99 ms)  927.5 µs   1.53 ms   1.59 ms
Web Assembly: Current   921.48 µs/iter   (853.68 µs … 1.48 ms) 915.98 µs   1.42 ms   1.47 ms
Node API: Current       867.78 µs/iter   (790.95 µs … 1.06 ms) 880.53 µs   1.04 ms   1.05 ms

summary for movie.findMany({ where: { reviews: { author: { ... } }, take: 100 }) (to-many -> to-one)
  Web Assembly: Current
   1.06x slower than Node API: Current
   1.03x faster than Web Assembly: Latest
   1.05x faster than Web Assembly: Baseline

• movie.findMany({ where: { cast: { person: { ... } }, take: 100 }) (m2m -> to-one)
-------------------------------------------------------------- -----------------------------
Web Assembly: Baseline  907.68 µs/iter   (860.36 µs … 1.39 ms)  914.8 µs   1.23 ms   1.34 ms
Web Assembly: Latest    897.83 µs/iter   (855.41 µs … 1.43 ms) 900.96 µs   1.27 ms   1.31 ms
Web Assembly: Current   921.14 µs/iter   (867.87 µs … 1.42 ms) 919.45 µs   1.29 ms   1.39 ms
Node API: Current       865.51 µs/iter   (790.95 µs … 1.15 ms) 883.95 µs   1.04 ms   1.11 ms

summary for movie.findMany({ where: { cast: { person: { ... } }, take: 100 }) (m2m -> to-one)
  Web Assembly: Current
   1.06x slower than Node API: Current
   1.03x slower than Web Assembly: Latest
   1.01x slower than Web Assembly: Baseline

After changes in cc1af55

@janpio janpio closed this Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@janpio @LucianBuzzo