)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"2a0cfa88250a4ae4b6c2916b7af84a764cbd0afe","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"As long as the reconstructor collects parts from all policies each job"},{"line_number":10,"context_line":"must be considered to have it\u0027s storage policy index and we can\u0027t use"},{"line_number":11,"context_line":"global state for policy specific headers.  It\u0027s good hygiene to avoid"},{"line_number":12,"context_line":"mutating the global state regardless."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Under load with multiple policies we observed essentially empty handoff"},{"line_number":15,"context_line":"parts \"re-appearing\" on nodes until adding these changes."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3f79a3b5_9e4a5a58","line":12,"range":{"start_line":11,"start_character":43,"end_line":12,"end_character":37},"updated":"2018-10-30 17:04:07.000000000","message":"++","commit_id":"931c398f5d447db827cf79463deaa4e84afacbe1"}],"test/unit/obj/test_reconstructor.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"2a0cfa88250a4ae4b6c2916b7af84a764cbd0afe","unresolved":false,"context_lines":[{"line_number":3425,"context_line":"        node \u003d job[\u0027sync_to\u0027][0]"},{"line_number":3426,"context_line":"        # process_job used to try and modify the instance base headers"},{"line_number":3427,"context_line":"        self.reconstructor.headers[\u0027X-Backend-Storage-Policy-Index\u0027] \u003d \\"},{"line_number":3428,"context_line":"            int(POLICIES[1])"},{"line_number":3429,"context_line":"        # ... which doesn\u0027t work out under concurrency with multiple policies"},{"line_number":3430,"context_line":"        self.assertNotEqual("},{"line_number":3431,"context_line":"            self.reconstructor.headers[\u0027X-Backend-Storage-Policy-Index\u0027],"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_7b443077","line":3428,"range":{"start_line":3428,"start_character":16,"end_line":3428,"end_character":28},"updated":"2018-10-30 17:04:07.000000000","message":"I think we maybe need to patch policies?\n\nERROR: test_get_suffixes_to_sync (test.unit.obj.test_reconstructor.TestObjectReconstructorECDuplicationFactor)\n----------------------------------------------------------------------\nTraceback (most recent call last):\n  File \"/vagrant/swift/test/unit/obj/test_reconstructor.py\", line 3428, in test_get_suffixes_to_sync\n    int(POLICIES[1])\n  File \"/vagrant/swift/swift/common/storage_policy.py\", line 954, in __getitem__\n    return _POLICIES[key]\n  File \"/vagrant/swift/swift/common/storage_policy.py\", line 743, in __getitem__\n    return self.by_index[key]\nKeyError: 1","commit_id":"931c398f5d447db827cf79463deaa4e84afacbe1"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"a356df0ea403b49fac6e50987c8547435acfe3a6","unresolved":false,"context_lines":[{"line_number":3407,"context_line":"            self.reconstructor.rehash_remote(node, job, [])"},{"line_number":3408,"context_line":"        self.assertEqual([int(job[\u0027policy\u0027])], ["},{"line_number":3409,"context_line":"            r[\u0027headers\u0027][\u0027X-Backend-Storage-Policy-Index\u0027]"},{"line_number":3410,"context_line":"            for r in request_log.requests])"},{"line_number":3411,"context_line":""},{"line_number":3412,"context_line":"    def test_get_suffixes_to_sync(self):"},{"line_number":3413,"context_line":"        part_path \u003d os.path.join(self.devices, self.local_dev[\u0027device\u0027],"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_d2066875","line":3410,"updated":"2018-10-31 19:33:31.000000000","message":"nit: Could maybe use a\n\n self.assertEqual(\n     self.reconstructor.headers[\u0027X-Backend-Storage-Policy-Index\u0027],\n     int(POLICIES[1]))\n\nto verify that process_job no longer modifies the base headers.","commit_id":"441df4fc930db7fc82d89b8c6a4d28d935554518"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5d83264ca225582616ffcd5b327c92420097b49d","unresolved":false,"context_lines":[{"line_number":3407,"context_line":"            self.reconstructor.rehash_remote(node, job, [])"},{"line_number":3408,"context_line":"        self.assertEqual([int(job[\u0027policy\u0027])], ["},{"line_number":3409,"context_line":"            r[\u0027headers\u0027][\u0027X-Backend-Storage-Policy-Index\u0027]"},{"line_number":3410,"context_line":"            for r in request_log.requests])"},{"line_number":3411,"context_line":""},{"line_number":3412,"context_line":"    def test_get_suffixes_to_sync(self):"},{"line_number":3413,"context_line":"        part_path \u003d os.path.join(self.devices, self.local_dev[\u0027device\u0027],"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_cdeb2d0f","line":3410,"in_reply_to":"3f79a3b5_d2066875","updated":"2018-10-31 20:12:57.000000000","message":"well, this test doesn\u0027t really call process_job - it just makes sure that rehash_remote isn\u0027t relying on self.headers for the policy id - in fact self.headers shouldn\u0027t really have the policy id in it.\n\nThe whole thing is a bit contrived.  But the change seemed sufficiently obvious we need not develop a sophisticated test to demonstrate the race with some concurrency.  It\u0027s possible these tests were too superficial to have even bothered.","commit_id":"441df4fc930db7fc82d89b8c6a4d28d935554518"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"a356df0ea403b49fac6e50987c8547435acfe3a6","unresolved":false,"context_lines":[{"line_number":4886,"context_line":"                                 ec_ndata\u003d10, ec_nparity\u003d4,"},{"line_number":4887,"context_line":"                                 ec_segment_size\u003d4096,"},{"line_number":4888,"context_line":"                                 ec_duplication_factor\u003d2),"},{"line_number":4889,"context_line":"                 StoragePolicy(1, name\u003d\u0027other\u0027)],"},{"line_number":4890,"context_line":"                fake_ring_args\u003d[{\u0027replicas\u0027: 28}, {\u0027replicas\u0027: 3}])"},{"line_number":4891,"context_line":"class TestObjectReconstructorECDuplicationFactor(TestObjectReconstructor):"},{"line_number":4892,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_92c9d053","line":4889,"range":{"start_line":4889,"start_character":17,"end_line":4889,"end_character":30},"updated":"2018-10-31 19:33:31.000000000","message":"So the bug is about \"Multiple EC policies\", but our tests always use one EC, one replicated...?","commit_id":"441df4fc930db7fc82d89b8c6a4d28d935554518"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5d83264ca225582616ffcd5b327c92420097b49d","unresolved":false,"context_lines":[{"line_number":4886,"context_line":"                                 ec_ndata\u003d10, ec_nparity\u003d4,"},{"line_number":4887,"context_line":"                                 ec_segment_size\u003d4096,"},{"line_number":4888,"context_line":"                                 ec_duplication_factor\u003d2),"},{"line_number":4889,"context_line":"                 StoragePolicy(1, name\u003d\u0027other\u0027)],"},{"line_number":4890,"context_line":"                fake_ring_args\u003d[{\u0027replicas\u0027: 28}, {\u0027replicas\u0027: 3}])"},{"line_number":4891,"context_line":"class TestObjectReconstructorECDuplicationFactor(TestObjectReconstructor):"},{"line_number":4892,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_4d663dbf","line":4889,"range":{"start_line":4889,"start_character":17,"end_line":4889,"end_character":30},"in_reply_to":"3f79a3b5_92c9d053","updated":"2018-10-31 20:12:57.000000000","message":"You know, I didn\u0027t think about it like that exactly - but I did try at first to make the second policy EC.\n\nThe test failures I observed seem to be related to the inherited tests asserting the reconstructor was ignoring the other policy (because in the default patched policies above it was also not EC).\n\nI made the judgement call about efficiency and effectiveness here.  I don\u0027t think anyone is advocating for a specific improvement in follow-up at this time.","commit_id":"441df4fc930db7fc82d89b8c6a4d28d935554518"}]}