Connection pool, understand idea

[DFilyushin]

I use an example of using a connection pool:

    connect_settings = ConnectSettings.default()
    pool = await asynch.create_pool(
        minsize=10, maxsize=30, host=connect_settings.host,
        port=int(connect_settings.port),
        user=connect_settings.user,
        password=connect_settings.password,
        database=connect_settings.database,
    )
    for item in range(100):
        async with pool.acquire() as conn:
            async with conn.cursor() as cursor:
                await cursor.execute("SELECT 1")
                ret = await cursor.fetchone()
                assert ret == (1,)
    pool.close()
    await pool.wait_closed()

I call the query SELECT * FROM system.metrics WHERE metric = 'TCPConnection' in Clickhouse, result: 100.

What's wrong?

[DFilyushin]

This approach actually exhausts the number of connections in the database.

[stankudrow]

@DFilyushin , greetings.

Thank you for reporting this issue, we faced it as well and had to roll back to the asynch v0.2.3 (the number of dangling connections disappeared or a kind of miracle happened anyhow).

Could you possibly have a look at this PR #109 that target the highlighted problems with connection management via Pool class instances? It also brings the async with pool context management support.

Here goes your example adapted to the PR #109 :

import asyncio

from asynch.connection import Connection
from asynch.pool import Pool
from asynch.proto import constants


async def get_tcp_connections(connection: Connection) -> int:
    stmt = "SELECT * FROM system.metrics WHERE metric = 'TCPConnection'"
    async with connection.cursor() as cursor:
        await cursor.execute(query=stmt)
        result = await cursor.fetchall()
        return int(result[0][1])


async def main():
    conn = Connection(
        user=constants.DEFAULT_USER,
        password=constants.DEFAULT_PASSWORD,
        host=constants.DEFAULT_HOST,
        port=constants.DEFAULT_PORT,
        database=constants.DEFAULT_DATABASE,
    )
    async with conn:
        init_tcps = await get_tcp_connections(connection=conn)

    pool = Pool(
        minsize=10,
        maxsize=30,
        user=constants.DEFAULT_USER,
        password=constants.DEFAULT_PASSWORD,
        host=constants.DEFAULT_HOST,
        port=constants.DEFAULT_PORT,
        database=constants.DEFAULT_DATABASE,
    )
    async with pool:
        for _ in range(100):
            async with pool.connection() as conn:
                async with conn.cursor() as cursor:
                    await cursor.execute("SELECT 42")
                    ret = await cursor.fetchone()
                    assert ret == (42,)

    async with conn:
        final_tcps =  await get_tcp_connections(connection=conn)

    assert init_tcps == final_tcps


if __name__ == "__main__":
    asyncio.run(main())

Could you possibly give your feedback on:

1. problems eliminated
2. your user experience (usability is of high importance)

[stankudrow]

@DFilyushin , please consider reviewing the PR #124 which is related to this discussion as well.

Also, could you close this discussion? The new API is in the v0.2.5, now it is being refined.

[stankudrow]

@DFilyushin , could you mark this as answered and close the topic?

Please have a look at the issue #121 which is solved and will be available in the next release (as soon as this issue #126 gets solved).

[stankudrow]

@DFilyushin