Reset github connections on every sync iteration

The HTTP connections to GitHub are managed by pygithub library. It
uses a connection pool, initialized when Github object is created (in
KPD we have one per BranchWorker), and the connections are actively
reused.

KPD creates the BranchWorkers, and by extension the Github objects,
only once at start up. However KPD runs the main sync_patches()
routine once every two minutes, which means that actual underlying
connections are very likely to become stale.

Of course, there is an encapsulated logic in pygithub to re-establish
the connections. However it appears that attempting to use a
connection that was never explicitly closed, causes issues when
interacting with ttlsproxy. In particular we often get
AsyncSocketException, which appears to be correlated with KPD's
misbehavior with respect to email notifications.

Attempt to mitigate these problems by re-creating GithubSync object
(which spawns BranchWorker-s) on every iteration of the main daemon
loop (that is: every 2 mins).

Differential Revision: D80043971
Reviewed-by: Song Liu <[email protected]>
Signed-off-by: Ihor Solodrai <[email protected]>

Author: theihor

kernel_patches_daemon/daemon.py

@@ -30,28 +30,49 @@ def __init__(
         loop_delay: int = DEFAULT_LOOP_DELAY,
     ) -> None:
         self.project: str = kpd_config.patchwork.project
-        self.github_sync_worker = GithubSync(
-            kpd_config=kpd_config, labels_cfg=labels_cfg
-        )
+        self.kpd_config = kpd_config
+        self.labels_cfg = labels_cfg
         self.loop_delay: Final[int] = loop_delay
         self.metrics_logger = metrics_logger
+        self.github_sync_worker: GithubSync = GithubSync(
+            kpd_config=self.kpd_config, labels_cfg=self.labels_cfg
+        )
+
+    def reset_github_sync(self) -> bool:
+        try:
+            self.github_sync_worker = GithubSync(
+                kpd_config=self.kpd_config, labels_cfg=self.labels_cfg
+            )
+            return True
+        except Exception:
+            logger.exception(
+                "Unhandled exception while creating GithubSync object",
+                exc_info=True,
+            )
+            return False
 
     async def submit_metrics(self) -> None:
-        if self.metrics_logger:
-            logger.info("Submitting run metrics into metrics logger")
-            try:
-                self.metrics_logger(self.project, self.github_sync_worker.stats)
-            except Exception:
-                logger.exception(
-                    "Failed to submit run metrics into metrics logger", exc_info=True
-                )
-        else:
+        if self.metrics_logger is None:
             logger.warn(
                 "Not submitting run metrics because metrics logger is not configured"
             )
+            return
+        try:
+            self.metrics_logger(self.project, self.github_sync_worker.stats)
+            logger.info("Submitted run metrics into metrics logger")
+        except Exception:
+            logger.exception(
+                "Failed to submit run metrics into metrics logger", exc_info=True
+            )
 
     async def run(self) -> None:
         while True:
+            ok = self.reset_github_sync()
+            if not ok:
+                logger.error(
+                    "Most likely something went wrong connecting to GitHub or Patchwork. Skipping this iteration without submitting metrics."
+                )
+                continue
             try:
                 await self.github_sync_worker.sync_patches()
                 self.github_sync_worker.increment_counter("runs_successful")

tests/test_daemon.py

@@ -6,7 +6,7 @@
 
 import unittest
 from typing import Any, Dict, List
-from unittest.mock import AsyncMock, patch
+from unittest.mock import AsyncMock, MagicMock, patch
 
 from kernel_patches_daemon.config import KPDConfig
 from kernel_patches_daemon.daemon import KernelPatchesWorker
@@ -68,6 +68,7 @@ def setUp(self) -> None:
         )
 
         self.worker.github_sync_worker.sync_patches = AsyncMock()
+        self.worker.reset_github_sync = MagicMock(return_value=True)
 
     async def test_run_ok(self) -> None:
         with (
@@ -81,6 +82,7 @@ async def test_run_ok(self) -> None:
 
         gh_sync = self.worker.github_sync_worker
         gh_sync.sync_patches.assert_called_once()
+        self.worker.reset_github_sync.assert_called_once()
         self.assertEqual(len(LOGGED_METRICS), 1)
         stats = LOGGED_METRICS[0][self.worker.project]
         self.assertEqual(stats["runs_successful"], 1)