Replace while True loops with bounded iteration patterns

app/adapters/osu_mirrors/__init__.py

@@ -173,27 +173,26 @@ async def fetch_beatmap_zip_data(beatmapset_id: int) -> bytes | None:
     availability and performance.
     """
     prev_mirror: AbstractBeatmapMirror | None = None
-    num_attempts = 0
 
     await OSZ_FILE_MIRROR_SELECTOR.update_all_mirror_and_selector_weights()
 
-    while True:
+    # Retry up to 2x the number of mirrors, with extra iterations for skipped attempts
+    max_attempts = OSZ_FILE_MIRROR_SELECTOR.get_num_mirrors() * 2
+    max_iterations = max_attempts * 2  # Allow for skipped iterations due to prev_mirror
+    actual_attempts = 0
+
+    for _ in range(max_iterations):
+        if actual_attempts >= max_attempts:
+            break
+
         mirror = OSZ_FILE_MIRROR_SELECTOR.select_mirror()
         if mirror is prev_mirror:
             # don't allow the same mirror to be run twice, to help
             # prevent loops which cause the mirror to lose all weighting
             # because of an error on a single beatmapset
             continue
 
-        # Only retry up to 2x the number of mirrors
-        if num_attempts > OSZ_FILE_MIRROR_SELECTOR.get_num_mirrors() * 2:
-            logging.warning(
-                "Failed to fetch beatmapset osz from any mirror",
-                extra={"beatmapset_id": beatmapset_id},
-            )
-            return None
-
-        num_attempts += 1
+        actual_attempts += 1
 
         started_at = datetime.now()
         mirror_response = await mirror.fetch_beatmap_zip_data(beatmapset_id)
@@ -221,7 +220,21 @@ async def fetch_beatmap_zip_data(beatmapset_id: int) -> bytes | None:
         await OSZ_FILE_MIRROR_SELECTOR.update_all_mirror_and_selector_weights()
 
         if mirror_response.is_success:
-            break
+            ms_elapsed = (ended_at.timestamp() - started_at.timestamp()) * 1000
+
+            logging.debug(
+                "Served beatmapset osz from mirror",
+                extra={
+                    "mirror_name": mirror.name,
+                    "mirror_weight": mirror.weight,
+                    "beatmapset_id": beatmapset_id,
+                    "ms_elapsed": ms_elapsed,
+                    "data_size": (
+                        len(mirror_response.data) if mirror_response.data else 0
+                    ),
+                },
+            )
+            return mirror_response.data
 
         logging.warning(
             "Failed to fetch beatmapset osz from mirror",
@@ -237,21 +250,12 @@ async def fetch_beatmap_zip_data(beatmapset_id: int) -> bytes | None:
             },
         )
         prev_mirror = mirror
-        continue
-
-    ms_elapsed = (ended_at.timestamp() - started_at.timestamp()) * 1000
 
-    logging.debug(
-        "Served beatmapset osz from mirror",
-        extra={
-            "mirror_name": mirror.name,
-            "mirror_weight": mirror.weight,
-            "beatmapset_id": beatmapset_id,
-            "ms_elapsed": ms_elapsed,
-            "data_size": len(mirror_response.data) if mirror_response.data else 0,
-        },
+    logging.warning(
+        "Failed to fetch beatmapset osz from any mirror",
+        extra={"beatmapset_id": beatmapset_id},
     )
-    return mirror_response.data
+    return None
 
 
 async def fetch_beatmap_background_image(beatmap_id: int) -> bytes | None:
@@ -261,27 +265,26 @@ async def fetch_beatmap_background_image(beatmap_id: int) -> bytes | None:
     availability and performance.
     """
     prev_mirror: AbstractBeatmapMirror | None = None
-    num_attempts = 0
 
     await BACKGROUND_IMAGE_MIRROR_SELECTOR.update_all_mirror_and_selector_weights()
 
