Merge "Speed up starting cinder-backup"

cinder/backup/manager.py

@@ -197,8 +197,14 @@ class BackupManager(manager.SchedulerDependentManager):
         LOG.info("Cleaning up incomplete backup operations.")
 
         # TODO(smulcahy) implement full resume of backup and restore
-        # operations on restart (rather than simply resetting)
+        # operations on restart (rather than simply resetting).
-        backups = objects.BackupList.get_all_by_host(ctxt, self.host)
+        # We only need to deal with the backups that aren't complete.
+        # N.B. NULL status is possible and we consider it incomplete.
+        incomplete_status = list(fields.BackupStatus.ALL)
+        incomplete_status.remove(fields.BackupStatus.AVAILABLE)
+        incomplete_status.append(None)
+        backups = objects.BackupList.get_all(
+            ctxt, filters={'host': self.host, 'status': incomplete_status})
         for backup in backups:
             try:
                 self._cleanup_one_backup(ctxt, backup)

cinder/db/sqlalchemy/api.py

@@ -6437,6 +6437,9 @@ def _process_backups_filters(query, filters):
                 col_attr = getattr(models.Backup, 'backup_metadata')
                 for k, v in value.items():
                     query = query.filter(col_attr.any(key=k, value=v))
+            elif isinstance(value, (list, tuple, set, frozenset)):
+                orm_field = getattr(models.Backup, key)
+                query = query.filter(or_(orm_field == v for v in value))
             else:
                 filters_dict[key] = value
 

cinder/tests/unit/backup/test_backup.py

@@ -397,15 +397,57 @@ class BackupTestCase(BaseBackupTest):
         self.backup_mgr.is_initialized = initialized
         self.assertEqual(initialized, self.backup_mgr.is_working())
 
+    def test_cleanup_incomplete_backup_operations(self):
+        """Test cleanup resilience when some are incomplete."""
+        # For correct operation, this test relies on the DB being
+        # pre-populated by a properly complete backup.
+
+        fake_incomplete_backup_list = [
+            self._create_backup_db_entry(status=s)
+            for s in (None,
+                      fields.BackupStatus.CREATING,
+                      fields.BackupStatus.DELETING,
+                      fields.BackupStatus.RESTORING,
+                      fields.BackupStatus.ERROR,
+                      fields.BackupStatus.ERROR_DELETING,
+                      fields.BackupStatus.DELETED)
+        ]
+
+        self._create_backup_db_entry(status=fields.BackupStatus.AVAILABLE)
+
+        mock_backup_cleanup = self.mock_object(
+            self.backup_mgr, '_cleanup_one_backup')
+        mock_temp_cleanup = self.mock_object(
+            self.backup_mgr, '_cleanup_temp_volumes_snapshots_for_one_backup')
+
+        result = self.backup_mgr._cleanup_incomplete_backup_operations(
+            self.ctxt)
+        self.assertIsNone(result)
+
+        self.assertEqual(len(fake_incomplete_backup_list),
+                         mock_backup_cleanup.call_count)
+        for b in fake_incomplete_backup_list:
+            mock_backup_cleanup.assert_any_call(self.ctxt, b)
+
+        self.assertEqual(len(fake_incomplete_backup_list),
+                         mock_temp_cleanup.call_count)
+        for b in fake_incomplete_backup_list:
+            mock_temp_cleanup.assert_any_call(self.ctxt, b)
+
     def test_cleanup_incomplete_backup_operations_with_exceptions(self):
         """Test cleanup resilience in the face of exceptions."""
 
-        fake_backup_list = [{'id': fake.BACKUP_ID},
+        fake_backup_list = [
-                            {'id': fake.BACKUP2_ID},
+            self._create_backup_db_entry(status=s)
-                            {'id': fake.BACKUP3_ID}]
+            for s in (fields.BackupStatus.CREATING,
-        mock_backup_get_by_host = self.mock_object(
+                      fields.BackupStatus.DELETING,
-            objects.BackupList, 'get_all_by_host')
+                      fields.BackupStatus.RESTORING,
-        mock_backup_get_by_host.return_value = fake_backup_list
+                      fields.BackupStatus.ERROR,
+                      fields.BackupStatus.ERROR_DELETING,
+                      fields.BackupStatus.DELETED)
+        ]
+
+        self._create_backup_db_entry(status=fields.BackupStatus.AVAILABLE)
 
         mock_backup_cleanup = self.mock_object(
             self.backup_mgr, '_cleanup_one_backup')
@@ -663,7 +705,8 @@ class BackupTestCase(BaseBackupTest):
 
     def test_create_backup_with_bad_volume_status(self):
         """Test creating a backup from a volume with a bad status."""
-        vol_id = self._create_volume_db_entry(status='restoring', size=1)
+        vol_id = self._create_volume_db_entry(
+            status='restoring-backup', size=1)
         backup = self._create_backup_db_entry(volume_id=vol_id)
         self.assertRaises(exception.InvalidVolume,
                           self.backup_mgr.create_backup,

releasenotes/notes/speed-up-starting-cinder-backup-76c1618b4cdb9d6e.yaml

@@ -0,0 +1,10 @@
+---
+other:
+  - |
+    The Cinder Backup service examined every known backup upon startup
+    previously, in order to restart the incomplete backups. This was
+    a problem for installations with a large number of backups.
+    We now use one database request in order to compile a list
+    of incomplete backups.
+    See Change-Id `I5c6065d99116ae5f223799e8558d25777aedd055
+    <https://review.opendev.org/#/q/I5c6065d99116ae5f223799e8558d25777aedd055>`.