]> arthur.barton.de Git - bup.git/commitdiff
save-cmd: progress meter wouldn't count identical files correctly.
authorAvery Pennarun <apenwarr@gmail.com>
Thu, 4 Mar 2010 00:21:20 +0000 (19:21 -0500)
committerAvery Pennarun <apenwarr@gmail.com>
Thu, 4 Mar 2010 01:32:34 +0000 (20:32 -0500)
This one was really tricky.  If a file was IX_HASHVALID but its object
wasn't available on the target server (eg. if you backed up to one server
server and now are backing up to a different one), we could correctly count
is toward the total bytes we expected to back up.

Now imagine there are two *identical* files (ie. with the same sha1sum) in
this situation.  When that happens, we'd back up the first one, after which
the objects for the second one *are* available.  So we'd skip it, thinking
that we had skipped it in the first place.  The result would be that our
backup count showed a final byte percentage less than 100%.

The workaround isn't very pretty, but should be correct: we add a new
IX_SHAMISSING flag, setting or clearing it during the initial index scan,
and then we use *that* as the indicator of whether to add bytes to the count
or not.

We also have to decide whether to recurse into subdirectories using this
algorithm.  If /etc/rc3.d and /etc/rc4.d are identical, and one of the files
in them had this problem, then we wouldn't even *recurse* into /etc/rc3.d
after backing up /etc/rc4.d.  That means we wouldn't check the IX_SHAMISSING
flag on the file inside.  So we had to fix that up too.

On the other hand, this is an awful lot of complexity just to make the
progress messages more exact...

cmd/save-cmd.py
lib/bup/index.py

index 4a8743997910dba818b2c20828aff40bc542f1b6..d37ee85d2f80c4fe87eb258f496334c9034f9a15 100755 (executable)
@@ -107,16 +107,20 @@ r = index.Reader(git.repo('bupindex'))
 def already_saved(ent):
     return ent.is_valid() and w.exists(ent.sha) and ent.sha
 
-def wantrecurse(ent):
+def wantrecurse_pre(ent):
     return not already_saved(ent)
 
+def wantrecurse_during(ent):
+    return not already_saved(ent) or ent.sha_missing()
+
 total = ftotal = 0
-if opt.progress:
-    for (transname,ent) in r.filter(extra, wantrecurse=wantrecurse):
+if opt.progress or 1:
+    for (transname,ent) in r.filter(extra, wantrecurse=wantrecurse_pre):
         if not (ftotal % 10024):
             progress('Reading index: %d\r' % ftotal)
-        exists = (ent.flags & index.IX_EXISTS)
+        exists = ent.exists()
         hashvalid = already_saved(ent)
+        ent.set_sha_missing(not hashvalid)
         if exists and not hashvalid:
             total += ent.size
         ftotal += 1
@@ -125,10 +129,11 @@ if opt.progress:
 
 tstart = time.time()
 count = subcount = fcount = 0
-for (transname,ent) in r.filter(extra, wantrecurse=wantrecurse):
+for (transname,ent) in r.filter(extra, wantrecurse=wantrecurse_during):
     (dir, file) = os.path.split(ent.name)
     exists = (ent.flags & index.IX_EXISTS)
     hashvalid = already_saved(ent)
+    oldsize = ent.size
     if opt.verbose:
         if not exists:
             status = 'D'
@@ -164,8 +169,8 @@ for (transname,ent) in r.filter(extra, wantrecurse=wantrecurse):
         if not oldtree:
             ent.validate(040000, newtree)
             ent.repack()
-        if exists and not hashvalid:
-            count += ent.size
+        if exists and ent.sha_missing():
+            count += oldsize
         continue
 
     # it's not a directory
@@ -204,8 +209,8 @@ for (transname,ent) in r.filter(extra, wantrecurse=wantrecurse):
             ent.validate(int(mode, 8), id)
             ent.repack()
             shalists[-1].append((mode, file, id))
-    if exists and not hashvalid:
-        count += ent.size
+    if exists and ent.sha_missing():
+        count += oldsize
         subcount = 0
 
 
index d771592f0317f809fa1cf0f2fd69596feff10d2e..8aea5c4258f8175e41b416dc0854d57fa4d2cc7a 100644 (file)
@@ -9,8 +9,9 @@ ENTLEN = struct.calcsize(INDEX_SIG)
 FOOTER_SIG = '!Q'
 FOOTLEN = struct.calcsize(FOOTER_SIG)
 
-IX_EXISTS = 0x8000
-IX_HASHVALID = 0x4000
+IX_EXISTS = 0x8000        # file exists on filesystem
+IX_HASHVALID = 0x4000     # the stored sha1 matches the filesystem
+IX_SHAMISSING = 0x2000    # the stored sha1 object doesn't seem to exist
 
 class Error(Exception):
     pass
@@ -116,6 +117,9 @@ class Entry:
     def exists(self):
         return not self.is_deleted()
 
+    def sha_missing(self):
+        return (self.flags & IX_SHAMISSING) or not (self.flags & IX_HASHVALID)
+
     def is_deleted(self):
         return (self.flags & IX_EXISTS) == 0
 
@@ -167,6 +171,24 @@ class ExistingEntry(Entry):
          self.flags, self.children_ofs, self.children_n
          ) = struct.unpack(INDEX_SIG, str(buffer(m, ofs, ENTLEN)))
 
+    # effectively, we don't bother messing with IX_SHAMISSING if
+    # not IX_HASHVALID, since it's redundant, and repacking is more
+    # expensive than not repacking.
+    # This is implemented by having sha_missing() check IX_HASHVALID too.
+    def set_sha_missing(self, val):
+        val = val and 1 or 0
+        oldval = self.sha_missing() and 1 or 0
+        if val != oldval:
+            flag = val and IX_SHAMISSING or 0
+            newflags = (self.flags & (~IX_SHAMISSING)) | flag
+            self.flags = newflags
+            self.repack()
+
+    def unset_sha_missing(self, flag):
+        if self.flags & IX_SHAMISSING:
+            self.flags &= ~IX_SHAMISSING
+            self.repack()
+
     def repack(self):
         self._m[self._ofs:self._ofs+ENTLEN] = self.packed()
         if self.parent and not self.is_valid():