)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c9fa42a540636346988cb26f1aaf76f81c4eaef3","unresolved":false,"context_lines":[{"line_number":25,"context_line":"*not* invalid.  This is a requirement for rsync replication because"},{"line_number":26,"context_line":"after a suffix is synced via rsync the contents of a suffix dir will"},{"line_number":27,"context_line":"likely have changed and the remote server needs to update it hashes.pkl"},{"line_number":28,"context_line":"to reflect the new data."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"SSYNC does not *need* to send a post-sync REPLICATE request.  Any"},{"line_number":31,"context_line":"suffixes that are modified by the SSYNC protocol will call _finalize_put"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"bff0334d_cba9fddf","line":28,"updated":"2017-04-06 07:48:45.000000000","message":"un-related history lesson","commit_id":"77e2ab326128d19d369f4f0bce84d9c01ca8cef6"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c9fa42a540636346988cb26f1aaf76f81c4eaef3","unresolved":false,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":"If a ssync_sender does not successful finish a sync the reconstructor"},{"line_number":43,"context_line":"should skip the REPLICATE call entirely and move on to the next"},{"line_number":44,"context_line":"partition without causing any useless remote IO."},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"Closes-Bug: #1665141"},{"line_number":47,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"bff0334d_2b84e95d","line":44,"updated":"2017-04-06 07:48:45.000000000","message":"+1 thanks past me!","commit_id":"77e2ab326128d19d369f4f0bce84d9c01ca8cef6"}],"swift/obj/reconstructor.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e0d732d099e9eccffa5f19acb301e5b5e39d373d","unresolved":false,"context_lines":[{"line_number":707,"context_line":"            success, _ \u003d ssync_sender("},{"line_number":708,"context_line":"                self, node, job, suffixes)()"},{"line_number":709,"context_line":"            # let remote end know to rehash it\u0027s suffixes"},{"line_number":710,"context_line":"            self.rehash_remote(node, job, suffixes)"},{"line_number":711,"context_line":"            # update stats for this attempt"},{"line_number":712,"context_line":"            self.suffix_sync +\u003d len(suffixes)"},{"line_number":713,"context_line":"            self.logger.update_stats(\u0027suffix.syncs\u0027, len(suffixes))"}],"source_content_type":"text/x-python","patch_set":5,"id":"bff0334d_08759995","line":710,"updated":"2017-04-06 16:36:41.000000000","message":"doesn\u0027t same logic apply here? we don\u0027t strictly need to do this and might be better  to be conditional upon success?","commit_id":"77e2ab326128d19d369f4f0bce84d9c01ca8cef6"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c9fa42a540636346988cb26f1aaf76f81c4eaef3","unresolved":false,"context_lines":[{"line_number":727,"context_line":"            success, in_sync_objs \u003d ssync_sender("},{"line_number":728,"context_line":"                self, node, job, job[\u0027suffixes\u0027])()"},{"line_number":729,"context_line":"            if success:"},{"line_number":730,"context_line":"                self.rehash_remote(node, job, job[\u0027suffixes\u0027])"},{"line_number":731,"context_line":"                syncd_with +\u003d 1"},{"line_number":732,"context_line":"                reverted_objs.update(in_sync_objs)"},{"line_number":733,"context_line":"        if syncd_with \u003e\u003d len(job[\u0027sync_to\u0027]):"}],"source_content_type":"text/x-python","patch_set":5,"id":"bff0334d_cb5b3d2c","line":730,"updated":"2017-04-06 07:48:45.000000000","message":"test sillyness aside, this is pretty straight forward and IMHO obviously correct change for a non trivial performance enhancement.  But it\u0027s hard to quantify just how much it helps unless you can get a cluster with dense parts and enough filesystem metadata to assert the memory pressure to make rehash painful.","commit_id":"77e2ab326128d19d369f4f0bce84d9c01ca8cef6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e0d732d099e9eccffa5f19acb301e5b5e39d373d","unresolved":false,"context_lines":[{"line_number":727,"context_line":"            success, in_sync_objs \u003d ssync_sender("},{"line_number":728,"context_line":"                self, node, job, job[\u0027suffixes\u0027])()"},{"line_number":729,"context_line":"            if success:"},{"line_number":730,"context_line":"                self.rehash_remote(node, job, job[\u0027suffixes\u0027])"},{"line_number":731,"context_line":"                syncd_with +\u003d 1"},{"line_number":732,"context_line":"                reverted_objs.update(in_sync_objs)"},{"line_number":733,"context_line":"        if syncd_with \u003e\u003d len(job[\u0027sync_to\u0027]):"}],"source_content_type":"text/x-python","patch_set":5,"id":"bff0334d_48528115","line":730,"in_reply_to":"bff0334d_cb5b3d2c","updated":"2017-04-06 16:36:41.000000000","message":"+1. \nAs I understand this call to rehash_remote is not strictly necessary because each ssync subrequest will have caused a hash invalidation on remote. But assuming ssync was successful and remote has inodes in cache then it\u0027s also not hugely expensive, possibly less of a hit than when the remote needs to rehash for some other reason.\n\nFrustratingly ssync cannot report partial success, so depsite the fact that we sync suffixes in order there is no way of knowing whether some suffixes were successfully sync\u0027d and therefore worth rehashing tha subset on remote.","commit_id":"77e2ab326128d19d369f4f0bce84d9c01ca8cef6"}],"test/unit/__init__.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"4e7d6fd58cb368eb29c01ec6e947199b5f64fb96","unresolved":false,"context_lines":[{"line_number":1025,"context_line":"        try:"},{"line_number":1026,"context_line":"            i, status \u003d next(conn_id_and_code_iter)"},{"line_number":1027,"context_line":"        except StopIteration:"},{"line_number":1028,"context_line":"            # the code under test may swallow the StopIteration, so by logging"},{"line_number":1029,"context_line":"            # unexpected requests here we allow the test framework to check for"},{"line_number":1030,"context_line":"            # them after the connect function has been used."},{"line_number":1031,"context_line":"            unexpected_requests.append((args, kwargs))"}],"source_content_type":"text/x-python","patch_set":6,"id":"5ff73747_f6863866","line":1028,"updated":"2017-04-28 21:23:14.000000000","message":"There\u0027s *no reason* that the real connect() should ever raise a StopIteration, though, yeah? Would it be better to raise like a socket.error or something?\n\nFor similar reasons, should we worry about the next() calls below (for etag, headers, expect_headers, timestamp, body) raising StopIteration and causing unexpected test behavior?\n\nWould something like http://paste.openstack.org/show/608369/ be better?","commit_id":"a0fcca1e0576a0dba7a61f05c86aba23d6ddd27f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"ee8c631c50d452e8091f082e5d82b761c6e1ba82","unresolved":false,"context_lines":[{"line_number":1025,"context_line":"        try:"},{"line_number":1026,"context_line":"            i, status \u003d next(conn_id_and_code_iter)"},{"line_number":1027,"context_line":"        except StopIteration:"},{"line_number":1028,"context_line":"            # the code under test may swallow the StopIteration, so by logging"},{"line_number":1029,"context_line":"            # unexpected requests here we allow the test framework to check for"},{"line_number":1030,"context_line":"            # them after the connect function has been used."},{"line_number":1031,"context_line":"            unexpected_requests.append((args, kwargs))"}],"source_content_type":"text/x-python","patch_set":6,"id":"5ff73747_12be3efe","line":1028,"in_reply_to":"5ff73747_f6863866","updated":"2017-05-01 10:07:01.000000000","message":"It might be that raising something other than a StopIteration makes sense, but the point here is that we do not expect to see the request occur - not \u0027if it occurs it should fail\u0027 but \u0027we really want to know these requests are never attempted\u0027. So I like getting an explicit indication that an unexpected request did occur. Otherwise the test (in this patch) would have to rely on whatever exception was raised breaking things enough that one of the test assertions would fail.\n\nTrying your diff, and applying the regression as before, the test still fails intermittently.\n\nPerhaps we should not *raise* anything before calling the give_conn_fn at line 1039, then raise some exception after. That way we could make assertions about how many calls were made to give_conn_fn. I didn\u0027t try that, but I fear any knock on to other tests. Maybe it would be fine.\n\nI feel there is a trade off between \u0027clean up the test harness, fixing dodgy tests as we do\u0027 vs \u0027land this patch\u0027. You\u0027re right about the etag, header etc iters - but they are  not used in this patch, so catching StopIteration around those could be seen as \u0027noise\u0027 in this patch.","commit_id":"a0fcca1e0576a0dba7a61f05c86aba23d6ddd27f"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ec961e7c2da8a86dc7d71bf5f8857cfbc84bb18b","unresolved":false,"context_lines":[{"line_number":1053,"context_line":"                        connection_id\u003di, give_send\u003dkwargs.get(\u0027give_send\u0027),"},{"line_number":1054,"context_line":"                        give_expect\u003dkwargs.get(\u0027give_expect\u0027))"},{"line_number":1055,"context_line":""},{"line_number":1056,"context_line":"    connect.unexpected_requests \u003d unexpected_requests"},{"line_number":1057,"context_line":"    connect.code_iter \u003d code_iter"},{"line_number":1058,"context_line":""},{"line_number":1059,"context_line":"    return connect"}],"source_content_type":"text/x-python","patch_set":6,"id":"bff0334d_e129649b","line":1056,"updated":"2017-04-08 18:49:47.000000000","message":"this wfm","commit_id":"a0fcca1e0576a0dba7a61f05c86aba23d6ddd27f"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"4e7d6fd58cb368eb29c01ec6e947199b5f64fb96","unresolved":false,"context_lines":[{"line_number":1083,"context_line":"        left_over_status \u003d list(fake_conn.code_iter)"},{"line_number":1084,"context_line":"        if left_over_status:"},{"line_number":1085,"context_line":"            raise AssertionError(\u0027left over status %r\u0027 % left_over_status)"},{"line_number":1086,"context_line":""},{"line_number":1087,"context_line":""},{"line_number":1088,"context_line":"def make_timestamp_iter():"},{"line_number":1089,"context_line":"    return iter(Timestamp(t) for t in itertools.count(int(time.time())))"}],"source_content_type":"text/x-python","patch_set":6,"id":"5ff73747_13341749","line":1086,"updated":"2017-04-28 21:23:14.000000000","message":"I feel like this should include a\n\n if fake_conn.unexpected_requests:\n     raise AssertionError(\u0027Extra requests %r\u0027 % fake_conn.unexpected_requests)\n\nor something.","commit_id":"a0fcca1e0576a0dba7a61f05c86aba23d6ddd27f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"ee8c631c50d452e8091f082e5d82b761c6e1ba82","unresolved":false,"context_lines":[{"line_number":1083,"context_line":"        left_over_status \u003d list(fake_conn.code_iter)"},{"line_number":1084,"context_line":"        if left_over_status:"},{"line_number":1085,"context_line":"            raise AssertionError(\u0027left over status %r\u0027 % left_over_status)"},{"line_number":1086,"context_line":""},{"line_number":1087,"context_line":""},{"line_number":1088,"context_line":"def make_timestamp_iter():"},{"line_number":1089,"context_line":"    return iter(Timestamp(t) for t in itertools.count(int(time.time())))"}],"source_content_type":"text/x-python","patch_set":6,"id":"5ff73747_12b75e3c","line":1086,"in_reply_to":"5ff73747_13341749","updated":"2017-05-01 10:07:01.000000000","message":"Probably but then we\u0027d have to fix the other tests that fail :/\n\nsee comment here https://review.openstack.org/#/c/435152/6/test/unit/obj/test_reconstructor.py@1040\n\nAnd we probably *should* look at those failing tests, but not in this patch? So I avoided the issue by making the assertions in the test case itself .","commit_id":"a0fcca1e0576a0dba7a61f05c86aba23d6ddd27f"}],"test/unit/obj/test_reconstructor.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c9fa42a540636346988cb26f1aaf76f81c4eaef3","unresolved":false,"context_lines":[{"line_number":923,"context_line":"                ssync_calls.append(context)"},{"line_number":924,"context_line":"                self.success \u003d True"},{"line_number":925,"context_line":"                for failure in (fail_jobs or []):"},{"line_number":926,"context_line":"                    if all(job.get(k) \u003d\u003d v for (k, v) in failure.items()):"},{"line_number":927,"context_line":"                        self.success \u003d False"},{"line_number":928,"context_line":"                        break"},{"line_number":929,"context_line":"                context[\u0027success\u0027] \u003d self.success"}],"source_content_type":"text/x-python","patch_set":5,"id":"bff0334d_507d98af","line":926,"updated":"2017-04-06 07:48:45.000000000","message":"this is pretty f\u0027ing weird - but i guess _make_fake_ssync looks like it will continue to be re-used and extended.  Even tho I only had he requirement of failing two jobs in the same device/part - i over-engineered this interface to support more complex failures.\n\nOne could argue that job[k] would be more fail loud and early.  If I try to match on the key \u0027part\u0027 instead of \u0027partition\u0027 instead of a KeyError it my fail description just doesn\u0027t match.\n\n/me shurgs","commit_id":"77e2ab326128d19d369f4f0bce84d9c01ca8cef6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e0d732d099e9eccffa5f19acb301e5b5e39d373d","unresolved":false,"context_lines":[{"line_number":923,"context_line":"                ssync_calls.append(context)"},{"line_number":924,"context_line":"                self.success \u003d True"},{"line_number":925,"context_line":"                for failure in (fail_jobs or []):"},{"line_number":926,"context_line":"                    if all(job.get(k) \u003d\u003d v for (k, v) in failure.items()):"},{"line_number":927,"context_line":"                        self.success \u003d False"},{"line_number":928,"context_line":"                        break"},{"line_number":929,"context_line":"                context[\u0027success\u0027] \u003d self.success"}],"source_content_type":"text/x-python","patch_set":5,"id":"bff0334d_e9eef486","line":926,"in_reply_to":"bff0334d_507d98af","updated":"2017-04-06 16:36:41.000000000","message":"OIC so fail_jobs can select subsets of jobs based on key matches.","commit_id":"77e2ab326128d19d369f4f0bce84d9c01ca8cef6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e0d732d099e9eccffa5f19acb301e5b5e39d373d","unresolved":false,"context_lines":[{"line_number":929,"context_line":"                context[\u0027success\u0027] \u003d self.success"},{"line_number":930,"context_line":""},{"line_number":931,"context_line":"            def __call__(self, *args, **kwargs):"},{"line_number":932,"context_line":"                return self.success, self.available_map"},{"line_number":933,"context_line":""},{"line_number":934,"context_line":"        return _fake_ssync"},{"line_number":935,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"bff0334d_487e01e5","line":932,"updated":"2017-04-06 16:36:41.000000000","message":"nit: a real SssyncSender will always return an empty available_map when success\u003dFalse","commit_id":"77e2ab326128d19d369f4f0bce84d9c01ca8cef6"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c9fa42a540636346988cb26f1aaf76f81c4eaef3","unresolved":false,"context_lines":[{"line_number":995,"context_line":"        captured_ssync \u003d []"},{"line_number":996,"context_line":"        # fail all jobs on part 2 on sda1"},{"line_number":997,"context_line":"        fail_jobs \u003d ["},{"line_number":998,"context_line":"            {\u0027device\u0027: \u0027sda1\u0027, \u0027partition\u0027: 2},"},{"line_number":999,"context_line":"        ]"},{"line_number":1000,"context_line":"        with mock.patch(\u0027swift.obj.reconstructor.ssync_sender\u0027,"},{"line_number":1001,"context_line":"                        self._make_fake_ssync("}],"source_content_type":"text/x-python","patch_set":5,"id":"bff0334d_301a147d","line":998,"updated":"2017-04-06 07:48:45.000000000","message":"It might have been more demonstrative of the interface to fail say:\n\n        fail_jobs \u003d [\n            {\u0027device\u0027: \u0027sda1\u0027, \u0027frag_index\u0027: 1},\n            {\u0027device\u0027: \u0027sda1\u0027, \u0027partition\u0027: 0},\n        ]\n\n/me shrugs","commit_id":"77e2ab326128d19d369f4f0bce84d9c01ca8cef6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e0d732d099e9eccffa5f19acb301e5b5e39d373d","unresolved":false,"context_lines":[{"line_number":995,"context_line":"        captured_ssync \u003d []"},{"line_number":996,"context_line":"        # fail all jobs on part 2 on sda1"},{"line_number":997,"context_line":"        fail_jobs \u003d ["},{"line_number":998,"context_line":"            {\u0027device\u0027: \u0027sda1\u0027, \u0027partition\u0027: 2},"},{"line_number":999,"context_line":"        ]"},{"line_number":1000,"context_line":"        with mock.patch(\u0027swift.obj.reconstructor.ssync_sender\u0027,"},{"line_number":1001,"context_line":"                        self._make_fake_ssync("}],"source_content_type":"text/x-python","patch_set":5,"id":"bff0334d_09b8f873","line":998,"in_reply_to":"bff0334d_301a147d","updated":"2017-04-06 16:36:41.000000000","message":"I get it :)","commit_id":"77e2ab326128d19d369f4f0bce84d9c01ca8cef6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e0d732d099e9eccffa5f19acb301e5b5e39d373d","unresolved":false,"context_lines":[{"line_number":1000,"context_line":"        with mock.patch(\u0027swift.obj.reconstructor.ssync_sender\u0027,"},{"line_number":1001,"context_line":"                        self._make_fake_ssync("},{"line_number":1002,"context_line":"                            captured_ssync, fail_jobs\u003dfail_jobs)), \\"},{"line_number":1003,"context_line":"                mocked_http_conn(*[200, 200],"},{"line_number":1004,"context_line":"                                 body\u003dpickle.dumps({})) as request_log:"},{"line_number":1005,"context_line":"            self.reconstructor.reconstruct()"},{"line_number":1006,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"bff0334d_b5c84155","line":1003,"updated":"2017-04-06 16:36:41.000000000","message":"I saw this test Pass intermittently [1] when the change in reconstructor was reverted. If the last 2 jobs to be ssync\u0027d happen to be ssync, then the REPLICATE requests (issued with change reverted) cause StopIteration in fake connect and no request is logged, but reconstructor rehash_remote() swallows all exceptions, so the assertion at line 1023 still passes.\n\nI tried fixing this by adding more request status codes, but then (with change in place) we get an assertion error  because there were unused response codes :/\n\nWe could check for error logs from ssync, but better might be to have the fake connect report unexpected connect attempts somehow.\n\n\n[1] False positive with change reverted, seen 12 times in 132 repeats, which makes sense wrt to jobs being shuffled and probability of two for part 2 being the last 2 from 4. (i.e. 1/12)\n\ntest-reconstructor DEBUG: Run listdir on /var/folders/m3/pvk8xs6d5hl7d8hpmmm_yx3c0000gp/T/tmpfdurHb/node/sda1/objects-1/0\ntest-reconstructor DEBUG: Skipping sync_only job for /var/folders/m3/pvk8xs6d5hl7d8hpmmm_yx3c0000gp/T/tmpfdurHb/node/sda1/objects-1/0 while in handoffs_only mode.\ntest-reconstructor DEBUG: Run listdir on /var/folders/m3/pvk8xs6d5hl7d8hpmmm_yx3c0000gp/T/tmpfdurHb/node/sda1/objects-1/1\ntest-reconstructor DEBUG: Skipping sync_only job for /var/folders/m3/pvk8xs6d5hl7d8hpmmm_yx3c0000gp/T/tmpfdurHb/node/sda1/objects-1/1 while in handoffs_only mode.\ntest-reconstructor DEBUG: Run listdir on /var/folders/m3/pvk8xs6d5hl7d8hpmmm_yx3c0000gp/T/tmpfdurHb/node/sda1/objects-1/2\ntest-reconstructor ERROR: Trying to sync suffixes with 127.0.0.2:6200/sda1/2 policy#1: \nTraceback (most recent call last):\n  File \"/Users/anc/0dev/swift/swift/obj/reconstructor.py\", line 547, in rehash_remote\n    headers\u003dself.headers)\n  File \"/Users/anc/0dev/swift/swift/common/bufferedhttp.py\", line 217, in http_connect\n    ipaddr, port, method, path, headers, query_string, ssl)\n  File \"/Users/anc/0dev/swift/test/unit/__init__.py\", line 1024, in connect\n    i, status \u003d next(conn_id_and_code_iter)\nStopIteration\ntest-reconstructor ERROR: Trying to sync suffixes with 127.0.0.0:6200/sda1/2 policy#1: \nTraceback (most recent call last):\n  File \"/Users/anc/0dev/swift/swift/obj/reconstructor.py\", line 547, in rehash_remote\n    headers\u003dself.headers)\n  File \"/Users/anc/0dev/swift/swift/common/bufferedhttp.py\", line 217, in http_connect\n    ipaddr, port, method, path, headers, query_string, ssl)\n  File \"/Users/anc/0dev/swift/test/unit/__init__.py\", line 1024, in connect\n    i, status \u003d next(conn_id_and_code_iter)\nStopIteration\ntest-reconstructor INFO: 3/3 (100.00%) partitions reconstructed in 0.01s (299.85/sec, 0s remaining)\ntest-reconstructor INFO: Handoffs only mode still has handoffs remaining. Next pass will continue to revert handoffs.\n..\n\n----------------------------------------------------------------------\nRan 1 test in 0.054s\n\nOK","commit_id":"77e2ab326128d19d369f4f0bce84d9c01ca8cef6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e0d732d099e9eccffa5f19acb301e5b5e39d373d","unresolved":false,"context_lines":[{"line_number":1020,"context_line":"            for context in captured_ssync"},{"line_number":1021,"context_line":"        ]))"},{"line_number":1022,"context_line":""},{"line_number":1023,"context_line":"        self.assertEqual(2, len(request_log.requests))"},{"line_number":1024,"context_line":"        expected_suffix_calls \u003d []"},{"line_number":1025,"context_line":"        for context in captured_ssync:"},{"line_number":1026,"context_line":"            if not context[\u0027success\u0027]:"}],"source_content_type":"text/x-python","patch_set":5,"id":"bff0334d_cb032842","line":1023,"updated":"2017-04-06 16:36:41.000000000","message":"so a revert job does no pre-flight REPLICATE? which makes sense for EC if we assume that if local node has the frag then the primary does not. But with EC duplication we now have multiple nodes that will be potentially trying to revert to the same primaries, so one or more of them will presumably do an ssync job to a primary that has already been reverted and is totally in sync...and cause the primary to open every diskfile???\n\nAlso, when an ssync job fails after successfully reverting 99% of it\u0027s suffixes, the entire job will be repeated, IIUC.","commit_id":"77e2ab326128d19d369f4f0bce84d9c01ca8cef6"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ec961e7c2da8a86dc7d71bf5f8857cfbc84bb18b","unresolved":false,"context_lines":[{"line_number":900,"context_line":"        :param ssync_calls: an empty list, a non_local, all calls to ssync will"},{"line_number":901,"context_line":"                            be captured for assertion in the caller."},{"line_number":902,"context_line":"        :param fail_jobs: optional iter of dicts, any job passed into Fake that"},{"line_number":903,"context_line":"                          matches a failure dict will return success \u003d\u003d False."},{"line_number":904,"context_line":"        \"\"\""},{"line_number":905,"context_line":"        class _fake_ssync(object):"},{"line_number":906,"context_line":"            def __init__(self, daemon, node, job, suffixes, **kwargs):"}],"source_content_type":"text/x-python","patch_set":6,"id":"bff0334d_4177f0d5","line":903,"updated":"2017-04-08 18:49:47.000000000","message":"oh, that\u0027s helpful!","commit_id":"a0fcca1e0576a0dba7a61f05c86aba23d6ddd27f"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ec961e7c2da8a86dc7d71bf5f8857cfbc84bb18b","unresolved":false,"context_lines":[{"line_number":929,"context_line":"                context[\u0027success\u0027] \u003d self.success"},{"line_number":930,"context_line":""},{"line_number":931,"context_line":"            def __call__(self, *args, **kwargs):"},{"line_number":932,"context_line":"                return self.success, self.available_map if self.success else {}"},{"line_number":933,"context_line":""},{"line_number":934,"context_line":"        return _fake_ssync"},{"line_number":935,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"bff0334d_d0a70cbd","line":932,"updated":"2017-04-08 18:49:47.000000000","message":"awesome thanks!","commit_id":"a0fcca1e0576a0dba7a61f05c86aba23d6ddd27f"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"4e7d6fd58cb368eb29c01ec6e947199b5f64fb96","unresolved":false,"context_lines":[{"line_number":1000,"context_line":"        with mock.patch(\u0027swift.obj.reconstructor.ssync_sender\u0027,"},{"line_number":1001,"context_line":"                        self._make_fake_ssync("},{"line_number":1002,"context_line":"                            captured_ssync, fail_jobs\u003dfail_jobs)), \\"},{"line_number":1003,"context_line":"                mocked_http_conn(*[200, 200],"},{"line_number":1004,"context_line":"                                 body\u003dpickle.dumps({})) as request_log:"},{"line_number":1005,"context_line":"            self.reconstructor.reconstruct()"},{"line_number":1006,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"5ff73747_73b11a47","line":1003,"updated":"2017-04-28 21:23:14.000000000","message":"nit: This feels silly; why not just\n\n mocked_http_conn(200, 200, body\u003d...)\n\n?","commit_id":"a0fcca1e0576a0dba7a61f05c86aba23d6ddd27f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2c8f1741c17ad76f2c9360c4e889f111dd6adc84","unresolved":false,"context_lines":[{"line_number":1036,"context_line":"                         set((r[\u0027ip\u0027], r[\u0027path\u0027])"},{"line_number":1037,"context_line":"                             for r in request_log.requests))"},{"line_number":1038,"context_line":"        self.assertFalse("},{"line_number":1039,"context_line":"            self.reconstructor.logger.get_lines_for_level(\u0027error\u0027))"},{"line_number":1040,"context_line":"        self.assertFalse(request_log.unexpected_requests)"},{"line_number":1041,"context_line":""},{"line_number":1042,"context_line":"    def test_get_part_jobs(self):"}],"source_content_type":"text/x-python","patch_set":6,"id":"bff0334d_6460c9cf","line":1039,"updated":"2017-04-06 16:41:14.000000000","message":"this is possibly sufficient but relies on no other error logging being added for the failed ssync job for example.","commit_id":"a0fcca1e0576a0dba7a61f05c86aba23d6ddd27f"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ec961e7c2da8a86dc7d71bf5f8857cfbc84bb18b","unresolved":false,"context_lines":[{"line_number":1036,"context_line":"                         set((r[\u0027ip\u0027], r[\u0027path\u0027])"},{"line_number":1037,"context_line":"                             for r in request_log.requests))"},{"line_number":1038,"context_line":"        self.assertFalse("},{"line_number":1039,"context_line":"            self.reconstructor.logger.get_lines_for_level(\u0027error\u0027))"},{"line_number":1040,"context_line":"        self.assertFalse(request_log.unexpected_requests)"},{"line_number":1041,"context_line":""},{"line_number":1042,"context_line":"    def test_get_part_jobs(self):"}],"source_content_type":"text/x-python","patch_set":6,"id":"bff0334d_215bcc54","line":1039,"in_reply_to":"bff0334d_6460c9cf","updated":"2017-04-08 18:49:47.000000000","message":"+1 if that pops easy to change this to a whitelist","commit_id":"a0fcca1e0576a0dba7a61f05c86aba23d6ddd27f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2c8f1741c17ad76f2c9360c4e889f111dd6adc84","unresolved":false,"context_lines":[{"line_number":1037,"context_line":"                             for r in request_log.requests))"},{"line_number":1038,"context_line":"        self.assertFalse("},{"line_number":1039,"context_line":"            self.reconstructor.logger.get_lines_for_level(\u0027error\u0027))"},{"line_number":1040,"context_line":"        self.assertFalse(request_log.unexpected_requests)"},{"line_number":1041,"context_line":""},{"line_number":1042,"context_line":"    def test_get_part_jobs(self):"},{"line_number":1043,"context_line":"        # yeah, this test code expects a specific setup"}],"source_content_type":"text/x-python","patch_set":6,"id":"bff0334d_0457bd04","line":1040,"updated":"2017-04-06 16:41:14.000000000","message":"if this assertion is done in the mocked_http_conn before it returns (right where we test for left over status codes) then other tests fail :/","commit_id":"a0fcca1e0576a0dba7a61f05c86aba23d6ddd27f"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ec961e7c2da8a86dc7d71bf5f8857cfbc84bb18b","unresolved":false,"context_lines":[{"line_number":1037,"context_line":"                             for r in request_log.requests))"},{"line_number":1038,"context_line":"        self.assertFalse("},{"line_number":1039,"context_line":"            self.reconstructor.logger.get_lines_for_level(\u0027error\u0027))"},{"line_number":1040,"context_line":"        self.assertFalse(request_log.unexpected_requests)"},{"line_number":1041,"context_line":""},{"line_number":1042,"context_line":"    def test_get_part_jobs(self):"},{"line_number":1043,"context_line":"        # yeah, this test code expects a specific setup"}],"source_content_type":"text/x-python","patch_set":6,"id":"bff0334d_8c22cfa3","line":1040,"in_reply_to":"bff0334d_0457bd04","updated":"2017-04-08 18:49:47.000000000","message":"that\u0027s fine, even if you could make this in the connection conext lots of other tests are probably not robust to ... oh, that\u0027s what you\u0027re saying\n\nyeah i\u0027m not surprised.  When I added the \"no unused mock requests!\" it was big cleanup.  If we get bit by it at some point well fix it then.","commit_id":"a0fcca1e0576a0dba7a61f05c86aba23d6ddd27f"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"4e7d6fd58cb368eb29c01ec6e947199b5f64fb96","unresolved":false,"context_lines":[{"line_number":1037,"context_line":"                             for r in request_log.requests))"},{"line_number":1038,"context_line":"        self.assertFalse("},{"line_number":1039,"context_line":"            self.reconstructor.logger.get_lines_for_level(\u0027error\u0027))"},{"line_number":1040,"context_line":"        self.assertFalse(request_log.unexpected_requests)"},{"line_number":1041,"context_line":""},{"line_number":1042,"context_line":"    def test_get_part_jobs(self):"},{"line_number":1043,"context_line":"        # yeah, this test code expects a specific setup"}],"source_content_type":"text/x-python","patch_set":6,"id":"5ff73747_b3afa328","line":1040,"in_reply_to":"bff0334d_8c22cfa3","updated":"2017-04-28 21:23:14.000000000","message":"...and it looks like you guys had a similar thought. I guess you guys wanted to tackle it as a follow-up?\n\nIDK, 3 failures doesn\u0027t seem so bad to try to clean up...","commit_id":"a0fcca1e0576a0dba7a61f05c86aba23d6ddd27f"}]}