TransactionSerializationError in UPDATE concurrent queries

[MrFoxPro]

• EdgeDB Version: 4.2, 4.6, 4.7, 5.3
• EdgeDB CLI Version: 5.1.0+7c5764f
• OS Version: GNU/Linux 6.8.9 x64_86

Steps to Reproduce:

module default {
  type A {
    required value: int64 { default := 0; }
  }
}

insert A {} 

Then, open two bash instances and run queries:

for i in {1..100}; do edgedb query "select(update A set { value := .value + 1 }).value"; done

Observe errors:

errors.mp4

After end of loops, A.value will not reach 200.
I got this also in JS and Rust client.

Compared to PostgresQL:

psql.mp4

[MrFoxPro]

I workaround this by using RwLock on client, so I can have select/insert/delete concurrent queries and sequential update queries. But I guess EdgeDB should handle it itself, as underlying postgres does?

[msullivan]

I think part of the reason for the difference with postgres is that we are using postgres's SERIALIZABLE isolation level, which is stricter than the default of READ COMMITTED.

When I try it with postgres natively using SERIALIZABLE or REPEATABLE READ, I get the same results.

Another part of this is that the CLI doesn't retry on fails, though I think all of our language clients do.

[MrFoxPro]

Another part of this is that the CLI doesn't retry on fails, though I think all of our language clients do.

My observation is completely opposite. I was getting this error on NodeJS separate workers and in multi-threaded Rust app too. I believe you're confusing it with TransactionConflictError https://github.com/edgedb/edgedb-rust/blob/093729876737738a559e35c2ec3f0ae27efd2f69/edgedb-tokio/src/options.rs#L26

Anyway, do you think retrying is correct way to workaround this? First time I tried to implement retries, and then I compared to RwLock implementation. I observed better results with manual locking on UPDATE operations. I guess receiving error and retrying multiple times is more expensive than synchronous locking.

[msullivan]

Hmmmm, I had thought that TransactionSerializationError was marked as SHOULD_RETRY also, but it is not: https://github.com/edgedb/edgedb/blob/b9b4177d8c0080235e46b7e825384224fbe5923c/edb/api/errors.txt#L128C18-L128C47
Maybe we need to change that

Doing synchronous locking should work great if you only have one client, but it doesn't help you in a distributed situation.

[elprans]

SHOULD_RETRY should be inherited, and it's set on the parent TransactionConflictError. The bindings might not be reflecting that faithfully, though.

[msullivan]

Oh, good point. It looks like the rust bindings do reflect that faithfully: https://github.com/edgedb/edgedb-rust/blob/093729876737738a559e35c2ec3f0ae27efd2f69/edgedb-errors/src/kinds.rs#L139.

This sort of counter workload is not a great fit for SERIALIZED/REPEATABLE READ, perhaps.

[1st1]

Oh, good point. It looks like the rust bindings do reflect that faithfully:

@msullivan Can you double check js/py/go bindings?

[MrFoxPro]

Is it possible to configure EdgeDB to work with postgres in READ COMMITTED mode? What are benefits/drawbacks, since I'm not planning to distrubute my db across servers so far

[MrFoxPro]

I changed global and database default_transaction_isolation to 'read committed' but TransactionSerializationError still happens

[msullivan]

EdgeDB sets the mode in the connection settings when it connects to postgres, I think.
Changing it to read committed there does fix this problem, though I don't think there is a way to do it as a user.
Which is fine, because a bunch of weird stuff can happen in that mode in edgedb. (Exclusive constraints not enforced properly, for one)