hashindex: test cleanup, decide for INT32_MAX as the refcount limit

Author: enkore

borg/hashindex.pyx

@@ -161,21 +161,17 @@ cdef class NSKeyIterator:
         cdef int32_t *value = <int32_t *>(self.key + self.key_size)
         return (<char *>self.key)[:self.key_size], (_le32toh(value[0]), _le32toh(value[1]))
 
-
-# to be safe from constants like EMPTY and DELETED
-# but don't decrease it any further, these *must* hold:
-# (1) max_ref > INT32_MAX/2
-# (2) max_ref > -INT32_MIN/2
-# If these don't hold the subtractions in the overflow checks themselves can overflow for values >max_ref
-cdef int32_t _MAX_REF = INT32_MAX
-# MAX_REF is visible to Python
-MAX_REF = _MAX_REF
-
 assert INT32_MAX == 2**31-1
-assert _MAX_REF > (<long long> INT32_MAX) / 2
-assert _MAX_REF > -(<long long> INT32_MIN) / 2
+
 
 cdef class ChunkIndex(IndexBase):
+    """
+    Mapping of 32 byte keys to (refcount, size, csize) (all 32 bit signed).
+
+    The reference count cannot overflow. If an overflow would occur, it is fixed to 2**31-1 and won't decrease
+    with decref() anymore.
+    """
+
     value_size = 12
 
     def __getitem__(self, key):
@@ -206,9 +202,9 @@ cdef class ChunkIndex(IndexBase):
         if not data:
             raise KeyError(key)
         cdef int32_t refcount = _le32toh(data[0])
-        if refcount != _MAX_REF:
-            if _MAX_REF - refcount < 1:
-                refcount = _MAX_REF
+        if refcount != INT32_MAX:
+            if INT32_MAX - refcount < 1:
+                refcount = INT32_MAX
             else:
                 refcount += 1
             data[0] = <int32_t> _htole32(refcount)
@@ -221,9 +217,9 @@ cdef class ChunkIndex(IndexBase):
         if not data:
             raise KeyError(key)
         cdef int32_t refcount = _le32toh(data[0])
-        if refcount != _MAX_REF:
-            if _MAX_REF - refcount < 1:
-                refcount = _MAX_REF
+        if refcount != INT32_MAX:
+            if INT32_MAX - refcount < 1:
+                refcount = INT32_MAX
             else:
                 refcount -= 1
             data[0] = _htole32(refcount)
@@ -274,10 +270,10 @@ cdef class ChunkIndex(IndexBase):
         if values:
             refcount1 = _le32toh(values[0])
             refcount2 = _le32toh(data[0])
-            if _MAX_REF - refcount1 < refcount2:
-                values[0] = _htole32(_MAX_REF)
-            elif _MAX_REF - refcount2 < refcount1:
-                values[0] = _htole32(_MAX_REF)
+            if INT32_MAX - refcount1 < refcount2:
+                values[0] = _htole32(INT32_MAX)
+            elif INT32_MAX - refcount2 < refcount1:
+                values[0] = _htole32(INT32_MAX)
             else:
                 values[0] = _htole32(refcount1 + refcount2)
         else:

borg/testsuite/hashindex.py

@@ -116,54 +116,55 @@ def test_chunkindex_summarize(self):
         assert chunks == 1 + 2 + 3
         assert unique_chunks == 3
 
+
+class HashIndexRefcountingTestCase(BaseTestCase):
+    MAX_REF = 2**31-1
+
     def test_chunkindex_overflow(self):
-        max_ref = hashindex.MAX_REF
         idx = ChunkIndex()
-        idx[H(1)] = max_ref - 1, 1, 2
-
-        refcount, *_ = idx.incref(H(1))
-        assert refcount == max_ref
+        idx[H(1)] = self.MAX_REF - 1, 1, 2
 
