relinker did not cleanup tombstones, causes EEXIST errors

[Alistair Coles]

With this change https://review.opendev.org/c/openstack/swift/+/778773 the relinker ceased to cleanup tombstones unless they were old enough to be reapable.

This unit test illustrates (patch to commit b8aefd750)

diff --git a/test/unit/cli/test_relinker.py b/test/unit/cli/test_relinker.py
index cd438d1e8..8f087af74 100644
--- a/test/unit/cli/test_relinker.py
+++ b/test/unit/cli/test_relinker.py
@@ -943,6 +943,7 @@ class TestRelinker(unittest.TestCase):
             '--devices', self.devices,
             '--skip-mount',
         ]))
+ self.assertFalse(os.path.exists(self.objname))

     def test_cleanup_reapable(self):
         # relink a tombstone
@@ -973,6 +974,32 @@ class TestRelinker(unittest.TestCase):
         # want self.objname around polluting the old partition space.
         self.assertFalse(os.path.exists(self.objname))

+ def test_cleanup_not_reapable(self):
+ # relink a tombstone
+ fname_ts = self.objname[:-4] + "ts"
+ os.rename(self.objname, fname_ts)
+ self.objname = fname_ts
+ self.expected_file = self.expected_file[:-4] + "ts"
+ self._common_test_cleanup()
+ self.assertTrue(os.path.exists(self.expected_file)) # sanity check
+
+ with mock.patch.object(relinker.logging, 'getLogger',
+ return_value=self.logger):
+ self.assertEqual(0, relinker.main([
+ 'cleanup',
+ '--swift-dir', self.testdir,
+ '--devices', self.devices,
+ '--skip-mount',
+ ]))
+ self.assertEqual(self.logger.get_lines_for_level('error'), [])
+ self.assertEqual(self.logger.get_lines_for_level('warning'), [])
+
+ # we do want the tombstone in the new partition space
+ self.assertTrue(os.path.exists(self.expected_file))
+ # we definitely *don't* want self.objname around polluting the old
+ # partition space.
+ self.assertFalse(os.path.exists(self.objname))
+
     def test_cleanup_doesnotexist(self):
         self._common_test_cleanup()

This was fixed by https://review.opendev.org/c/openstack/swift/+/779308, but any partition power increase conducted between those commits would have left stale tombstones in the old part power locations.

This then results in error logs when conducting a subsequent partition power increase, due to the scenarios below, such as:

"Relinking path/to/hash_dir/1614305112.47630.ts failed: [Errno 17] File exists"

(notation in following: "PN location" refers to "the expected location an object when partition power is N")

During the previous part-power increase (for example from 16 to 17), some tombstones were not cleaned up from their P16 location. The same tombstones did have links created in their P17 locations.

After the completion of the 16->17 part-power increase, the misplaced tombstones from P16 locations were replicated to devices on which there were also tombstones in P17 locations. The tombstones in P16 would also exist elsewhere but the error logs would only manifest for devices on which both the P16 and P17 partitions are placed.

Because they were replicated, the P16 tombstones have different inodes than the P17 tombstones on the same device.

During the next part power increase (in tis example, from 17 to 18), the relinker visits partitions on a device in reverse order, so finds the P17 tombstones first and links them to their new P18 locations. For example a tombstone in part 96000 would be linked to a tombstone in part 192000.

Assume the P17 partition is < 2^16, so that the P18 partition IS NOT rehashed after relinking.

The relinker moves on to find the P16 tombstones, and attempts to relink those to their P18 locations. For example a tombstone in part 48000 would be linked to part 192000. However, the P18 location already has a link to the P17 location, so the attempt to link to the P16 location fails.

The inode linked from P18 is different from the inode in P16, so the failure to relink P16 to P18 is logged as an EEXIST error.

Inspection of the device after the first pass of the relinker will show that there is a tombstone in all three locations P16, P17 and P18, with P17 and P18 having the same inode.

The required files are intact, but the same error will be logged if the relinker makes further passes over the same device.

[Alistair Coles (alistair-coles)]

https://review.opendev.org/c/openstack/swift/+/783467

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/swift 2.28.0 release.