microsoft / sample-app-aoai-chatGPT

Sample code for a simple web chat experience through Azure OpenAI, including Azure OpenAI On Your Data.
MIT License
1.54k stars 2.38k forks source link

ERROR:asyncio:Unclosed client session #751

Closed qad002 closed 1 month ago

qad002 commented 4 months ago

Describe the bug ERROR:asyncio:Unclosed client session

To Reproduce Steps to reproduce the behavior:

  1. Use latest updates you provided
  2. Unsure how to reproduce except for enabling chat history and making multiple concurrent requests.
  3. We were not able to reproduce in our QA env so far but it seems to happen in production with about 200 users per day.

Expected behavior Expected the client sessions to close but for some reason it is staying open.

Screenshots

2024-04-02T11:04:36.654029827Z ERROR:asyncio:Unclosed client session
2024-04-02T11:04:36.654049227Z client_session: <aiohttp.client.ClientSession object at 0x706d143d01d0>

Configuration: Please provide the following

Additional context Add any other context about the problem here.

Hadi2525 commented 4 months ago

@qad002 Can you share what hardware tier you're using for the Web Service? Do you have an estimate population of users accessing the web app?

Check Your Azure Logs for the subscription you are running the App Service, run AppServiceHTTPLogs and watch for crashes or code errors.

QuentinAd commented 4 months ago

@abhahn I have been able to reproduce. What happens is in app.py the init_cosmosdb_client(), whenever AZURE_COSMOSDB_ACCOUNT_KEY is not used, the credential uses the azure.identity.aio import DefaultAzureCredential which is the async declination, but the DefaultAzureCredential session is not being closed, as seen here:

            if not AZURE_COSMOSDB_ACCOUNT_KEY:
                credential = DefaultAzureCredential()
            else:
                credential = AZURE_COSMOSDB_ACCOUNT_KEY

What has worked for me in removing this memory leak error message was applying context manager to this async method and adding async to the init_cosmosdb_client() and adding await whenever it is instantiated.


  async def init_cosmosdb_client():
    cosmos_conversation_client = None
    if CHAT_HISTORY_ENABLED:
        try:
            cosmos_endpoint = (
                f"https://{AZURE_COSMOSDB_ACCOUNT}.documents.azure.com:443/"
            )

            if not AZURE_COSMOSDB_ACCOUNT_KEY:
                async with DefaultAzureCredential() as credential:
                    credential = credential
            else:
                credential = AZURE_COSMOSDB_ACCOUNT_KEY

Your opinion is most welcome on this fix.

Epstone commented 4 months ago

Same issue as this one I guess:

593 Unclosed client session error

I still have the cosmos features deactivated to get around this. Maybe one could find the time for a PR? :)

beplay commented 4 months ago

When looking at the azure cosmos Python library, the CosmosClient class says:

... It's recommended to maintain a single instance of CosmosClient per lifetime of the application which enables efficient connection management and performance. CosmosClient initialization is a heavy operation, ...

Hence, I assumed it would be better to create only a single instance that is used by the different functions rather than always creating a new instance when the database needs to be accessed.

What I changed (in addition to @QuentinAd's solution) is adding a database object to the application context and use a new init() function within create_app() to instantiate the database globally.

...

from quart import current_app
import asyncio

...

cosmos_db_ready = asyncio.Event()  # Functions can wait on the database to become available

def create_app():
    app = Quart(__name__)
    app.register_blueprint(bp)
    app.config["TEMPLATES_AUTO_RELOAD"] = True

    @app.before_serving
    async def init():
        try:
            app.cosmos_conversation_client = await init_cosmosdb_client()
            cosmos_db_ready.set()
        except Exception as e:
            logging.exception("Failed to initialize CosmosDB client")
            app.cosmos_conversation_client = None
            raise e

    return app

...

For every route, add await cosmos_db_ready.wait() right at the beginning, e.g.,

async def add_conversation():
    await cosmos_db_ready.wait()
    ...

Remove all occurrences of cosmos_conversation_client = init_cosmosdb_client() since this is already done when the app is created (i.e., here: app = create_app()), and, where necessary, use current_app to access the cosmos_conversation_client like this current_app.cosmos_conversation_client.

By doing that, the Unclosed client session error disappeared in my case.

github-actions[bot] commented 1 month ago

This issue is stale because it has been open for 30 days with no activity.

github-actions[bot] commented 1 month ago

This issue was closed because it has been inactive for 14 days since being marked as stale.