prisma-engines
prisma-engines copied to clipboard
fix: remove actor system for transactions
Closes https://github.com/prisma/team-orm/issues/1219
WASM Query Engine file Size
| Engine | This PR | Base branch | Diff |
|---|---|---|---|
| Postgres | 2.050MiB | 2.057MiB | -7.005KiB |
| Postgres (gzip) | 817.279KiB | 820.453KiB | -3.174KiB |
| Mysql | 2.019MiB | 2.027MiB | -8.780KiB |
| Mysql (gzip) | 804.657KiB | 808.035KiB | -3.378KiB |
| Sqlite | 1.915MiB | 1.919MiB | -3.804KiB |
| Sqlite (gzip) | 764.001KiB | 766.277KiB | -2.276KiB |
CodSpeed Performance Report
Merging #4940 will not alter performance
Comparing feat/remove-actors-attempt-2 (4146626) with main (c4fe130)
Summary
✅ 11 untouched benchmarks
I think I have found a bug after testing with smash. Since we are only dropping the connection from the transactions hashmap in the manager after the timeout has passed, the connection we took from the pool lingers for the timeout period, making future transactions wait for connections longer than necessary.
Update: This is fixed now.
Just so it won't get lost in Slack threads. Right now it breaks tracing tests in prisma/prisma link.
Looks like the reason for that is that itx_query_builder span got renamed into itx_execute_single.
New name is much better, but it's sort of a breaking change for tracing feature - span is user-facing. It's fine to do it, tracing is still in preview, we just need to be conscious and explicit about it.
cc: @millsp @aqrln see this comment from @SevInf: https://github.com/prisma/prisma-engines/pull/4940#issuecomment-2218199242
I just read "breaking change for Tracing feature" just to keep it in mind in case it's relevant.
I've found the reason why spans are missing and it's a doozy!
Reportedly missing spans are actually emitted by the query engine, but they lack the parentSpanId property. The only thing they have is a link to the operation that they are related to. For example, span with INSERT statement will have a link to the create() operation.
parentSpanId is missing because previously the tokio task running the transaction was responsible for setting it and it is no longer there. Moreover the sequence of spans related to the transactions was completely separate from the query operations. On the same level as create() and findMany() operations, we had an itx_runner span, which had all interactive transactions operations under it.
Now that the interactive transaction operations are executed in the same task as the create() and findMany() operations, we find ourselves in the world where itx_execute_single spans are under their respective operations, but the commit and rollback operations are still under itx_runner. There is also some logic to derive trace_id from transaction id, which might be obsolete now - I need to double-check it.
In any case, it looks like the actor system isn't going to give up without a fight until the very end!