Broken .with_transaction_options and .with_retry_options on AsyncIOPool

[unmade]

I'm on the latest edgedb-python (0.14.0) with EdgeDB 1-beta2 running in Docker

I've tried to call with_retry_options method on AsyncIOPool instance, but it fails with:

        ...
        result = self._shallow_clone()
>      result._options = self._options.with_retry_options(options)
E      AttributeError: 'NoneType' object has no attribute '_options'

I've also noticed that AsyncIOPool doesn't implement _shallow_clone required by _OptionsMixin.

Also, is there is any specific reason why _OptionsMixin doesn't inherit from abc.ABC but still uses abstractmethod decorator?

[unmade]

any updates on the issue?

[1st1]

cc @tailhook

Not yet; we'll take a look soon.

[unmade]

Hey guys! This bug becomes very annoying, since there is no way to change transaction isolation level on the pool. This is a real bummer, since in the latest EdgeDB SERIALIZABLE is now a default isolation level.

Here is how you can reproduce the bug:

import edgedb
from edgedb import TransactionOptions, IsolationLevel


async with edgedb.create_async_pool(...) as pool:
    mpool = pool.with_transaction_options(TransactionOptions(isolation=IsolationLevel.RepeatableRead))

Please, can you at least help me to figure out how can I change the transaction isolation level for every pool.query(...) call?

---

I'm on the latest edgedb and edgedb-python (both built from source from master).

[unmade]

Oh I see. Because of geldata/gel#2877, isolation level set by .with_transaction_options() is always ignored

[tailhook]

Sorry for long delay. Somehow this slipped from my attention. Fixed setting options in #237.

But yes, we are reconsidering transaction isolation. What's your use case of REPEATABLE READ? Do you need it for performance reasons? Or have you just tested different isolation levels?

[unmade]

I actually do need it for performance reasons. I have a tree-like model with materialized path:

    type File {
        required property path -> str;
        single link parent -> File {
            on target delete DELETE SOURCE;
        };
    }

And I need to restore all missing parents on a large number of records. Now, I'm doing something like this:

query = """
    UPDATE
        File
    FILTER
        .path LIKE <str>$parent_path ++ '/%'
        AND
        .path NOT LIKE <str>$parent_path ++ '/%/%'
    SET {
        parent := (
            SELECT
                File
            FILTER
                .path = <str>$parent_path
            LIMIT 1
        )
    }
"""

coros = (
    pool.query(query, parent_path=parent_path)
    for parent_path in parents
)
await asyncio.gather(*coros)

With SERIALIZABLE level I get:

could not serialize access due to read/write dependencies among transactions

[tailhook]

Have you tried wrap pool.query() into a pool.retrying_transaction ? I wonder how much retry attempts are there (i.e. if only few of them fail, it may be good enough to just rely on transaction retrying mechanism to make this work).

[unmade]

Yes, I've tried wrapping it in retrying_transaction (I've also increased number of retries). Still got the same error with сould not serialize access.

I don't think retrying_transaction is a good option for my case, since parent can have lots of children and each child can also have lots of children and so on. So there definitely will be conflicts on SERIALIZABLE level and retrying_transaction can be even slower than updating everything in sequential order

[msullivan]

What did your retrying_transaction solution look like?

[msullivan]

(And it worked when we were using repeatable read?)

[unmade]

I'm sorry, I oversimplified my example a little bit. In real life I'm using FOR...IN to bulk update and it seems like this causes my issues with concurrent updates.

What did your retrying_transaction solution look like?

I prepared a gist with my example: https://gist.github.com/unmade/135cb59092202750b953b3285cc6851d

(And it worked when we were using repeatable read?)

Yes, the example from gist works with no problem when isolation level is REPEATABLE READ, however it fails with SERIALIZABLE