)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"743d7eabb1ce30f98514c9a59bffce25deb8fb4b","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Tim Burke \u003ctim.burke@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2024-03-05 10:10:58 -0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"sq: raise DiskFileMetadataUnavailable more"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: Ia89aef5dbc06c49a34947e50ace2365c2c59ea62"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"c2626181_3dea6d47","line":7,"updated":"2024-05-13 12:48:09.000000000","message":"I think it would be worth spelling out the consequences of the change.\n\nIIUC the problem this addresses is further cases of the ondisk state changing while opening the files. We want to avoid ever returning a 404 when there *might* in fact always have been a data file present in the datadir.\n\nThe parent patch dealt with concurrent POSTs. \n\nDo we now catch all of those scenarios?\n\nBut the trade-off is some cases when we\u0027ll now return 503 when in fact a 404 was reasonable (e.g. .ts file disappears due to concurrent PUT).","commit_id":"c0c0eee84a8e54eaf6274a4975145323f385cab8"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"743d7eabb1ce30f98514c9a59bffce25deb8fb4b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c6b9d824_c9b05d80","updated":"2024-05-13 12:48:09.000000000","message":"I think I get the motivation. I\u0027d like us to capture all the whys and wherefores in the commit message/comments/tests to help future me remember.\n\nI wonder if the exception coming out of open should be generalised? IDK, I\u0027ll reflect on it some more.","commit_id":"c0c0eee84a8e54eaf6274a4975145323f385cab8"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"07a9d2857278026fd034a0d1fbefdb9a019e08c7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"0b7fcfef_25c4195d","updated":"2024-06-18 22:48:14.000000000","message":"Just one question: why 503 and not 409? We return 409 in other cases of such overlaps. To be fair, all the existing 409s in object-server are triggered by timestamp comparisons. But container server throws 409 on deletes if consistency checks fail. Looking at a black box from the outside, these cases seem similar to me. Do you have some kind of interaction with proxy that I\u0027m missing?","commit_id":"ac1acc2e8ebd80e9f4ca9ef232b253732f2847bc"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e5da9b21f7b0f9f828e0b5e5bb87b6407a7ffd38","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"50ebb15d_5b153df6","updated":"2024-05-17 16:54:42.000000000","message":"some comments I forgot to push","commit_id":"ac1acc2e8ebd80e9f4ca9ef232b253732f2847bc"}],"swift/obj/diskfile.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"743d7eabb1ce30f98514c9a59bffce25deb8fb4b","unresolved":true,"context_lines":[{"line_number":2589,"context_line":"            the object representation does not exist."},{"line_number":2590,"context_line":""},{"line_number":2591,"context_line":"        :raises DiskFileCollision: on name mis-match with metadata"},{"line_number":2592,"context_line":"        :raises DiskFileNotExist: if the object does not exist"},{"line_number":2593,"context_line":"        :raises DiskFileDeleted: if the object was previously deleted"},{"line_number":2594,"context_line":"        :raises DiskFileQuarantined: if while reading metadata of the file"},{"line_number":2595,"context_line":"                                     some data did pass cross checks"}],"source_content_type":"text/x-python","patch_set":1,"id":"f66e34d4_a4b6bcb0","line":2592,"updated":"2024-05-13 12:48:09.000000000","message":"IIUC this can now raise DiskFileMetadataUnavailable - but is DiskFileMetadataUnavailable the best named exception to raise from open? Maybe the data file was replaced? Should we translate to another new exception that reflects the \"uncertain state\" or \"something changed\" nature of these scenarios e.g. \"DiskFileTryAgain\"?","commit_id":"c0c0eee84a8e54eaf6274a4975145323f385cab8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e5da9b21f7b0f9f828e0b5e5bb87b6407a7ffd38","unresolved":false,"context_lines":[{"line_number":2589,"context_line":"            the object representation does not exist."},{"line_number":2590,"context_line":""},{"line_number":2591,"context_line":"        :raises DiskFileCollision: on name mis-match with metadata"},{"line_number":2592,"context_line":"        :raises DiskFileNotExist: if the object does not exist"},{"line_number":2593,"context_line":"        :raises DiskFileDeleted: if the object was previously deleted"},{"line_number":2594,"context_line":"        :raises DiskFileQuarantined: if while reading metadata of the file"},{"line_number":2595,"context_line":"                                     some data did pass cross checks"}],"source_content_type":"text/x-python","patch_set":1,"id":"926e4ad2_6da65d58","line":2592,"in_reply_to":"f66e34d4_a4b6bcb0","updated":"2024-05-17 16:54:42.000000000","message":"Done","commit_id":"c0c0eee84a8e54eaf6274a4975145323f385cab8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"743d7eabb1ce30f98514c9a59bffce25deb8fb4b","unresolved":true,"context_lines":[{"line_number":2885,"context_line":"                          format"},{"line_number":2886,"context_line":"        :returns: an opened data file pointer"},{"line_number":2887,"context_line":"        :raises DiskFileError: various exceptions from"},{"line_number":2888,"context_line":"                    :func:`swift.obj.diskfile.DiskFile._verify_data_file`"},{"line_number":2889,"context_line":"        \"\"\""},{"line_number":2890,"context_line":"        try:"},{"line_number":2891,"context_line":"            fp \u003d open(data_file, \u0027rb\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"a7c36e6f_48e6b162","line":2888,"updated":"2024-05-13 12:48:09.000000000","message":"needs :raises DiskFileMetadataUnavailable:","commit_id":"c0c0eee84a8e54eaf6274a4975145323f385cab8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e5da9b21f7b0f9f828e0b5e5bb87b6407a7ffd38","unresolved":false,"context_lines":[{"line_number":2885,"context_line":"                          format"},{"line_number":2886,"context_line":"        :returns: an opened data file pointer"},{"line_number":2887,"context_line":"        :raises DiskFileError: various exceptions from"},{"line_number":2888,"context_line":"                    :func:`swift.obj.diskfile.DiskFile._verify_data_file`"},{"line_number":2889,"context_line":"        \"\"\""},{"line_number":2890,"context_line":"        try:"},{"line_number":2891,"context_line":"            fp \u003d open(data_file, \u0027rb\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"4d33b944_2431fa43","line":2888,"in_reply_to":"a7c36e6f_48e6b162","updated":"2024-05-17 16:54:42.000000000","message":"Done","commit_id":"c0c0eee84a8e54eaf6274a4975145323f385cab8"}],"swift/obj/server.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"1fb14a4fc1cc110dc26953e74f11f3ac6e6594d3","unresolved":true,"context_lines":[{"line_number":1242,"context_line":"            # Bail out early because unread metadata may have an X-Delete-At."},{"line_number":1243,"context_line":"            # This condition can occur when a concurrent request has replaced"},{"line_number":1244,"context_line":"            # the data or meta file that this thread was trying to open. Force"},{"line_number":1245,"context_line":"            # client to retry, hoping the on-disk files have settled."},{"line_number":1246,"context_line":"            return HTTPServiceUnavailable(request\u003drequest)"},{"line_number":1247,"context_line":"        else:"},{"line_number":1248,"context_line":"            orig_timestamp \u003d disk_file.data_timestamp"}],"source_content_type":"text/x-python","patch_set":3,"id":"b8e2698e_10ad870b","line":1245,"updated":"2024-05-16 16:14:10.000000000","message":"This cleanup could go in the parent patch.","commit_id":"5159174f77e25b68d77d3588aa00e317425b4ed8"}],"test/unit/obj/test_diskfile.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"743d7eabb1ce30f98514c9a59bffce25deb8fb4b","unresolved":true,"context_lines":[{"line_number":860,"context_line":"            diskfile.write_metadata(fd, metadata)"},{"line_number":861,"context_line":"        check_metadata(as_native, str)"},{"line_number":862,"context_line":""},{"line_number":863,"context_line":"    def test_round_trip_metadata(self):"},{"line_number":864,"context_line":"        path \u003d os.path.join(self.testdir, str(uuid.uuid4()))"},{"line_number":865,"context_line":"        metadata \u003d {\u0027name\u0027: \u0027/a/c/o\u0027}"},{"line_number":866,"context_line":"        with open(path, \u0027wb\u0027) as fd:"}],"source_content_type":"text/x-python","patch_set":1,"id":"0e62e33f_8f78ee31","line":863,"updated":"2024-05-13 12:48:09.000000000","message":"the test above also does round-trip test, but this one uses a path as well as fd.\n\nmaybe test_write_read_metadata_using_path ??","commit_id":"c0c0eee84a8e54eaf6274a4975145323f385cab8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e5da9b21f7b0f9f828e0b5e5bb87b6407a7ffd38","unresolved":false,"context_lines":[{"line_number":860,"context_line":"            diskfile.write_metadata(fd, metadata)"},{"line_number":861,"context_line":"        check_metadata(as_native, str)"},{"line_number":862,"context_line":""},{"line_number":863,"context_line":"    def test_round_trip_metadata(self):"},{"line_number":864,"context_line":"        path \u003d os.path.join(self.testdir, str(uuid.uuid4()))"},{"line_number":865,"context_line":"        metadata \u003d {\u0027name\u0027: \u0027/a/c/o\u0027}"},{"line_number":866,"context_line":"        with open(path, \u0027wb\u0027) as fd:"}],"source_content_type":"text/x-python","patch_set":1,"id":"8ac2022e_59c71300","line":863,"in_reply_to":"0e62e33f_8f78ee31","updated":"2024-05-17 16:54:42.000000000","message":"Done","commit_id":"c0c0eee84a8e54eaf6274a4975145323f385cab8"}],"test/unit/obj/test_server.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"743d7eabb1ce30f98514c9a59bffce25deb8fb4b","unresolved":true,"context_lines":[{"line_number":8483,"context_line":"                headers\u003d{\u0027X-Timestamp\u0027: delete_timestamp})"},{"line_number":8484,"context_line":"            resp \u003d req.get_response(self.object_controller)"},{"line_number":8485,"context_line":"            self.assertNotIn(\u0027X-Backend-Timestamp\u0027, resp.headers)"},{"line_number":8486,"context_line":"            self.assertEqual(resp.status_int, 503)"},{"line_number":8487,"context_line":""},{"line_number":8488,"context_line":"        qdir \u003d os.path.join(self.testdir, \u0027sda1\u0027, \u0027quarantined\u0027)"},{"line_number":8489,"context_line":"        self.assertFalse(os.path.exists(qdir))"}],"source_content_type":"text/x-python","patch_set":1,"id":"14c9b279_156a5593","line":8486,"updated":"2024-05-13 12:48:09.000000000","message":"The race could equally be with a cleanup in which case a 404 would be reasonable when it\u0027s a tombstone that\u0027s gone missing (404 was a sensible state at some point in time)\n\nMight be worth  \"documenting\" that race with a test:\n\n```\ndiff --git a/test/unit/obj/test_server.py b/test/unit/obj/test_server.py\nindex 6c24cf156..14bed33b9 100644\n--- a/test/unit/obj/test_server.py\n+++ b/test/unit/obj/test_server.py\n@@ -8724,7 +8724,41 @@ class TestObjectController(BaseTestCase):\n             self.assertEqual(resp.status_int, 503)\n             self.assertFalse(os.path.isdir(object_dir))\n\n-    def test_race_doesnt_quarantine(self):\n+    def test_DELETE_race_with_cleanup_doesnt_quarantine(self):\n+        existing_timestamp \u003d normalize_timestamp(time())\n+        delete_timestamp \u003d normalize_timestamp(time() + 1)\n+\n+        # make a .ts\n+        req \u003d Request.blank(\n+            \u0027/sda1/p/a/c/o\u0027, environ\u003d{\u0027REQUEST_METHOD\u0027: \u0027DELETE\u0027},\n+            headers\u003d{\u0027X-Timestamp\u0027: existing_timestamp})\n+        req.get_response(self.object_controller)\n+        objfile \u003d self.df_mgr.get_diskfile(\u0027sda1\u0027, \u0027p\u0027, \u0027a\u0027, \u0027c\u0027, \u0027o\u0027,\n+                                           policy\u003dPOLICIES.legacy)\n+\n+        # force a cleanup between the listdir and read_metadata of a DELETE\n+        list_once \u003d [False]\n+        orig_listdir \u003d os.listdir\n+\n+        def mock_listdir(path):\n+            listing \u003d orig_listdir(path)\n+            if not list_once[0]:\n+                list_once[0] \u003d True\n+                rmtree(objfile._datadir)\n+            return listing\n+\n+        with mock.patch(\u0027os.listdir\u0027, mock_listdir):\n+            req \u003d Request.blank(\n+                \u0027/sda1/p/a/c/o\u0027, environ\u003d{\u0027REQUEST_METHOD\u0027: \u0027DELETE\u0027},\n+                headers\u003d{\u0027X-Timestamp\u0027: delete_timestamp})\n+            resp \u003d req.get_response(self.object_controller)\n+            self.assertNotIn(\u0027X-Backend-Timestamp\u0027, resp.headers)\n+            self.assertEqual(resp.status_int, 503)\n+\n+        qdir \u003d os.path.join(self.testdir, \u0027sda1\u0027, \u0027quarantined\u0027)\n+        self.assertFalse(os.path.exists(qdir))\n+\n+    def test_DELETE_race_with_PUT_doesnt_quarantine(self):\n         existing_timestamp \u003d normalize_timestamp(time())\n         delete_timestamp \u003d normalize_timestamp(time() + 1)\n         put_timestamp \u003d normalize_timestamp(time() + 2)\n         ```","commit_id":"c0c0eee84a8e54eaf6274a4975145323f385cab8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e5da9b21f7b0f9f828e0b5e5bb87b6407a7ffd38","unresolved":false,"context_lines":[{"line_number":8483,"context_line":"                headers\u003d{\u0027X-Timestamp\u0027: delete_timestamp})"},{"line_number":8484,"context_line":"            resp \u003d req.get_response(self.object_controller)"},{"line_number":8485,"context_line":"            self.assertNotIn(\u0027X-Backend-Timestamp\u0027, resp.headers)"},{"line_number":8486,"context_line":"            self.assertEqual(resp.status_int, 503)"},{"line_number":8487,"context_line":""},{"line_number":8488,"context_line":"        qdir \u003d os.path.join(self.testdir, \u0027sda1\u0027, \u0027quarantined\u0027)"},{"line_number":8489,"context_line":"        self.assertFalse(os.path.exists(qdir))"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba33708b_fe238474","line":8486,"in_reply_to":"14c9b279_156a5593","updated":"2024-05-17 16:54:42.000000000","message":"Acknowledged","commit_id":"c0c0eee84a8e54eaf6274a4975145323f385cab8"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"5a9de0fe9fd436b5ef57b62e98dcb4a725c7a907","unresolved":true,"context_lines":[{"line_number":8533,"context_line":"                headers\u003d{\u0027X-Timestamp\u0027: post_timestamp})"},{"line_number":8534,"context_line":"            resp \u003d req.get_response(self.object_controller)"},{"line_number":8535,"context_line":"            self.assertNotIn(\u0027X-Backend-Timestamp\u0027, resp.headers)"},{"line_number":8536,"context_line":"            self.assertEqual(resp.status_int, 503)"},{"line_number":8537,"context_line":""},{"line_number":8538,"context_line":"        qdir \u003d os.path.join(self.testdir, \u0027sda1\u0027, \u0027quarantined\u0027)"},{"line_number":8539,"context_line":"        self.assertFalse(os.path.exists(qdir))"}],"source_content_type":"text/x-python","patch_set":1,"id":"1ddf93a5_2449a021","line":8536,"range":{"start_line":8536,"start_character":46,"end_line":8536,"end_character":49},"updated":"2024-03-06 20:22:37.000000000","message":"This currently 404s, both on master and the parent patch. If the POST went through slightly earlier, it would 202; slightly after and it would 409. 404 is just plain **wrong**.","commit_id":"c0c0eee84a8e54eaf6274a4975145323f385cab8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"743d7eabb1ce30f98514c9a59bffce25deb8fb4b","unresolved":true,"context_lines":[{"line_number":8533,"context_line":"                headers\u003d{\u0027X-Timestamp\u0027: post_timestamp})"},{"line_number":8534,"context_line":"            resp \u003d req.get_response(self.object_controller)"},{"line_number":8535,"context_line":"            self.assertNotIn(\u0027X-Backend-Timestamp\u0027, resp.headers)"},{"line_number":8536,"context_line":"            self.assertEqual(resp.status_int, 503)"},{"line_number":8537,"context_line":""},{"line_number":8538,"context_line":"        qdir \u003d os.path.join(self.testdir, \u0027sda1\u0027, \u0027quarantined\u0027)"},{"line_number":8539,"context_line":"        self.assertFalse(os.path.exists(qdir))"}],"source_content_type":"text/x-python","patch_set":1,"id":"7d5fa2d4_4bdc29cd","line":8536,"range":{"start_line":8536,"start_character":46,"end_line":8536,"end_character":49},"in_reply_to":"1ddf93a5_2449a021","updated":"2024-05-13 12:48:09.000000000","message":"ok so 404 was never the correct state at any point in time here, similar to the POST vs POST race","commit_id":"c0c0eee84a8e54eaf6274a4975145323f385cab8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e5da9b21f7b0f9f828e0b5e5bb87b6407a7ffd38","unresolved":false,"context_lines":[{"line_number":8533,"context_line":"                headers\u003d{\u0027X-Timestamp\u0027: post_timestamp})"},{"line_number":8534,"context_line":"            resp \u003d req.get_response(self.object_controller)"},{"line_number":8535,"context_line":"            self.assertNotIn(\u0027X-Backend-Timestamp\u0027, resp.headers)"},{"line_number":8536,"context_line":"            self.assertEqual(resp.status_int, 503)"},{"line_number":8537,"context_line":""},{"line_number":8538,"context_line":"        qdir \u003d os.path.join(self.testdir, \u0027sda1\u0027, \u0027quarantined\u0027)"},{"line_number":8539,"context_line":"        self.assertFalse(os.path.exists(qdir))"}],"source_content_type":"text/x-python","patch_set":1,"id":"f827ee91_250885ce","line":8536,"range":{"start_line":8536,"start_character":46,"end_line":8536,"end_character":49},"in_reply_to":"7d5fa2d4_4bdc29cd","updated":"2024-05-17 16:54:42.000000000","message":"Acknowledged","commit_id":"c0c0eee84a8e54eaf6274a4975145323f385cab8"}]}