-    while True:
+    # Retry up to 2x the number of mirrors, with extra iterations for skipped attempts
+    max_attempts = BACKGROUND_IMAGE_MIRROR_SELECTOR.get_num_mirrors() * 2
+    max_iterations = max_attempts * 2  # Allow for skipped iterations due to prev_mirror
+    actual_attempts = 0
+
+    for _ in range(max_iterations):
+        if actual_attempts >= max_attempts:
+            break
+
         mirror = BACKGROUND_IMAGE_MIRROR_SELECTOR.select_mirror()
         if mirror is prev_mirror:
             # don't allow the same mirror to be run twice, to help
             # prevent loops which cause the mirror to lose all weighting
             # because of an error on a single beatmapset
             continue
 
-        # Only retry up to 2x the number of mirrors
-        if num_attempts > BACKGROUND_IMAGE_MIRROR_SELECTOR.get_num_mirrors() * 2:
-            logging.warning(
-                "Failed to fetch beatmap background image from any mirror",
-                extra={"beatmap_id": beatmap_id},
-            )
-            return None
-
-        num_attempts += 1
+        actual_attempts += 1
 
         started_at = datetime.now()
         mirror_response = await mirror.fetch_beatmap_background_image(beatmap_id)
@@ -302,7 +305,21 @@ async def fetch_beatmap_background_image(beatmap_id: int) -> bytes | None:
         await BACKGROUND_IMAGE_MIRROR_SELECTOR.update_all_mirror_and_selector_weights()
 
         if mirror_response.is_success:
-            break
+            ms_elapsed = (ended_at.timestamp() - started_at.timestamp()) * 1000
+
+            logging.debug(
+                "Served beatmap background image from mirror",
+                extra={
+                    "mirror_name": mirror.name,
+                    "mirror_weight": mirror.weight,
+                    "beatmap_id": beatmap_id,
+                    "ms_elapsed": ms_elapsed,
+                    "data_size": (
+                        len(mirror_response.data) if mirror_response.data else 0
+                    ),
+                },
+            )
+            return mirror_response.data
 
         logging.warning(
             "Failed to fetch beatmap background image from mirror",
@@ -318,18 +335,9 @@ async def fetch_beatmap_background_image(beatmap_id: int) -> bytes | None:
             },
         )
         prev_mirror = mirror
-        continue
-
-    ms_elapsed = (ended_at.timestamp() - started_at.timestamp()) * 1000
 
-    logging.debug(
-        "Served beatmap background image from mirror",
-        extra={
-            "mirror_name": mirror.name,
-            "mirror_weight": mirror.weight,
-            "beatmap_id": beatmap_id,
-            "ms_elapsed": ms_elapsed,
-            "data_size": len(mirror_response.data) if mirror_response.data else 0,
-        },
+    logging.warning(
+        "Failed to fetch beatmap background image from any mirror",
+        extra={"beatmap_id": beatmap_id},
     )
-    return mirror_response.data
+    return None

app/adapters/osu_mirrors/selectors/dynamic_round_robin.py

@@ -41,7 +41,16 @@ async def update_all_mirror_and_selector_weights(self) -> None:
         )
 
     def select_mirror(self) -> AbstractBeatmapMirror:
-        while True:
+        # Maximum iterations is bounded by the number of mirrors * max_weight / gcd_weight
+        # Add safety factor of 2x to handle edge cases
+        max_iterations = (
+            len(self.mirrors) * (self.max_weight // max(self.gcd_weight, 1)) * 2
+        )
+        max_iterations = max(
+            max_iterations, len(self.mirrors) * 10
+        )  # Ensure minimum iterations
+
+        for _ in range(max_iterations):
             self.index = (self.index + 1) % len(self.mirrors)
             if self.index == 0:
                 self.current_weight -= self.gcd_weight
@@ -53,5 +62,11 @@ def select_mirror(self) -> AbstractBeatmapMirror:
             if self.mirrors[self.index].weight >= self.current_weight:
                 return self.mirrors[self.index]
 
+        # This should never happen in practice with correct weights, but provides safety
+        raise RuntimeError(
+            f"Failed to select a mirror after {max_iterations} iterations. "
+            "This indicates a bug in the weighted round-robin algorithm."
+        )
+
     def get_num_mirrors(self) -> int:
         return len(self.mirrors)