Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DB connection leak #5580

Closed
yangxikun opened this issue Jan 8, 2025 · 10 comments
Closed

DB connection leak #5580

yangxikun opened this issue Jan 8, 2025 · 10 comments
Assignees
Labels
bug Something isn't working

Comments

@yangxikun
Copy link

Bug Description

DB connection pool overflow:
企业微信截图_2acb957e-0ede-41e3-a0cb-82ea911f03ae

Reproduction

  1. Create simple flow
Clipboard_Screenshot_1736305727
  1. curl call api: api/v1/build/47b14ff4-f165-44c5-af90-d6a742ff7f6f/flow
curl --location 'http://localhost:3000/api/v1/build/47b14ff4-f165-44c5-af90-d6a742ff7f6f/flow' \
--header 'x-api-key: sk-xxx-xxx' \
--header 'Content-Type: application/json' \
--data '{
    "inputs": {
        "input_value": "hello",
        "session": "xxx"
    }
}'

Expected behavior

Release unused db connection.

Who can help?

@ogabrielluiz

Operating System

mac

Langflow Version

v1.1.1

Python Version

3.12

Screenshot

Every curl will leak a db connection:

Clipboard_Screenshot_1736306030

I debugged and found the problem is in build_flow->event_generator:
Clipboard_Screenshot_1736306163

If not data, build_graph_and_get_order will run by asyncio.create_task, this will cause AsyncDbSession doesn't close.

If run build_graph_and_get_order directly with await, AsyncDbSession close correctly.

Flow File

No response

@yangxikun yangxikun added the bug Something isn't working label Jan 8, 2025
Copy link

dosubot bot commented Jan 8, 2025

The issue you're encountering with the database connection leak seems to be related to how the build_graph_and_get_order function is being executed. When using asyncio.create_task, the AsyncDbSession might not be closing properly, leading to a connection pool overflow.

To address this, you can try running build_graph_and_get_order directly with await instead of using asyncio.create_task. This change ensures that the AsyncDbSession is properly managed and closed, preventing the connection leak.

Here's a potential modification to the event_generator function:

async def event_generator(event_manager: EventManager, client_consumed_queue: asyncio.Queue) -> None:
    if not data:
        try:
            ids, vertices_to_run, graph = await build_graph_and_get_order()
        except asyncio.CancelledError:
            return
        except Exception as e:
            error_message = ErrorMessage(
                flow_id=flow_id,
                exception=e,
            )
            event_manager.on_error(data=error_message.data)
            raise
    else:
        try:
            ids, vertices_to_run, graph = await build_graph_and_get_order()
        except Exception as e:
            error_message = ErrorMessage(
                flow_id=flow_id,
                exception=e,
            )
            event_manager.on_error(data=error_message.data)
            raise

    event_manager.on_vertices_sorted(data={"ids": ids, "to_run": vertices_to_run})
    await client_consumed_queue.get()

    tasks = []
    for vertex_id in ids:
        task = asyncio.create_task(build_vertices(vertex_id, graph, client_consumed_queue, event_manager))
        tasks.append(task)
    try:
        await asyncio.gather(*tasks)
    except asyncio.CancelledError:
        for task in tasks:
            task.cancel()
        return
    except Exception as e:
        logger.error(f"Error building vertices: {e}")
        error_message = ErrorMessage(
            flow_id=flow_id,
            exception=e,
        )
        event_manager.on_error(data=error_message.data)
        raise
    event_manager.on_end(data={})
    await event_manager.queue.put((None, None, time.time))

By using await directly, you ensure that the session lifecycle is properly managed, which should help in releasing unused database connections.

To continue talking to Dosu, mention @dosu.


Help Dosu learn! Give it feedback: Great Response | Irrelevant Answer | Incorrect Sources | Too Verbose | Hallucination | Other

@severfire
Copy link

severfire commented Jan 8, 2025

@yangxikun is it for both sqllite and postgresql databases? I found out that there is somewhere leak in memory, while i run like 25k api calls, the RAM usage can go even to 10GB or more.

may be related:
#5287
#5275
#4008

@Cristhianzl

@yangxikun
Copy link
Author

@severfire I just test postgresql

@Cristhianzl
Copy link
Member

@severfire we are already working to improve It!
thanks for your feedback!

@severfire
Copy link

@Cristhianzl thank you! please let me know how I can help here. In general I am using sqlite, as I do not store really data in langflow. Just using it to analyze what I need via API and I use simple installation with python or uv. For me the goal is to have like up to 500k API calls a month cause my client seems to grow. I love langflow and I would love to use it, yet I am worried it could be optimized bit better :-) for example if the worker could be lighter regarding RAM usage, memory leaks etc... I hate to complain and so on, yet I really want to consider langflow for business use :-)

@cbornet
Copy link
Collaborator

cbornet commented Jan 14, 2025

Well, although it is not really useful to create a task and to explicitly propagate the cancellation (cancelations are already propagated by coroutines awaiting other coroutines), I don't see why it makes a difference for the AsyncDbSession.
So we can make the change but it would be nice to understand why it fixes the issue.
Also it seems the behavior is identical whether data is None or not so the if loop doesn't seem needed.

@severfire
Copy link

@cbornet Hello, Were you able to see #5275 ? Hmm... Do you see RAM memory increase when doing a lot of API calls? :-/ Thank you for your hard work!

@cbornet
Copy link
Collaborator

cbornet commented Jan 17, 2025

Do you see RAM memory increase when doing a lot of API calls?

We have done some fixes for the memory leaks. It seems to be a different issue there with DB connections.

@cbornet
Copy link
Collaborator

cbornet commented Jan 17, 2025

@yangxikun I couldn't reproduce the issue on latest main branch. I tested both sqlite and postgres.
Can you check if you still have the issue on latest main ?

@yangxikun
Copy link
Author

@cbornet I tested the latest main branch, and there is no database connection leak. #5516 fix this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants