From 92d3cd509a734d59dbdc7601fc1c044d87f4ce9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1ty=C3=A1s=20Kuti?= Date: Thu, 14 Dec 2023 09:59:27 +0100 Subject: [PATCH] Improve REST app shutdown REST webapp improvement: Using `on_cleanup` is not the correct hook to use here, as this would run _after_ the event loop has closed, making it unsuitable for cancelling background tasks for example. `on_shutdown` is triggered before the REST app shuts down, thus it's able to clean up eg. Kafka clients, background tasks, etc. properly. Before this change, the symptom of the bug is most prevalent in the Karapace REST proxy and its "idle proxy janitor" background task. Stopping the application when the janitor task is not running is straightforward, however when any `UserRestProxy` is present (ie. some requests have already been handled) and the task is running, stopping the REST proxy hangs or needs multiple signals to shut down. With the new `AIOKafkaProducer` implementation (which runs a poll-thread in the background) this results in an application that is unable to gracefully shutdown, only SIGKILL works. Using the `on_shutdown` hook fixes this issue, as we still have an event loop available to be able to cancel background tasks, etc. --- karapace/kafka_rest_apis/__init__.py | 1 + karapace/rapu.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/karapace/kafka_rest_apis/__init__.py b/karapace/kafka_rest_apis/__init__.py index 3c8bfc67c..54929bdf1 100644 --- a/karapace/kafka_rest_apis/__init__.py +++ b/karapace/kafka_rest_apis/__init__.py @@ -72,6 +72,7 @@ def __init__(self, config: Config) -> None: self._idle_proxy_janitor_task: Optional[asyncio.Task] = None async def close(self) -> None: + log.info("Closing REST proxy application") if self._idle_proxy_janitor_task is not None: self._idle_proxy_janitor_task.cancel() self._idle_proxy_janitor_task = None diff --git a/karapace/rapu.py b/karapace/rapu.py index 2b9decf12..b02a29fb1 100644 --- a/karapace/rapu.py +++ b/karapace/rapu.py @@ -167,7 +167,7 @@ def __init__( self.app = self._create_aiohttp_application(config=config) self.log = logging.getLogger(self.app_name) self.stats = StatsClient(config=config) - self.app.on_cleanup.append(self.close_by_app) + self.app.on_shutdown.append(self.close_by_app) self.not_ready_handler = not_ready_handler def _create_aiohttp_application(self, *, config: Config) -> aiohttp.web.Application: