Fix a bug in background updates wherein background updates are never …

…run using the default batch size (#12157)
matrix-org · Mar 7, 2022 · 26211fe · 26211fe
1 parent f63bede
commit 26211fe
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 15 deletions.
diff --git a/changelog.d/12157.bugfix b/changelog.d/12157.bugfix
@@ -0,0 +1 @@
+Fix a bug introduced in #4864 whereby background updates are never run with the default background batch size.
diff --git a/synapse/storage/background_updates.py b/synapse/storage/background_updates.py
@@ -102,10 +102,12 @@ def average_items_per_ms(self) -> Optional[float]:
         Returns:
             A duration in ms as a float
         """
-        if self.avg_duration_ms == 0:
-            return 0
-        elif self.total_item_count == 0:
+        # We want to return None if this is the first background update item
+        if self.total_item_count == 0:
             return None
+        # Avoid dividing by zero
+        elif self.avg_duration_ms == 0:
+            return 0
         else:
             # Use the exponential moving average so that we can adapt to
             # changes in how long the update process takes.

diff --git a/tests/rest/admin/test_background_updates.py b/tests/rest/admin/test_background_updates.py
@@ -155,10 +155,10 @@ def test_status_bg_update(self) -> None:
                 "current_updates": {
                     "master": {
                         "name": "test_update",
-                        "average_items_per_ms": 0.001,
+                        "average_items_per_ms": 0.1,
                         "total_duration_ms": 1000.0,
                         "total_item_count": (
-                            BackgroundUpdater.MINIMUM_BACKGROUND_BATCH_SIZE
+                            BackgroundUpdater.DEFAULT_BACKGROUND_BATCH_SIZE
                         ),
                     }
                 },
@@ -210,10 +210,10 @@ def test_enabled(self) -> None:
                 "current_updates": {
                     "master": {
                         "name": "test_update",
-                        "average_items_per_ms": 0.001,
+                        "average_items_per_ms": 0.1,
                         "total_duration_ms": 1000.0,
                         "total_item_count": (
-                            BackgroundUpdater.MINIMUM_BACKGROUND_BATCH_SIZE
+                            BackgroundUpdater.DEFAULT_BACKGROUND_BATCH_SIZE
                         ),
                     }
                 },
@@ -239,10 +239,10 @@ def test_enabled(self) -> None:
                 "current_updates": {
                     "master": {
                         "name": "test_update",
-                        "average_items_per_ms": 0.001,
+                        "average_items_per_ms": 0.1,
                         "total_duration_ms": 1000.0,
                         "total_item_count": (
-                            BackgroundUpdater.MINIMUM_BACKGROUND_BATCH_SIZE
+                            BackgroundUpdater.DEFAULT_BACKGROUND_BATCH_SIZE
                         ),
                     }
                 },
@@ -278,11 +278,9 @@ def test_enabled(self) -> None:
                 "current_updates": {
                     "master": {
                         "name": "test_update",
-                        "average_items_per_ms": 0.001,
+                        "average_items_per_ms": 0.05263157894736842,
                         "total_duration_ms": 2000.0,
-                        "total_item_count": (
-                            2 * BackgroundUpdater.MINIMUM_BACKGROUND_BATCH_SIZE
-                        ),
+                        "total_item_count": (110),
                     }
                 },
                 "enabled": True,

diff --git a/tests/storage/test_background_update.py b/tests/storage/test_background_update.py
@@ -66,13 +66,13 @@ async def update(progress, count):
         self.update_handler.reset_mock()
         res = self.get_success(
             self.updates.do_next_background_update(False),
-            by=0.01,
+            by=0.02,
         )
         self.assertFalse(res)
 
         # on the first call, we should get run with the default background update size
         self.update_handler.assert_called_once_with(
-            {"my_key": 1}, self.updates.MINIMUM_BACKGROUND_BATCH_SIZE
+            {"my_key": 1}, self.updates.DEFAULT_BACKGROUND_BATCH_SIZE
         )
 
         # second step: complete the update
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Fix a bug introduced in #4864 whereby background updates are never run with the default background batch size.