From cff9dc5918232f3868be4c68432d2118c5e47e41 Mon Sep 17 00:00:00 2001 From: Daniel Goldstein Date: Thu, 18 Jan 2024 13:19:14 -0500 Subject: [PATCH] [ci] Push cleanup methods in on_startup to ensure proper execution order (#14172) The current `on_cleanup` code carefully attempts to close resources in the correct order (if B depends on A, we should close B before we close A). Doing so is pretty error prone though and we have messed it up in the past, leading to noisy error logs when pods are shut down. If we instead push `.close` methods onto a stack immediately after they are initialized, the exit stack cannot be executed in the wrong order. --- ci/ci/ci.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/ci/ci/ci.py b/ci/ci/ci.py index 0ad0bced0e5..b97c6e1e0fd 100644 --- a/ci/ci/ci.py +++ b/ci/ci/ci.py @@ -767,17 +767,24 @@ class AppKeys(CommonAiohttpAppKeys): FROZEN_MERGE_DEPLOY = web.AppKey('frozen_merge_deploy', bool) TASK_MANAGER = web.AppKey('task_manager', aiotools.BackgroundTaskManager) DEVELOPERS = web.AppKey('developers', List[UserData]) + EXIT_STACK = web.AppKey('exit_stack', AsyncExitStack) async def on_startup(app: web.Application): + exit_stack = AsyncExitStack() + app[AppKeys.EXIT_STACK] = exit_stack + client_session = httpx.client_session() + exit_stack.push_async_callback(client_session.close) app[AppKeys.CLIENT_SESSION] = client_session app[AppKeys.GH_CLIENT] = gh_aiohttp.GitHubAPI(client_session.client_session, 'ci', oauth_token=oauth_token) app[AppKeys.BATCH_CLIENT] = await BatchClient.create('ci') + exit_stack.push_async_callback(app[AppKeys.BATCH_CLIENT].close) app[AppKeys.DB] = Database() await app[AppKeys.DB].async_init() + exit_stack.push_async_callback(app[AppKeys.DB].async_close) row = await app[AppKeys.DB].select_and_fetchone( """ @@ -787,6 +794,7 @@ async def on_startup(app: web.Application): app[AppKeys.FROZEN_MERGE_DEPLOY] = row['frozen_merge_deploy'] app[AppKeys.TASK_MANAGER] = aiotools.BackgroundTaskManager() + exit_stack.callback(app[AppKeys.TASK_MANAGER].shutdown) if DEFAULT_NAMESPACE == 'default': kubernetes_asyncio.config.load_incluster_config() @@ -817,11 +825,7 @@ async def on_startup(app: web.Application): async def on_cleanup(app: web.Application): - async with AsyncExitStack() as cleanup: - cleanup.push_async_callback(app[AppKeys.DB].async_close) - cleanup.push_async_callback(app[AppKeys.CLIENT_SESSION].close) - cleanup.push_async_callback(app[AppKeys.BATCH_CLIENT].close) - cleanup.callback(app[AppKeys.TASK_MANAGER].shutdown) + await app[AppKeys.EXIT_STACK].aclose() def run():