-        refcount, *_ = idx.decref(H(1))
-        assert refcount == max_ref
+        # 5 is arbitray, any number of incref/decrefs shouldn't move it once it maxes out
+        for i in range(5):
+            refcount, *_ = idx.incref(H(1))
+            assert refcount == self.MAX_REF
+        for i in range(5):
+            refcount, *_ = idx.decref(H(1))
+            assert refcount == self.MAX_REF
 
     def test_chunkindex_merge_overflow1(self):
-        # Check that it does not overflow at max_ref - 1
+        # Check that it does not overflow at 2**31-2
         idx1 = ChunkIndex()
-        half = int(hashindex.MAX_REF / 2)
-        inverse_parity = 1 - hashindex.MAX_REF % 2
-        # if max_ref is even, then the result is
-        # 2 * floor(max_ref/2) - 1
-        # ^^^^^^^^^^^^^^^^^^^^ = max_ref
-        # if max_ref is odd, then
-        # 2 * floor(max_ref/2) - 0
-        # ^^^^^^^^^^^^^^^^^^^^ = max_ref - 1
-        idx1[H(1)] = half - inverse_parity, 1, 2
+        # n.b. 2**31-1 is odd
+        half = self.MAX_REF // 2
+        idx1[H(1)] = half, 1, 2
         idx2 = ChunkIndex()
         idx2[H(1)] = half, 1, 2
         idx1.merge(idx2)
         refcount, *_ = idx1[H(1)]
-        assert refcount == hashindex.MAX_REF - 1
+        assert refcount == self.MAX_REF - 1
+        refcount, *_ = idx1.decref(H(1))
+        assert refcount == self.MAX_REF - 2
 
     def test_chunkindex_merge_overflow2(self):
-        # Check that it goes to max_ref even with values that are larged than max_ref themselves
         idx1 = ChunkIndex()
-        idx1[H(1)] = 147483647, 1, 2
+        idx1[H(1)] = 2000000000, 1, 2
         idx2 = ChunkIndex()
         idx2[H(1)] = 2000000000, 1, 2
         idx1.merge(idx2)
         refcount, *_ = idx1[H(1)]
-        assert refcount == hashindex.MAX_REF
+        assert refcount == self.MAX_REF
 
     def test_chunkindex_merge_overflow3(self):
+        # Check that merging maxes out right at 2**31-1
         idx1 = ChunkIndex()
-        half = hashindex.MAX_REF / 2
+        half = self.MAX_REF // 2
         idx1[H(1)] = half + 1, 1, 2
         idx2 = ChunkIndex()
         idx2[H(1)] = half, 1, 2
         idx1.merge(idx2)
         refcount, *_ = idx1[H(1)]
-        assert refcount == hashindex.MAX_REF
+        assert refcount == self.MAX_REF
 
     def test_chunkindex_add(self):
         idx1 = ChunkIndex()
@@ -178,16 +179,16 @@ def test_chunkindex_addinc(self):
         idx1.add(H(1), 2, 3, 4)
         assert idx1[H(1)] == (7, 6, 7)
 
-    def test_max_ref_transparent_inc(self):
+    def test_transparent_inc(self):
         idx1 = ChunkIndex()
-        idx1[H(1)] = (2**31-1, 6, 7)
+        idx1[H(1)] = (self.MAX_REF, 6, 7)
         idx1.incref(H(1))
         refcount, *_ = idx1[H(1)]
-        assert refcount == hashindex.MAX_REF
+        assert refcount == self.MAX_REF
 
-    def test_max_ref_transparent_dec(self):
+    def test_transparent_dec(self):
         idx1 = ChunkIndex()
-        idx1[H(1)] = (2**31-1, 6, 7)
+        idx1[H(1)] = (self.MAX_REF, 6, 7)
         idx1.decref(H(1))
         refcount, *_ = idx1[H(1)]
-        assert refcount == hashindex.MAX_REF
+        assert refcount == self.MAX_REF