Topics

Drizzle Team•13mo ago

shikishikichangchang

Transaction doesn't support Promise.all

This works:

 await db.transaction(async (tx) => {
        await tx.insert...
        await tx.insert...
      });

 await db.transaction(async (tx) => {
        await tx.insert...
        await tx.insert...
      });

This does not:

 await db.transaction(async (tx) => {
        const p0 = tx.insert...
        const p1 = tx.insert...
        await Promise.all([
          p0,
          p1,
        ]);
      });

 await db.transaction(async (tx) => {
        const p0 = tx.insert...
        const p1 = tx.insert...
        await Promise.all([
          p0,
          p1,
        ]);
      });

45 Replies

Angelelz•13mo ago

There is no benefit in doing it in a promise.all. You can't insert values at the same time, that's the reason you have you use a transaction, because database operations are sequencial

shikishikichangchangOP•13mo ago

The two statements are for two tables. Is there a reason why they can't be executd in parallel? Yes they're part of one transaction, but I'd imageine the statemnets get executed in parallel, and if no errors occured, the transaction is completed. Else, rollback.

Angelelz•12mo ago

Have you tried this?

 await db.transaction(async (tx) => {
        const p0 = tx.insert...
        const p1 = tx.insert...
        await Promise.all([
          await p0,
          await p1,
        ]);
      });

 await db.transaction(async (tx) => {
        const p0 = tx.insert...
        const p1 = tx.insert...
        await Promise.all([
          await p0,
          await p1,
        ]);
      });

Now, the limitation is in the database itself. A transaction is designed so that the operations performed are sequential, because if you're using a transaction each operation is supposed to depend on the operation before it If you don't have that requirement, then you don't need a transaction In your case, it looks like you'd want to do it in a transaction anyway, just in case one of the inserts fails, the other will fail as well

predragnikolic•12mo ago

Not necessarily Drizzle related, but rather how JavaScript Promise.all behaves, for more info see this thread(from TypeOrm) https://github.com/typeorm/typeorm/issues/1014#issuecomment-336397434

GitHub

Transaction doesn't rollback · Issue #1014 · typeorm/typeorm

Version: 0.0.11 The "Database entity is not set for the given subject" error does not rollback transactions // this will remove row with id 1 but not id 2 repository.transaction(async rep...

sakura•12mo ago

Having this same issue, are you using mysql/planetscale? This worked fine for me with postgres

shikishikichangchangOP•12mo ago

I don't see how that's helpful/different from

 await db.transaction(async (tx) => {
        await tx.insert...
        await tx.insert...
      });

 await db.transaction(async (tx) => {
        await tx.insert...
        await tx.insert...
      });

?

Angelelz•12mo ago

Because all the query builder implement a lazy promise

Luxaritas•12mo ago

The promise.all is doing nothing in your case though Angelelz What you'd be passing are the resolved promise values

shikishikichangchangOP•12mo ago

"because if you're using a transaction each operation is supposed to depend on the operation before it" This is simply not true. Imagine you have a user account table and a profile info table and when ther user deletes their account you wanna delete both. The operations are independent but you definitely want them both in the same transaction Likle Luxaritas said, the promise.all doesn't do anything in your code

Angelelz•12mo ago

I don't know how the lazy promises would work in a promise.all then But my suspicion is that's probably why it's not working how you want it That's the usual purpose of using a transaction anyway

shikishikichangchangOP•12mo ago

Transactions are used to represent a single logical unit of work, not for dependent operations although the latter usually represents itself as the former say if I transfer money from account A to B. I need to increment B and decrement A. These two operations are not necessarily dependent (incrementing one number is independent of decrementing the other), but they need to happen "as a whole". Hence, the use of transactions

Angelelz•12mo ago

Yeah but can you parallelize them?

Luxaritas•12mo ago

No description

Luxaritas•12mo ago

Once you pass them to Promise.all, they will all evaluate, since Promise.all is implicitly calling the then method on the promises passed in, which is what Drizzle uses to trigger the actual evaluation I am in agreement with @shikishikichangchang here - it is completely reasonable to want to use a transaction as a means to commit if all succeed or fail if anything fails. What you're describing maybe relates more to transaction isolation level, and it's actually possible to relax the isolation level so that you don't have the same ordering garuntees Or actually maybe not realted to isolation level, as we're talking about operations within the transaction, not how it responds to operations outside of the transaction

Angelelz•12mo ago

I understand that and I partially agree, my point is I don't think using promise.all would achieve any performance benefit over just running the promises as at the DB level it won't be parallel anyway?

Luxaritas•12mo ago

That I don't know Looking at the mysql docs, they don't mention operations in a transaction being serialized, but they dont say the arent either ooooh wait But statements within a connection area serial And transactions run within a single connection, right? So that means they must be serial

shikishikichangchangOP•12mo ago

I understand what you mean now. You’re saying it can’t be parallelised on the database level due to SQL being ACID and all So there’s no performance benefits to promiseAll when wrapped in a transaction since they have to be executed serially anyway Is that what you mean and if so this is by design from DrizzleORM? I’d imagine an error or warning would be thrown if it was by design

Angelelz•12mo ago

This is what I mean, but I'm not sure if it's by design

shikishikichangchangOP•12mo ago

I see, are you on the DrizzleORM team? Prisma allows transactions promise.all so I’m wondering what the tradeoffs are here

Luxaritas•12mo ago

How does Drizzle behave when you pass transaction operations to Promise.all? I don't think you ever specified earlier in this thread

