From 092d409c4bbe542f6b9f3c217aa05ffd8669c815 Mon Sep 17 00:00:00 2001
From: Alistair Coles <alistairncoles@gmail.com>
Date: Fri, 19 Nov 2021 12:55:59 +0000
Subject: [PATCH] reconstructor: silence traceback when purging

Catch DiskFileNotExist exceptions when attempting to purge files. The
file may have passed its reclaim age since being reverted and will be
cleaned up when the reconstructor opens it for purging, raising a
DiskFileNotExist. The exception is OK - the diskfile was about to be
purged.

Change-Id: I5dfdf5950c6bd7fb130ab557347fbe959270c6e9
---
 swift/obj/reconstructor.py          |  6 +++-
 test/unit/obj/test_reconstructor.py | 56 +++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/swift/obj/reconstructor.py b/swift/obj/reconstructor.py
index 4b444cd44a..cc4d42f6f8 100644
--- a/swift/obj/reconstructor.py
+++ b/swift/obj/reconstructor.py
@@ -47,7 +47,7 @@ from swift.obj.diskfile import DiskFileRouter, get_data_dir, \
     get_tmp_dir, DEFAULT_RECLAIM_AGE
 from swift.common.storage_policy import POLICIES, EC_POLICY
 from swift.common.exceptions import ConnectionTimeout, DiskFileError, \
-    SuffixSyncError, PartitionLockTimeout
+    SuffixSyncError, PartitionLockTimeout, DiskFileNotExist
 
 SYNC, REVERT = ('sync_only', 'sync_revert')
 UNKNOWN_RESPONSE_STATUS = 0  # used as response status for timeouts, exceptions
@@ -988,6 +988,10 @@ class ObjectReconstructor(Daemon):
                                           else df_mgr.commit_window)
                 df.purge(timestamps['ts_data'], frag_index,
                          nondurable_purge_delay)
+            except DiskFileNotExist:
+                # may have passed reclaim age since being reverted, or may have
+                # raced with another reconstructor process trying the same
+                pass
             except DiskFileError:
                 self.logger.exception(
                     'Unable to purge DiskFile (%r %r %r)',
diff --git a/test/unit/obj/test_reconstructor.py b/test/unit/obj/test_reconstructor.py
index 3a7501e1fc..1035f74169 100644
--- a/test/unit/obj/test_reconstructor.py
+++ b/test/unit/obj/test_reconstructor.py
@@ -4666,6 +4666,62 @@ class TestObjectReconstructor(BaseTestObjectReconstructor):
 
         self.assertEqual(self.reconstructor.handoffs_remaining, 0)
 
+    def test_process_job_revert_cleanup_but_already_reclaimed(self):
+        frag_index = random.randint(
+            0, self.policy.ec_n_unique_fragments - 1)
+        sync_to = [random.choice([n for n in self.policy.object_ring.devs
+                                  if n != self.local_dev])]
+        sync_to[0]['index'] = frag_index
+        partition = 0
+
+        part_path = os.path.join(self.devices, self.local_dev['device'],
+                                 diskfile.get_data_dir(self.policy),
+                                 str(partition))
+        os.makedirs(part_path)
+        df_mgr = self.reconstructor._df_router[self.policy]
+        df = df_mgr.get_diskfile(self.local_dev['device'], partition, 'a',
+                                 'c', 'data-obj', policy=self.policy)
+        ts_delete = self.ts()
+        df.delete(ts_delete)
+        ohash = os.path.basename(df._datadir)
+        suffix = os.path.basename(os.path.dirname(df._datadir))
+
+        job = {
+            'job_type': object_reconstructor.REVERT,
+            'frag_index': frag_index,
+            'suffixes': [suffix],
+            'sync_to': sync_to,
+            'partition': partition,
+            'path': part_path,
+            'hashes': {},
+            'policy': self.policy,
+            'local_dev': self.local_dev,
+            'device': self.local_dev['device'],
+        }
+
+        fake_time = [float(ts_delete) + df_mgr.reclaim_age - 100]
+
+        def mock_time():
+            return fake_time[0]
+
+        def ssync_response_callback(*args):
+            # pretend ssync completed and time has moved just beyonf the
+            # reclaim age for the tombstone
+            fake_time[0] = float(ts_delete) + df_mgr.reclaim_age + 1
+            return True, {ohash: {'ts_data': ts_delete}}
+
+        ssync_calls = []
+        with mock.patch('swift.obj.diskfile.time.time', mock_time):
+            with mock_ssync_sender(ssync_calls,
+                                   response_callback=ssync_response_callback):
+                self.reconstructor.process_job(job)
+
+        self.assertFalse(os.path.exists(df._datadir))
+        self.assertEqual(self.reconstructor.handoffs_remaining, 0)
+        # check there's no tracebacks for opening the reclaimed tombstone
+        self.assertEqual(
+            [], self.reconstructor.logger.logger.get_lines_for_level('error'))
+
     def test_process_job_revert_cleanup_tombstone(self):
         partition = 0
         sync_to = [random.choice([