Skip to content

Commit

Permalink
Improve REST app shutdown
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Mátyás Kuti committed Jan 19, 2024
1 parent ce345fb commit f69219a
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 1 deletion.
1 change: 1 addition & 0 deletions karapace/kafka_rest_apis/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion karapace/rapu.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down

0 comments on commit f69219a

Please sign in to comment.