Check database in has_completed_background_updates

so that the right thing happens on workers.

3 files changed, 33 insertions, 12 deletions

diff --git a/synapse/rest/media/v1/preview_url_resource.py b/synapse/rest/media/v1/preview_url_resource.py
index dd76e3f7d5..385e4079ec 100644
--- a/synapse/rest/media/v1/preview_url_resource.py
+++ b/synapse/rest/media/v1/preview_url_resource.py
@@ -354,7 +354,7 @@ class PreviewUrlResource(Resource):
 
         logger.info("Running url preview cache expiry")
 
-        if not self.store.has_completed_background_updates():
+        if not (yield self.store.has_completed_background_updates()):
             logger.info("Still running DB updates; skipping expiry")
             return
 
diff --git a/synapse/storage/_base.py b/synapse/storage/_base.py
index e6eefdd6fe..476c84c621 100644
--- a/synapse/storage/_base.py
+++ b/synapse/storage/_base.py
@@ -600,20 +600,18 @@ class SQLBaseStore(object):
 
     @staticmethod
     def _simple_select_onecol_txn(txn, table, keyvalues, retcol):
-        if keyvalues:
-            where = "WHERE %s" % " AND ".join("%s = ?" % k for k in keyvalues.iterkeys())
-        else:
-            where = ""
-
         sql = (
-            "SELECT %(retcol)s FROM %(table)s %(where)s"
+            "SELECT %(retcol)s FROM %(table)s"
         ) % {
             "retcol": retcol,
             "table": table,
-            "where": where,
         }
 
-        txn.execute(sql, keyvalues.values())
+        if keyvalues:
+            sql += "WHERE %s" % " AND ".join("%s = ?" % k for k in keyvalues.iterkeys())
+            txn.execute(sql, keyvalues.values())
+        else:
+            txn.execute(sql)
 
         return [r[0] for r in txn]
 
@@ -624,7 +622,7 @@ class SQLBaseStore(object):
 
         Args:
             table (str): table name
-            keyvalues (dict): column names and values to select the rows with
+            keyvalues (dict|None): column names and values to select the rows with
             retcol (str): column whos value we wish to retrieve.
 
         Returns:
diff --git a/synapse/storage/background_updates.py b/synapse/storage/background_updates.py
index e755afc18e..11a1b942f1 100644
--- a/synapse/storage/background_updates.py
+++ b/synapse/storage/background_updates.py
@@ -110,13 +110,36 @@ class BackgroundUpdateStore(SQLBaseStore):
                     self._all_done = True
                     defer.returnValue(None)
 
+    @defer.inlineCallbacks
     def has_completed_background_updates(self):
         """Check if all the background updates have completed
 
         Returns:
-            bool: True if all background updates have completed
+            Deferred[bool]: True if all background updates have completed
         """
-        return self._all_done
+        # if we've previously determined that there is nothing left to do, that
+        # is easy
+        if self._all_done:
+            defer.returnValue(True)
+
+        # obviously, if we have things in our queue, we're not done.
+        if self._background_update_queue:
+            defer.returnValue(False)
+
+        # otherwise, check if there are updates to be run. This is important,
+        # as we may be running on a worker which doesn't perform the bg updates
+        # itself, but still wants to wait for them to happen.
+        updates = yield self._simple_select_onecol(
+            "background_updates",
+            keyvalues=None,
+            retcol="1",
+            desc="check_background_updates",
+        )
+        if not updates:
+            self._all_done = True
+            defer.returnValue(True)
+
+        defer.returnValue(False)
 
     @defer.inlineCallbacks
     def do_next_background_update(self, desired_duration_ms):