Compute parent revision at runtime

changelog.d/20230703_144928_jb_PL_131601_fix_rev_parent.rst

@@ -0,0 +1 @@
+- Compute parent revision at runtime

src/backy/backends/cowfile.py

@@ -15,10 +15,10 @@ def __init__(self, revision, log):
 
     def open(self, mode="rb"):
         if not os.path.exists(self.revision.filename):
-            if not self.revision.parent:
+            parent = self.revision.get_parent()
+            if not parent:
                 open(self.revision.filename, "wb").close()
             else:
-                parent = self.revision.backup.find(self.revision.parent)
                 cp_reflink(parent.filename, self.revision.filename)
             self.revision.writable()
         return open(self.revision.filename, mode, buffering=CHUNK_SIZE)

src/backy/revision.py

@@ -34,7 +34,6 @@ class Revision(object):
     backup: "backy.backup.Backup"
     uuid: str
     timestamp: datetime.datetime
-    parent: Optional[str] = None
     stats: dict
     tags: set[str]
     trust: Trust = Trust.TRUSTED
@@ -53,8 +52,6 @@ def __init__(self, backup, log, uuid=None, timestamp=None):
     def create(cls, backup, tags, log):
         r = Revision(backup, log)
         r.tags = tags
-        if backup.history:
-            r.parent = backup.history[-1].uuid
         r.backend_type = backup.backend_type
         return r
 
@@ -73,7 +70,6 @@ def load(cls, filename, backup, log):
         r = Revision(
             backup, log, uuid=metadata["uuid"], timestamp=metadata["timestamp"]
         )
-        r.parent = metadata["parent"]
         r.stats = metadata.get("stats", {})
         r.tags = set(metadata.get("tags", []))
         # Assume trusted by default to support migration
@@ -102,7 +98,9 @@ def write_info(self):
             "uuid": self.uuid,
             "backend_type": self.backend_type,
             "timestamp": self.timestamp,
-            "parent": self.parent,
+            "parent": getattr(
+                self.get_parent(), "uuid", ""
+            ),  # compatibility with older versions
             "stats": self.stats,
             "trust": self.trust.value,
             "tags": list(self.tags),
@@ -145,7 +143,11 @@ def readonly(self):
             os.chmod(self.filename, 0o440)
         os.chmod(self.info_filename, 0o440)
 
-    def get_parent(self):
-        if self.parent:
-            return self.backup.find_by_uuid(self.parent)
-        return
+    def get_parent(self) -> Optional["Revision"]:
+        """defaults to last rev if not in history"""
+        prev = None
+        for r in self.backup.history:
+            if r.uuid == self.uuid:
+                break
+            prev = r
+        return prev

src/backy/sources/ceph/source.py

@@ -71,22 +71,22 @@ def backup(self, target):
             return
         revision = self.revision
         while True:
-            if not revision.parent:
+            parent = revision.get_parent()
+            if not parent:
                 self.log.info("backup-no-valid-parent")
                 self.full(target)
                 return
-            parent = self.revision.backup.find(revision.parent)
             if parent.trust == Trust.DISTRUSTED:
                 self.log.info(
                     "ignoring-distrusted-rev",
-                    revision_uuid=revision.parent,
+                    revision_uuid=parent.uuid,
                 )
                 revision = parent
                 continue
             if not self.rbd.exists(self._image_name + "@backy-" + parent.uuid):
                 self.log.info(
                     "ignoring-rev-without-snapshot",
-                    revision_uuid=revision.parent,
+                    revision_uuid=parent.uuid,
                 )
                 revision = parent
                 continue

src/backy/sources/ceph/tests/test_ceph_source.py

@@ -1,3 +1,4 @@
+import datetime
 import io
 import os.path as p
 import subprocess
