Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Fix a bug in background updates wherein background updates are never run using the default batch size #12157

Merged
merged 7 commits into from
Mar 7, 2022
Merged

Fix a bug in background updates wherein background updates are never run using the default batch size #12157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9941a69
first bit of bugfix
H-Shay Mar 2, 2022
50f876e
fix test
H-Shay Mar 3, 2022
d95de98
fix test
H-Shay Mar 3, 2022
94243e0
newsfragment
H-Shay Mar 3, 2022
ea36f9a
Merge branch 'develop' into shay/fix_background_update
H-Shay Mar 3, 2022
36008b5
lint
H-Shay Mar 3, 2022
17148a5
newsfragment
H-Shay Mar 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/12157.bugfix
View file
Open in desktop
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.
8 changes: 5 additions & 3 deletions synapse/storage/background_updates.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
18 changes: 8 additions & 10 deletions tests/rest/admin/test_background_updates.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
),
}
},
Expand Down Expand Up @@ -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
),
}
},
Expand All @@ -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
),
}
},
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions tests/storage/test_background_update.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down