shikishikichangchangOP•12mo ago

It doesn’t do anything Meaning the queries don’t get executed

Luxaritas•12mo ago

Oh, thats odd. Didnt expect that

shikishikichangchangOP•12mo ago

Yea if the queries are executed, there wouldn’t be this thread 😂

Luxaritas•12mo ago

Thought it mightve broken some other way ¯\_(ツ)_/¯

Angelelz•12mo ago

I'm not part of the drizzle team just a collaborator and very familiar with the codebase

shikishikichangchangOP•12mo ago

How I thought it’d work is those queries get executed in parallel, and then if an error occurs, all is rolled back. If not, return success

Angelelz•12mo ago

This is why I thought awaiting would help

Luxaritas•12mo ago

I'm guessing the behavior is not intended But as we've established, the DBs themselves can only run one statement in a transaction at a time So its technically not buying you anything

shikishikichangchangOP•12mo ago

Yes but if I’m awaiting like that, promise.all is unnecessary So how does Prisma do it then? Different isolation levels of the two ORMs?

Luxaritas•12mo ago

Just because they let you do it doesnt mean its actually running in parallel You queue up the statements, but they still get run one at a time

Angelelz•12mo ago

You can control the isolation level with drizzle too

Luxaritas•12mo ago

Isolation level only effects how statements outside the transaction affect statements inside the transaction

shikishikichangchangOP•12mo ago

True

Luxaritas•12mo ago

Its irrelevant to the behavior of multiple statements running inside a transaction

shikishikichangchangOP•12mo ago

Yeap but not a DB expert so I don’t really wanna mess things up configuring that I see

Angelelz•12mo ago

I'm on vacation but I'm curious about this... I'll do some tests when I go back, hopefully I don't forget

shikishikichangchangOP•12mo ago

sounds good thank you!

Angelelz•12mo ago

I just tested this and I cannot reproduce your issue This is the test code:

import { db } from "./mysql/mysql2";
import { users } from "./mysql/schema";

await db.transaction(async (tx) => {
  const query1 = tx.insert(users).values({ name: "new" });
  const query2 = tx.insert(users).values({ name: "new2" });

  const res1 = await Promise.all([query1, query2]);

  console.log(res1);
});

process.exit(0);

import { db } from "./mysql/mysql2";
import { users } from "./mysql/schema";

await db.transaction(async (tx) => {
  const query1 = tx.insert(users).values({ name: "new" });
  const query2 = tx.insert(users).values({ name: "new2" });

  const res1 = await Promise.all([query1, query2]);

  console.log(res1);
});

process.exit(0);

This is my stdout:

> bun run ./src/mysql.ts
Query: begin
Query: insert into `users` (`id`, `name`, `manager_id`, `created_at`) values (default, ?, ?, default) -- params: ["new", 1]
Query: insert into `users` (`id`, `name`, `manager_id`, `created_at`) values (default, ?, ?, default) -- params: ["new2", 1]
[
  [
    {
      fieldCount: 0,
      affectedRows: 1,
      insertId: 12,
      info: "",
      serverStatus: 3,
      warningStatus: 0,
      changedRows: 0
    }, undefined
  ], [
    {
      fieldCount: 0,
      affectedRows: 1,
      insertId: 13,
      info: "",
      serverStatus: 3,
      warningStatus: 0,
      changedRows: 0
    }, undefined
  ]
]
Query: commit

> bun run ./src/mysql.ts
Query: begin
Query: insert into `users` (`id`, `name`, `manager_id`, `created_at`) values (default, ?, ?, default) -- params: ["new", 1]
Query: insert into `users` (`id`, `name`, `manager_id`, `created_at`) values (default, ?, ?, default) -- params: ["new2", 1]
[
  [
    {
      fieldCount: 0,
      affectedRows: 1,
      insertId: 12,
      info: "",
      serverStatus: 3,
      warningStatus: 0,
      changedRows: 0
    }, undefined
  ], [
    {
      fieldCount: 0,
      affectedRows: 1,
      insertId: 13,
      info: "",
      serverStatus: 3,
      warningStatus: 0,
      changedRows: 0
    }, undefined
  ]
]
Query: commit

And I can see the new rows inserted in my test db If you still have the issue, the first thing I would try is calling execute() method on the queries you are passing to Promise.all() because that method directly calls .then() in the lazy promise If not, you could create a reproduction repo to investigate

shikishikichangchangOP•12mo ago

I’m using planetscale does that matter?

Angelelz•12mo ago

hmmm, probably

shikishikichangchangOP•12mo ago

can you try that query on a planetscale db?

Angelelz•12mo ago

I could, what I'm thinking is, if it doesn't work, then it would be a driver issue. As it works in Mysql2

shikishikichangchangOP•12mo ago

thanks!

christrading•5mo ago

How do we have the .run() or .execute() of our queries run in the Promise.all and not when we are defining the queries?

await Promise.all(insertStatements.map((query) => query.run()))

await Promise.all(insertStatements.map((query) => query.run()))

should it be .run() or .execute() to have these queries done in parallel?

ibluk•5d ago

I'm running into this exact issue right now using PlanetScale. Is there any news on this? I've tried it with execute() and different isolation levels. It doesn't seem to work for some reason. I have two updates, and one insert. The updates don't work, but the insert does, which is weird.

Drizzle Team Join

The official Discord for all Drizzle related projects, such as Drizzle ORM, Drizzle Kit, Drizzle Studio and more!

9.5KMembers

View on Discord

Want results from more Discord servers?

Add your server