@@ -78,11 +79,9 @@ def test_context_manager_cleans_out_snapshots(
     # unexpected revision snapshots are cleaned
     source.rbd.snap_create("test/foo@backy-2")
 
-    revision = Revision(backup, log, "1")
+    revision = Revision(backup, log, "1", backy.utils.now())
     with source(revision):
         revision.materialize()
-        revision.timestamp = backy.utils.now()
-        revision.write_info()
         backup.scan()
 
     assert source.rbd.snap_ls("test/foo") == [
@@ -140,14 +139,12 @@ def test_choose_full_without_snapshot(
     source.diff = mock.Mock()
     source.full = mock.Mock()
 
-    revision1 = Revision(backup, log, "a1")
-    revision1.timestamp = backy.utils.now()
+    revision1 = Revision(backup, log, "a1", backy.utils.now())
     revision1.materialize()
 
     backup.scan()
 
     revision2 = Revision(backup, log, "a2")
-    revision2.parent = "a1"
 
     backend = backend_factory(revision2, log)
     with source(revision2):
@@ -170,8 +167,7 @@ def test_choose_diff_with_snapshot(
     source.diff = mock.Mock()
     source.full = mock.Mock()
 
-    revision1 = Revision(backup, log, "a1")
-    revision1.timestamp = backy.utils.now()
+    revision1 = Revision(backup, log, "a1", backy.utils.now())
     revision1.materialize()
 
     # part of test setup: we check backy's behavior when a previous version not only
@@ -181,7 +177,6 @@ def test_choose_diff_with_snapshot(
     backup.scan()
 
     revision2 = Revision(backup, log, "a2")
-    revision2.parent = "a1"
 
     backend = backend_factory(revision2, log)
     with source(revision2):
@@ -204,15 +199,19 @@ def test_diff_backup(
 
     source = ceph_rbd_imagesource
 
-    parent = Revision(backup, log, "ed968696-5ab0-4fe0-af1c-14cadab44661")
-    parent.timestamp = backy.utils.now()
+    parent = Revision(
+        backup, log, "ed968696-5ab0-4fe0-af1c-14cadab44661", backy.utils.now()
+    )
     parent.materialize()
 
     # Those revision numbers are taken from the sample snapshot and need
     # to match, otherwise our diff integration will (correctly) complain.
-    revision = Revision(backup, log, "f0e7292e-4ad8-4f2e-86d6-f40dca2aa802")
-    revision.timestamp = backy.utils.now()
-    revision.parent = parent.uuid
+    revision = Revision(
+        backup,
+        log,
+        "f0e7292e-4ad8-4f2e-86d6-f40dca2aa802",
+        backy.utils.now() + datetime.timedelta(seconds=1),
+    )
 
     with backend_factory(parent, log).open("wb") as f:
         f.write(b"asdf")
@@ -233,8 +232,7 @@ def test_diff_backup(
         )
         backend = backend_factory(revision, log)
         with source(revision):
-            parent = backup.find(revision.parent)
-            source.diff(backend, parent)
+            source.diff(backend, revision.get_parent())
             backup.history.append(revision)
         export.assert_called_with(
             "test/foo@backy-f0e7292e-4ad8-4f2e-86d6-f40dca2aa802",
@@ -258,8 +256,7 @@ def test_full_backup(
 
     # Those revision numbers are taken from the sample snapshot and need
     # to match, otherwise our diff integration will (correctly) complain.
-    revision = Revision(backup, log, "a0")
-    revision.timestamp = backy.utils.now()
+    revision = Revision(backup, log, "a0", backy.utils.now())
     revision.materialize()
     backup.scan()
 
@@ -277,8 +274,9 @@ def test_full_backup(
         assert f.read() == b"Han likes Leia."
 
     # Now make another full backup. This overwrites the first.
-    revision2 = Revision(backup, log, "a1")
-    revision2.parent = revision.uuid
+    revision2 = Revision(
+        backup, log, "a1", backy.utils.now() + datetime.timedelta(seconds=1)
+    )
     revision2.materialize()
     backup.scan()
 
@@ -310,13 +308,13 @@ def test_full_backup_integrates_changes(
     content0 = BLOCK * b"A" + BLOCK * b"B" + BLOCK * b"C" + BLOCK * b"D"
     content1 = BLOCK * b"A" + BLOCK * b"X" + BLOCK * b"\0" + BLOCK * b"D"
 
-    rev0 = Revision(backup, log, "a0")
-    rev0.timestamp = backy.utils.now()
+    rev0 = Revision(backup, log, "a0", backy.utils.now())
     rev0.materialize()
     backup.scan()
 
-    rev1 = Revision(backup, log, "a1")
-    rev1.parent = "a0"
+    rev1 = Revision(
+        backup, log, "a1", backy.utils.now() + datetime.timedelta(seconds=1)
+    )
     rev1.materialize()
 
     # check fidelity
@@ -342,8 +340,7 @@ def test_verify_fail(
 
     # Those revision numbers are taken from the sample snapshot and need
     # to match, otherwise our diff integration will (correctly) complain.
-    revision = Revision(backup, log, "a0")
-    revision.timestamp = backy.utils.now()
+    revision = Revision(backup, log, "a0", backy.utils.now())
     revision.materialize()
 
     backup.scan()
@@ -374,8 +371,7 @@ def test_verify(
 
     # Those revision numbers are taken from the sample snapshot and need
     # to match, otherwise our diff integration will (correctly) complain.
-    revision = Revision(backup, log, "a0")
-    revision.timestamp = backy.utils.now()
+    revision = Revision(backup, log, "a0", backy.utils.now())
     revision.materialize()
 
     backup.scan()

src/backy/tests/test_archive.py

@@ -56,7 +56,9 @@ def test_load_revisions(backup_with_revisions):
     a = backup_with_revisions
     assert [x.uuid for x in a.history] == ["123-0", "123-1", "123-2"]
     assert a.history[1].uuid == "123-1"
-    assert a.history[1].parent == "123-0"
+    assert a.history[1].get_parent().uuid == "123-0"
+    assert a.history[2].get_parent().uuid == "123-1"
+    assert a.history[0].get_parent() is None
     assert a.find_revisions("all") == a.history
     assert a.find_revisions(1) == [a.find(1)]
 

src/backy/tests/test_revision.py

@@ -31,7 +31,7 @@ def test_revision_create_child(backup, log):
     r = Revision.create(backup, {"test"}, log)
     assert r.uuid is not None
     assert r.tags == {"test"}
-    assert r.parent == "asdf"
+    assert r.get_parent().uuid == "asdf"
     assert (backy.utils.now() - r.timestamp).total_seconds() < 10
     assert r.backup is backup
 
@@ -40,15 +40,15 @@ def test_load_sample1(backup, log):
     r = Revision.load(SAMPLE_DIR + "/sample1.rev", backup, log)
     assert r.uuid == "asdf"
     assert r.timestamp == datetime.datetime(2015, 8, 1, 20, 0, tzinfo=UTC)
-    assert r.parent is None
+    assert r.get_parent() is None
     assert r.backup is backup
 
 
 def test_load_sample2(backup, log):
     r = Revision.load(SAMPLE_DIR + "/sample2.rev", backup, log)
     assert r.uuid == "asdf2"
     assert r.timestamp == datetime.datetime(2015, 8, 1, 21, 0, tzinfo=UTC)
-    assert r.parent == "asdf"
+    assert r.get_parent() is None
     assert r.backup is backup
 
 
@@ -61,8 +61,8 @@ def test_filenames_based_on_uuid_and_backup_dir(log):
 
 
 def test_store_revision_data(backup, clock, log):
+    backup.history = [Revision(backup, log, "asdf", backy.utils.now())]
     r = Revision(backup, log, "asdf2", backy.utils.now())
-    r.parent = "asdf"
     r.backup = backup
     r.write_info()
     with open(r.info_filename, encoding="utf-8") as info:
@@ -77,6 +77,22 @@ def test_store_revision_data(backup, clock, log):
         }
 
 
+def test_store_revision_data_no_parent(backup, clock, log):
+    r = Revision(backup, log, "asdf2", backy.utils.now())
+    r.backup = backup
+    r.write_info()
+    with open(r.info_filename, encoding="utf-8") as info:
+        assert yaml.safe_load(info) == {
+            "parent": "",
+            "backend_type": "chunked",
+            "uuid": "asdf2",
+            "stats": {"bytes_written": 0},
+            "tags": [],
+            "trust": "trusted",
+            "timestamp": datetime.datetime(2015, 9, 1, 7, 6, 47, tzinfo=UTC),
+        }
+
+
 def test_delete_revision(backup, log):
     r = Revision(backup, log, "123-456", backy.utils.now())
     r.materialize()