yugabyte / yugabyte-db

YugabyteDB - the cloud native distributed SQL database for mission-critical applications.
https://www.yugabyte.com
Other
9.04k stars 1.08k forks source link

[YSQL] CLOSE ALL cursor not supported yet #3639

Open mvanderkroon opened 4 years ago

mvanderkroon commented 4 years ago

Jira Link: DB-2112 I'm trying to use asyncpg in combination with yugabytedb. I use a connection pool and issuing queries results in the following message:

asyncpg.exceptions.FeatureNotSupportedError: CLOSE ALL cursor not supported yet
HINT:  Please report the issue on https://github.com/YugaByte/yugabyte-db/issues

If I use individual connections, it works fine (-ish, there is an incompatibility in how asyncpg handles PG version detection but that issue belongs to asyncpg).

Version info:

version 2.0.11.0 build 23 revision 2d52e1dfef65294a0ddf7c6377e3da2b8ed5a6b0
ddorian commented 4 years ago

@mvanderkroon can you specify what you do when the error happens ? pool.release(conn) ?

mvanderkroon commented 4 years ago

@ddorian, sure thing!

It occurs on various statements, including TRUNCATE, INSERT and SELECT. See below example of how I issue these statements.

async with pool.acquire() as con:
        values = await con.fetch(
            f"SELECT * FROM mytab LIMIT 5;"
        )
ddorian commented 4 years ago

We don't yet support CLOSE ALL. And LISTEN/NOTIFY

The full query from here: https://github.com/MagicStack/asyncpg/blob/master/asyncpg/connection.py#L1114

SELECT pg_advisory_unlock_all();
CLOSE ALL;

                DO $$
                BEGIN
                    PERFORM * FROM pg_listening_channels() LIMIT 1;
                    IF FOUND THEN
                        UNLISTEN *;
                    END IF;
                END;
                $$;

RESET ALL;

Let me check more for a workaround

mvanderkroon commented 4 years ago

@ddorian for the time being I have monkeypatched the piece of code that you mention. This allows me to continue testen - although I'm afraid it will cause problems further down the line (unreleased cursors clogging up resources). If you have a better workaround I'd be very thankful!

ddorian commented 4 years ago

What about using aiopg which is built on top of psycopg2 ? That is the most used postgresql driver in python.

mvanderkroon commented 4 years ago

I'll give that a try, thanks!

ddorian commented 4 years ago

Looking further, asyncpg is also using advisory locks https://github.com/MagicStack/asyncpg/blob/master/asyncpg/connection.py#L1278. Tracked here https://github.com/yugabyte/yugabyte-db/issues/3642

p-null commented 4 years ago

Hi @mvanderkroon , I wonder how did you get asyncpg to work with yb? See the issue here #5154

ddorian commented 4 years ago

@tingkai-zhang you can maybe monkey-patch it ?

p-null commented 4 years ago

Hi @ddorian Thanks. After I tried patching, I got:

  File "/home/tingkai/.local/share/virtualenvs/app-WO43lLll/lib/python3.7/site-packages/asyncpg/pool.py", line 654, in release
    return await asyncio.shield(ch.release(timeout))
  File "/home/tingkai/.local/share/virtualenvs/app-WO43lLll/lib/python3.7/site-packages/asyncpg/pool.py", line 216, in release
    raise ex
  File "/home/tingkai/.local/share/virtualenvs/app-WO43lLll/lib/python3.7/site-packages/asyncpg/pool.py", line 206, in release
    await self._con.reset(timeout=budget)
  File "/home/tingkai/.local/share/virtualenvs/app-WO43lLll/lib/python3.7/site-packages/asyncpg/connection.py", line 1114, in reset
    await self.execute(reset_query, timeout=timeout)
  File "/home/tingkai/.local/share/virtualenvs/app-WO43lLll/lib/python3.7/site-packages/asyncpg/connection.py", line 272, in execute
    return await self._protocol.query(query, timeout)
  File "asyncpg/protocol/protocol.pyx", line 316, in query
asyncpg.exceptions.FeatureNotSupportedError: CLOSE ALL cursor not supported yet
HINT:  Please report the issue on https://github.com/YugaByte/yugabyte-db/issues

Process finished with exit code 1

Now I am here to report the same issue.

I know aiopg is an option but not ideal. There has been no new commits for months and the interface is not quite the same as asyncpg. And asyncpg is reported much faster.

ddorian commented 4 years ago

While asyncpg is much faster, it's because the features it requires which also have negatives.

The negative is that it's hard to do connection pooling. In pgbouncer it can only be "session pooling". Each connection can only run 1 query at the same time (I don't see them implementing BATCH mode). And each connection in PostgreSQL/YSQL is 1 forked process with memory overhead ~4MB & cpu overhead because of context switching between threads/processes.

How many concurrent queries do you expect to have in 1 python process and how many processes per client-node ? Example: in 1 app that I've developed I had (24cores * ~20 concurrent queries-per-process) in 1 client node.

aiopg is based on psycopg2 which is based on libpq. The last 2 are actively maintained.

Gugu7264 commented 3 years ago

@ddorian Hey there! I'm currently experiencing the same issue. Could you explain me what would I need to do so this can work, any workaround? I would prefer using asyncpg if possible, not to switch to aiopg.

ddorian commented 3 years ago

@Gugu7264 we have support for DECLARE CURSOR in master but it still requires listen/notify,advisory locks. Can you try disabling connection pooling ?

altanozlu commented 3 years ago

Any news ?

ddorian commented 3 years ago

@altanozlu regarding CLOSE ALL or asyncpg support ?

altanozlu commented 3 years ago

@altanozlu regarding CLOSE ALL or asyncpg support ?

asyncpg support

ddorian commented 3 years ago

@altanozlu can you try manually changing https://github.com/MagicStack/asyncpg/blob/232adcff95becd7370b0f637e54335e4d4d8642f/asyncpg/connection.py#L88 to disable support for listen_notify,close_all,advisory_locks and see if that works ? It's what it does for Redshift,cratedb,cockroach.