)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"2533610c1baa6dcf0bc87be698b4612a4abc8434","unresolved":true,"context_lines":[{"line_number":9,"context_line":"While auditing a shard container that is in a shrinking state, the"},{"line_number":10,"context_line":"sharder will merge any shard ranges fetched from the root that cover"},{"line_number":11,"context_line":"the shard\u0027s namespace. Previously this was conditional upon the"},{"line_number":12,"context_line":"sharder also fetching the shard\u0027s *own* shard range from the"},{"line_number":13,"context_line":"root. However, in extreme circumstances, the shard\u0027s own shard range"},{"line_number":14,"context_line":"could become deleted and reclaimed from the root while the shard DB"},{"line_number":15,"context_line":"still needs to shrink to its acceptor ranges. This patch therefore"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"42af3f42_a79b70fc","line":12,"updated":"2022-09-30 15:25:03.000000000","message":"so this is what this change is really up to","commit_id":"f15b92084f0f10148f887b75b031686279bab6f3"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"8bcc21fca32e04872707987322f9b09b98839863","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"f90a7d03_0c5cc9bc","updated":"2022-09-19 21:30:01.000000000","message":"I think this lines up with the current thinking.\n\nI\u0027m worried about the probe test timeout, on my machine it\u0027s working ok - but there\u0027s SO many tests and different TestCase configurations in probe/test_sharder.py is taking over an hour!?\n\n\tvagrant@saio:~$ pytest swift/test/probe/test_sharder.py \n\t\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d test session starts \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\n\tplatform linux -- Python 3.10.4, pytest-7.1.2, pluggy-1.0.0\n\trootdir: /home/vagrant/swift\n\tplugins: requests-mock-1.9.3\n\tcollected 48 items                                                                                                                                                                                        \n\n\tswift/test/probe/test_sharder.py .........................................^T.......                                                                                                                   [100%]\n\n\t\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d warnings summary \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\n\tswift/swift/common/utils.py:253\n\t  /home/vagrant/swift/swift/common/utils.py:253: DeprecationWarning: This method will be removed in Python 3.12. Use \u0027parser.read_file()\u0027 instead.\n\t    hash_conf.readfp(swift_conf_file)\n\n\t../../usr/local/lib/python3.10/dist-packages/nose/importer.py:12\n\t  /usr/local/lib/python3.10/dist-packages/nose/importer.py:12: DeprecationWarning: the imp module is deprecated in favour of importlib and slated for removal in Python 3.12; see the module\u0027s documentation for alternative uses\n\t    from imp import find_module, load_module, acquire_lock, release_lock\n\n\ttest/probe/test_sharder.py: 20 warnings\n\t  /usr/local/lib/python3.10/dist-packages/eventlet/tpool.py:295: DeprecationWarning: setDaemon() is deprecated, set the daemon attribute instead\n\t    t.setDaemon(True)\n\n\ttest/probe/test_sharder.py: 35 warnings\n\t  /usr/local/lib/python3.10/dist-packages/eventlet/tpool.py:117: DeprecationWarning: currentThread() is deprecated, use current_thread() instead\n\t    my_thread \u003d threading.currentThread()\n\n\t-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html\n\t\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d 48 passed, 57 warnings in 4370.67s (1:12:50) \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\n","commit_id":"f4849240731a2563e8a51ba6aa94bfc5369de8a1"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"5f748be396803277e4ebf14287fa48d83286849b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"796d9037_d32d6fa5","updated":"2022-09-21 06:09:55.000000000","message":"New refactoring is great, the readability of original code is improved a lot!","commit_id":"5404d5cf10e6642c89f7edad3ae82c03a1f27b99"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0fdc1ff2e59da502fb310e9fed1d0280f2531b4c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"5637eaa2_3906b625","updated":"2022-09-21 18:11:20.000000000","message":"I thought there was some ugly in here, but I think it\u0027s really just that there was a few different existing things that need to be improved.  i think this is a nice/complete improvement in it\u0027s own right.\n\n... but I couldn\u0027t help trying to make it better:\n\nhttps://review.opendev.org/c/openstack/swift/+/858791","commit_id":"a0a5ad571143f851729bd1774d16f53c5c5b6a76"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"27ddbaa938fc0c51287e2eea27154e21a1217406","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"c8e1fd15_13083392","updated":"2022-09-30 16:03:41.000000000","message":"recheck\n\nProbe test failure has to do with the reconciler; not related to this change:\n\nFAIL: test_reconciler_move_object_twice (test.probe.test_container_merge_policy_index.TestReservedNamespaceMergePolicyIndex)\n----------------------------------------------------------------------\nTraceback (most recent call last):\n  File \"/home/zuul/src/opendev.org/openstack/swift/test/probe/test_container_merge_policy_index.py\", line 554, in test_reconciler_move_object_twice\n    self.fail(\u0027Found unexpected object %r in the queue\u0027 % obj)\nAssertionError: Found unexpected object {\u0027bytes\u0027: 0, \u0027hash\u0027: \u00271664537095.49824_3\u0027, \u0027name\u0027: \u00271:/AUTH_test/\\x00container\\x00e97b8edf-cf1e-4768-b864-45d27b549659/\\x00object\\x002cbd30dc-bb92-4ec8-9474-43aa590c37c6\u0027, \u0027content_type\u0027: \u0027application/x-delete\u0027, \u0027last_modified\u0027: \u00272022-09-30T11:24:55.498240\u0027} in the queue\n\n","commit_id":"f15b92084f0f10148f887b75b031686279bab6f3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"2533610c1baa6dcf0bc87be698b4612a4abc8434","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"a8ca58eb_0ea7e462","updated":"2022-09-30 15:25:03.000000000","message":"so i\u0027m totally onboard with this - most of the new tests also pass on master and the SHRINKING_STATES is obvious a good idea\n\nI don\u0027t really know what to think about the change to merge when root is missing osr - it sounds like we think that\u0027s something can happen under normal operation and the extra conservative condition was just an untested pre-existing bug?","commit_id":"f15b92084f0f10148f887b75b031686279bab6f3"}],"swift/common/utils.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"2533610c1baa6dcf0bc87be698b4612a4abc8434","unresolved":true,"context_lines":[{"line_number":5316,"context_line":"              SHARDED: \u0027sharded\u0027,"},{"line_number":5317,"context_line":"              SHRUNK: \u0027shrunk\u0027}"},{"line_number":5318,"context_line":"    STATES_BY_NAME \u003d dict((v, k) for k, v in STATES.items())"},{"line_number":5319,"context_line":"    SHRINKING_STATES \u003d (SHRINKING, SHRUNK)"},{"line_number":5320,"context_line":""},{"line_number":5321,"context_line":"    @functools.total_ordering"},{"line_number":5322,"context_line":"    class MaxBound(ShardRangeOuterBound):"}],"source_content_type":"text/x-python","patch_set":9,"id":"e5c64eb8_1a10827d","line":5319,"updated":"2022-09-30 15:25:03.000000000","message":"this is a nice consolidation - good DRY/SSOT","commit_id":"f15b92084f0f10148f887b75b031686279bab6f3"}],"swift/container/sharder.py":[{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"146043c2518324c39e1adaacdefa3fbfc4160bf9","unresolved":true,"context_lines":[{"line_number":1219,"context_line":"                        break"},{"line_number":1220,"context_line":"                else:"},{"line_number":1221,"context_line":"                    # this is not necessarily an error - some replicas of the"},{"line_number":1222,"context_line":"                    # root may not yet know about this shard container"},{"line_number":1223,"context_line":"                    warnings.append(\u0027root has no matching shard range\u0027)"},{"line_number":1224,"context_line":"            elif not own_shard_range.deleted:"},{"line_number":1225,"context_line":"                warnings.append(\u0027unable to get shard ranges from root\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"c5bd2354_262ef86d","line":1222,"updated":"2022-09-19 06:14:41.000000000","message":"add \"...know about this shard container, or the shard\u0027s own shard range could become deleted and reclaimed from the root under rare conditions\"","commit_id":"b692c9da86037de24856b9387d38e22b5ccbbeea"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"ef183e06b400f771bce511f611e2e814181fbf24","unresolved":false,"context_lines":[{"line_number":1219,"context_line":"                        break"},{"line_number":1220,"context_line":"                else:"},{"line_number":1221,"context_line":"                    # this is not necessarily an error - some replicas of the"},{"line_number":1222,"context_line":"                    # root may not yet know about this shard container"},{"line_number":1223,"context_line":"                    warnings.append(\u0027root has no matching shard range\u0027)"},{"line_number":1224,"context_line":"            elif not own_shard_range.deleted:"},{"line_number":1225,"context_line":"                warnings.append(\u0027unable to get shard ranges from root\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9d763d5b_b1963692","line":1222,"in_reply_to":"c5bd2354_262ef86d","updated":"2022-09-19 15:49:53.000000000","message":"Done","commit_id":"b692c9da86037de24856b9387d38e22b5ccbbeea"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"146043c2518324c39e1adaacdefa3fbfc4160bf9","unresolved":true,"context_lines":[{"line_number":1264,"context_line":"            # We need to include shrunk state too, because one replica of a"},{"line_number":1265,"context_line":"            # shard may already have moved the own_shard_range state to"},{"line_number":1266,"context_line":"            # shrunk while another replica may still be in the process of"},{"line_number":1267,"context_line":"            # shrinking."},{"line_number":1268,"context_line":"            other_shard_ranges \u003d [sr for sr in shard_ranges"},{"line_number":1269,"context_line":"                                  if sr is not own_shard_range_from_root]"},{"line_number":1270,"context_line":"            self.logger.debug(\u0027Updating %s other shard range(s) from root\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"f8bbcb37_c4fb55e5","line":1267,"updated":"2022-09-19 06:14:41.000000000","message":"should we add a print here for the case of own_shard_range_from_root \u003d\u003d None, also verify it in test test_audit_shard_root_ranges_missing_own_merged_while_shrinking? since it\u0027s a rare condition, we may need a log here for production monitoring/debugging purpose.","commit_id":"b692c9da86037de24856b9387d38e22b5ccbbeea"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"5f748be396803277e4ebf14287fa48d83286849b","unresolved":false,"context_lines":[{"line_number":1264,"context_line":"            # We need to include shrunk state too, because one replica of a"},{"line_number":1265,"context_line":"            # shard may already have moved the own_shard_range state to"},{"line_number":1266,"context_line":"            # shrunk while another replica may still be in the process of"},{"line_number":1267,"context_line":"            # shrinking."},{"line_number":1268,"context_line":"            other_shard_ranges \u003d [sr for sr in shard_ranges"},{"line_number":1269,"context_line":"                                  if sr is not own_shard_range_from_root]"},{"line_number":1270,"context_line":"            self.logger.debug(\u0027Updating %s other shard range(s) from root\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"c864f56e_ae243c7b","line":1267,"in_reply_to":"c3da15da_46e56030","updated":"2022-09-21 06:09:55.000000000","message":"Done","commit_id":"b692c9da86037de24856b9387d38e22b5ccbbeea"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"ef183e06b400f771bce511f611e2e814181fbf24","unresolved":true,"context_lines":[{"line_number":1264,"context_line":"            # We need to include shrunk state too, because one replica of a"},{"line_number":1265,"context_line":"            # shard may already have moved the own_shard_range state to"},{"line_number":1266,"context_line":"            # shrunk while another replica may still be in the process of"},{"line_number":1267,"context_line":"            # shrinking."},{"line_number":1268,"context_line":"            other_shard_ranges \u003d [sr for sr in shard_ranges"},{"line_number":1269,"context_line":"                                  if sr is not own_shard_range_from_root]"},{"line_number":1270,"context_line":"            self.logger.debug(\u0027Updating %s other shard range(s) from root\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"c3da15da_46e56030","line":1267,"in_reply_to":"f8bbcb37_c4fb55e5","updated":"2022-09-19 15:49:53.000000000","message":"We do already have a warning log if the own shard range is not fetched from root (line 1223 and 1231), so the output from test test_audit_shard_root_ranges_missing_own_merged_while_shrinking shows:\n\n  sharder-test WARNING: Audit warnings for shard /var/folders/n4/nf8krx110f39llj82fbw_xjm0000gp/T/tmp_iobzl4p/sda/containers/0/60c/30d464df9f9f03bc9a2fb28743bdf60c/30d464df9f9f03bc9a2fb28743bdf60c.db (.shards_a/c-4a8a08f09d37b73795649038408b5f33-1663578047.00000-0): root has no matching shard range\nsharder-test DEBUG: Updating 2 other shard range(s) from root","commit_id":"b692c9da86037de24856b9387d38e22b5ccbbeea"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"90b6085cd105d75f4b191e55cb8305730c1d1df6","unresolved":true,"context_lines":[{"line_number":1188,"context_line":""},{"line_number":1189,"context_line":"        other_shard_ranges \u003d []"},{"line_number":1190,"context_line":"        for shard_range in shard_ranges:"},{"line_number":1191,"context_line":"            if shard_range.name \u003d\u003d own_shard_range.name:"},{"line_number":1192,"context_line":"                # If we find our own shard range in the root response, merge"},{"line_number":1193,"context_line":"                # it and reload own shard range (note: own_range_from_root may"},{"line_number":1194,"context_line":"                # not necessarily be \u0027newer\u0027 than the own shard range we"}],"source_content_type":"text/x-python","patch_set":3,"id":"d019c1be_cf95c032","line":1191,"updated":"2022-09-19 16:51:31.000000000","message":"I know, we found this range already back in _audit_shard_container, but we don\u0027t so this often and I\u0027m hoping the repetition is an acceptable trade off for the code being easier to follow with this method broken out","commit_id":"f4849240731a2563e8a51ba6aa94bfc5369de8a1"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3340e359de6b1314cdb8134e6c0b24070863bf41","unresolved":false,"context_lines":[{"line_number":1188,"context_line":""},{"line_number":1189,"context_line":"        other_shard_ranges \u003d []"},{"line_number":1190,"context_line":"        for shard_range in shard_ranges:"},{"line_number":1191,"context_line":"            if shard_range.name \u003d\u003d own_shard_range.name:"},{"line_number":1192,"context_line":"                # If we find our own shard range in the root response, merge"},{"line_number":1193,"context_line":"                # it and reload own shard range (note: own_range_from_root may"},{"line_number":1194,"context_line":"                # not necessarily be \u0027newer\u0027 than the own shard range we"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba0cd2db_16373a80","line":1191,"in_reply_to":"d019c1be_cf95c032","updated":"2022-09-21 14:59:19.000000000","message":"Done","commit_id":"f4849240731a2563e8a51ba6aa94bfc5369de8a1"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"8bcc21fca32e04872707987322f9b09b98839863","unresolved":true,"context_lines":[{"line_number":1201,"context_line":"                        orig_own_shard_range.state !\u003d own_shard_range.state):"},{"line_number":1202,"context_line":"                    self.logger.debug("},{"line_number":1203,"context_line":"                        \u0027Updated own shard range from %s to %s\u0027,"},{"line_number":1204,"context_line":"                        orig_own_shard_range, own_shard_range)"},{"line_number":1205,"context_line":"            else:"},{"line_number":1206,"context_line":"                other_shard_ranges.append(shard_range)"},{"line_number":1207,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"a7cb371f_9c6b6903","line":1204,"updated":"2022-09-19 21:30:01.000000000","message":"maybe even INFO?  We\u0027re going to merge our osr anytime we can get it from the root, but mostly it\u0027ll be what we\u0027re reporting, right?  This is NOT the common case?","commit_id":"f4849240731a2563e8a51ba6aa94bfc5369de8a1"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c219050be1e14ac26fe4adc38e4972d631184143","unresolved":true,"context_lines":[{"line_number":1201,"context_line":"                        orig_own_shard_range.state !\u003d own_shard_range.state):"},{"line_number":1202,"context_line":"                    self.logger.debug("},{"line_number":1203,"context_line":"                        \u0027Updated own shard range from %s to %s\u0027,"},{"line_number":1204,"context_line":"                        orig_own_shard_range, own_shard_range)"},{"line_number":1205,"context_line":"            else:"},{"line_number":1206,"context_line":"                other_shard_ranges.append(shard_range)"},{"line_number":1207,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"dbd6b580_28bce05f","line":1204,"in_reply_to":"a7cb371f_9c6b6903","updated":"2022-09-20 11:10:48.000000000","message":"Done.\n\nYou\u0027re right, this is not the common case: state may change as the shard is CREATED -\u003e CLEAVED -\u003e ACTIVE but then there is no expected change until perhaps state is changed to SHRINKING, or namespace is changed due to expanding this shard to be an acceptor","commit_id":"f4849240731a2563e8a51ba6aa94bfc5369de8a1"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3340e359de6b1314cdb8134e6c0b24070863bf41","unresolved":false,"context_lines":[{"line_number":1201,"context_line":"                        orig_own_shard_range.state !\u003d own_shard_range.state):"},{"line_number":1202,"context_line":"                    self.logger.debug("},{"line_number":1203,"context_line":"                        \u0027Updated own shard range from %s to %s\u0027,"},{"line_number":1204,"context_line":"                        orig_own_shard_range, own_shard_range)"},{"line_number":1205,"context_line":"            else:"},{"line_number":1206,"context_line":"                other_shard_ranges.append(shard_range)"},{"line_number":1207,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"54bb9b95_2820b555","line":1204,"in_reply_to":"dbd6b580_28bce05f","updated":"2022-09-21 14:59:19.000000000","message":"Done","commit_id":"f4849240731a2563e8a51ba6aa94bfc5369de8a1"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"8bcc21fca32e04872707987322f9b09b98839863","unresolved":true,"context_lines":[{"line_number":1229,"context_line":"            warnings.append(\u0027account not in shards namespace %r\u0027 %"},{"line_number":1230,"context_line":"                            self.shards_account_prefix)"},{"line_number":1231,"context_line":""},{"line_number":1232,"context_line":"        own_shard_range \u003d broker.get_own_shard_range(no_default\u003dTrue)"},{"line_number":1233,"context_line":""},{"line_number":1234,"context_line":"        if own_shard_range:"},{"line_number":1235,"context_line":"            # Get the root view of the world, at least that part of the world"}],"source_content_type":"text/x-python","patch_set":3,"id":"9193ea96_1cfedcf4","line":1232,"updated":"2022-09-19 21:30:01.000000000","message":"maybe \"local_shard_range\" ?","commit_id":"f4849240731a2563e8a51ba6aa94bfc5369de8a1"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2679167145a166ff34a6d2ddb8c5d3afee63373c","unresolved":false,"context_lines":[{"line_number":1229,"context_line":"            warnings.append(\u0027account not in shards namespace %r\u0027 %"},{"line_number":1230,"context_line":"                            self.shards_account_prefix)"},{"line_number":1231,"context_line":""},{"line_number":1232,"context_line":"        own_shard_range \u003d broker.get_own_shard_range(no_default\u003dTrue)"},{"line_number":1233,"context_line":""},{"line_number":1234,"context_line":"        if own_shard_range:"},{"line_number":1235,"context_line":"            # Get the root view of the world, at least that part of the world"}],"source_content_type":"text/x-python","patch_set":3,"id":"434f09cb_3db25182","line":1232,"in_reply_to":"41f6a720_020377d6","updated":"2022-09-30 10:22:25.000000000","message":"Ack","commit_id":"f4849240731a2563e8a51ba6aa94bfc5369de8a1"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c219050be1e14ac26fe4adc38e4972d631184143","unresolved":true,"context_lines":[{"line_number":1229,"context_line":"            warnings.append(\u0027account not in shards namespace %r\u0027 %"},{"line_number":1230,"context_line":"                            self.shards_account_prefix)"},{"line_number":1231,"context_line":""},{"line_number":1232,"context_line":"        own_shard_range \u003d broker.get_own_shard_range(no_default\u003dTrue)"},{"line_number":1233,"context_line":""},{"line_number":1234,"context_line":"        if own_shard_range:"},{"line_number":1235,"context_line":"            # Get the root view of the world, at least that part of the world"}],"source_content_type":"text/x-python","patch_set":3,"id":"41f6a720_020377d6","line":1232,"in_reply_to":"9193ea96_1cfedcf4","updated":"2022-09-20 11:10:48.000000000","message":"own_shard_range is used *so* widely (98 times in this file!) that I\u0027d rather not introduce a variant - I\u0027m using *_from_root to add clarity","commit_id":"f4849240731a2563e8a51ba6aa94bfc5369de8a1"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"8bcc21fca32e04872707987322f9b09b98839863","unresolved":true,"context_lines":[{"line_number":1253,"context_line":"                    # upper bounds for this shard (e.g. if this shard has been"},{"line_number":1254,"context_line":"                    # expanded in the root to accept a shrinking shard) so we"},{"line_number":1255,"context_line":"                    # only match on name."},{"line_number":1256,"context_line":"                    if shard_range.name \u003d\u003d own_shard_range.name:"},{"line_number":1257,"context_line":"                        break"},{"line_number":1258,"context_line":"                else:"},{"line_number":1259,"context_line":"                    # this is not necessarily an error - some replicas of the"}],"source_content_type":"text/x-python","patch_set":3,"id":"1589cee6_8c74560a","line":1256,"updated":"2022-09-19 21:30:01.000000000","message":"maybe go ahead and pull out and pass around remote_shard_range_index_for_own_range or something?  Or split the list into other_shard_ranges and own_shard_range_from_root right here.","commit_id":"f4849240731a2563e8a51ba6aa94bfc5369de8a1"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c219050be1e14ac26fe4adc38e4972d631184143","unresolved":true,"context_lines":[{"line_number":1253,"context_line":"                    # upper bounds for this shard (e.g. if this shard has been"},{"line_number":1254,"context_line":"                    # expanded in the root to accept a shrinking shard) so we"},{"line_number":1255,"context_line":"                    # only match on name."},{"line_number":1256,"context_line":"                    if shard_range.name \u003d\u003d own_shard_range.name:"},{"line_number":1257,"context_line":"                        break"},{"line_number":1258,"context_line":"                else:"},{"line_number":1259,"context_line":"                    # this is not necessarily an error - some replicas of the"}],"source_content_type":"text/x-python","patch_set":3,"id":"4d470caa_7670463b","line":1256,"in_reply_to":"1589cee6_8c74560a","updated":"2022-09-20 11:10:48.000000000","message":"I\u0027ve tried another refactor in next patchset","commit_id":"f4849240731a2563e8a51ba6aa94bfc5369de8a1"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3340e359de6b1314cdb8134e6c0b24070863bf41","unresolved":false,"context_lines":[{"line_number":1253,"context_line":"                    # upper bounds for this shard (e.g. if this shard has been"},{"line_number":1254,"context_line":"                    # expanded in the root to accept a shrinking shard) so we"},{"line_number":1255,"context_line":"                    # only match on name."},{"line_number":1256,"context_line":"                    if shard_range.name \u003d\u003d own_shard_range.name:"},{"line_number":1257,"context_line":"                        break"},{"line_number":1258,"context_line":"                else:"},{"line_number":1259,"context_line":"                    # this is not necessarily an error - some replicas of the"}],"source_content_type":"text/x-python","patch_set":3,"id":"797817ab_544a2d9f","line":1256,"in_reply_to":"4d470caa_7670463b","updated":"2022-09-21 14:59:19.000000000","message":"Done","commit_id":"f4849240731a2563e8a51ba6aa94bfc5369de8a1"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"8bcc21fca32e04872707987322f9b09b98839863","unresolved":true,"context_lines":[{"line_number":1280,"context_line":"            self._increment_stat(\u0027audit_shard\u0027, \u0027failure\u0027, statsd\u003dTrue)"},{"line_number":1281,"context_line":"            return False"},{"line_number":1282,"context_line":""},{"line_number":1283,"context_line":"        # note: we only reach here if own_shard_range is not None"},{"line_number":1284,"context_line":"        own_shard_range \u003d self._merge_shard_ranges_from_root("},{"line_number":1285,"context_line":"            broker, shard_ranges, own_shard_range)"},{"line_number":1286,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"f931374c_994c5202","line":1283,"updated":"2022-09-19 21:30:01.000000000","message":"I\u0027d like to suggest this comment is telling us this method has bad cyclomatic complexity and needs further refactoring.  Perhaps better as a TODO","commit_id":"f4849240731a2563e8a51ba6aa94bfc5369de8a1"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3340e359de6b1314cdb8134e6c0b24070863bf41","unresolved":false,"context_lines":[{"line_number":1280,"context_line":"            self._increment_stat(\u0027audit_shard\u0027, \u0027failure\u0027, statsd\u003dTrue)"},{"line_number":1281,"context_line":"            return False"},{"line_number":1282,"context_line":""},{"line_number":1283,"context_line":"        # note: we only reach here if own_shard_range is not None"},{"line_number":1284,"context_line":"        own_shard_range \u003d self._merge_shard_ranges_from_root("},{"line_number":1285,"context_line":"            broker, shard_ranges, own_shard_range)"},{"line_number":1286,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"ba9996ab_609ce8f4","line":1283,"in_reply_to":"484cb900_04d1976f","updated":"2022-09-21 14:59:19.000000000","message":"Done","commit_id":"f4849240731a2563e8a51ba6aa94bfc5369de8a1"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c219050be1e14ac26fe4adc38e4972d631184143","unresolved":true,"context_lines":[{"line_number":1280,"context_line":"            self._increment_stat(\u0027audit_shard\u0027, \u0027failure\u0027, statsd\u003dTrue)"},{"line_number":1281,"context_line":"            return False"},{"line_number":1282,"context_line":""},{"line_number":1283,"context_line":"        # note: we only reach here if own_shard_range is not None"},{"line_number":1284,"context_line":"        own_shard_range \u003d self._merge_shard_ranges_from_root("},{"line_number":1285,"context_line":"            broker, shard_ranges, own_shard_range)"},{"line_number":1286,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"484cb900_04d1976f","line":1283,"in_reply_to":"f931374c_994c5202","updated":"2022-09-20 11:10:48.000000000","message":"Agree, I\u0027ve tried another refactor in next patchset.","commit_id":"f4849240731a2563e8a51ba6aa94bfc5369de8a1"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"90b6085cd105d75f4b191e55cb8305730c1d1df6","unresolved":true,"context_lines":[{"line_number":1282,"context_line":""},{"line_number":1283,"context_line":"        # note: we only reach here if own_shard_range is not None"},{"line_number":1284,"context_line":"        own_shard_range \u003d self._merge_shard_ranges_from_root("},{"line_number":1285,"context_line":"            broker, shard_ranges, own_shard_range)"},{"line_number":1286,"context_line":""},{"line_number":1287,"context_line":"        delete_age \u003d time.time() - self.reclaim_age"},{"line_number":1288,"context_line":"        deletable_states \u003d (ShardRange.SHARDED, ShardRange.SHRUNK)"}],"source_content_type":"text/x-python","patch_set":3,"id":"e81bf85f_349e9337","line":1285,"updated":"2022-09-19 16:51:31.000000000","message":"_audit_shard_container is already a long method and it is going to get longer with following patches in this chain, so I\u0027m breaking out everything to do with merging ranges from root","commit_id":"f4849240731a2563e8a51ba6aa94bfc5369de8a1"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c219050be1e14ac26fe4adc38e4972d631184143","unresolved":false,"context_lines":[{"line_number":1282,"context_line":""},{"line_number":1283,"context_line":"        # note: we only reach here if own_shard_range is not None"},{"line_number":1284,"context_line":"        own_shard_range \u003d self._merge_shard_ranges_from_root("},{"line_number":1285,"context_line":"            broker, shard_ranges, own_shard_range)"},{"line_number":1286,"context_line":""},{"line_number":1287,"context_line":"        delete_age \u003d time.time() - self.reclaim_age"},{"line_number":1288,"context_line":"        deletable_states \u003d (ShardRange.SHARDED, ShardRange.SHRUNK)"}],"source_content_type":"text/x-python","patch_set":3,"id":"0914e341_40214ead","line":1285,"in_reply_to":"2f381f4d_157147d1","updated":"2022-09-20 11:10:48.000000000","message":"Ack","commit_id":"f4849240731a2563e8a51ba6aa94bfc5369de8a1"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"8bcc21fca32e04872707987322f9b09b98839863","unresolved":true,"context_lines":[{"line_number":1282,"context_line":""},{"line_number":1283,"context_line":"        # note: we only reach here if own_shard_range is not None"},{"line_number":1284,"context_line":"        own_shard_range \u003d self._merge_shard_ranges_from_root("},{"line_number":1285,"context_line":"            broker, shard_ranges, own_shard_range)"},{"line_number":1286,"context_line":""},{"line_number":1287,"context_line":"        delete_age \u003d time.time() - self.reclaim_age"},{"line_number":1288,"context_line":"        deletable_states \u003d (ShardRange.SHARDED, ShardRange.SHRUNK)"}],"source_content_type":"text/x-python","patch_set":3,"id":"2f381f4d_157147d1","line":1285,"in_reply_to":"e81bf85f_349e9337","updated":"2022-09-19 21:30:01.000000000","message":"YES!!! +100","commit_id":"f4849240731a2563e8a51ba6aa94bfc5369de8a1"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"5f748be396803277e4ebf14287fa48d83286849b","unresolved":true,"context_lines":[{"line_number":1181,"context_line":"        self._increment_stat(\u0027audit_root\u0027, \u0027success\u0027, statsd\u003dTrue)"},{"line_number":1182,"context_line":"        return True"},{"line_number":1183,"context_line":""},{"line_number":1184,"context_line":"    def _merge_shard_ranges_from_root(self, broker, shard_ranges,"},{"line_number":1185,"context_line":"                                      own_shard_range):"},{"line_number":1186,"context_line":"        own_shard_range_from_root \u003d None"},{"line_number":1187,"context_line":"        if not shard_ranges:"}],"source_content_type":"text/x-python","patch_set":4,"id":"dbc811d3_0293d058","line":1184,"updated":"2022-09-21 06:09:55.000000000","message":"how about adding a simple docstring to this new function?","commit_id":"5404d5cf10e6642c89f7edad3ae82c03a1f27b99"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"34834ffd7e0d9c3cddcf90a416361163f17de280","unresolved":false,"context_lines":[{"line_number":1181,"context_line":"        self._increment_stat(\u0027audit_root\u0027, \u0027success\u0027, statsd\u003dTrue)"},{"line_number":1182,"context_line":"        return True"},{"line_number":1183,"context_line":""},{"line_number":1184,"context_line":"    def _merge_shard_ranges_from_root(self, broker, shard_ranges,"},{"line_number":1185,"context_line":"                                      own_shard_range):"},{"line_number":1186,"context_line":"        own_shard_range_from_root \u003d None"},{"line_number":1187,"context_line":"        if not shard_ranges:"}],"source_content_type":"text/x-python","patch_set":4,"id":"df94e4f1_63c19bba","line":1184,"in_reply_to":"dbc811d3_0293d058","updated":"2022-09-21 13:51:20.000000000","message":"Done","commit_id":"5404d5cf10e6642c89f7edad3ae82c03a1f27b99"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"5f748be396803277e4ebf14287fa48d83286849b","unresolved":true,"context_lines":[{"line_number":1184,"context_line":"    def _merge_shard_ranges_from_root(self, broker, shard_ranges,"},{"line_number":1185,"context_line":"                                      own_shard_range):"},{"line_number":1186,"context_line":"        own_shard_range_from_root \u003d None"},{"line_number":1187,"context_line":"        if not shard_ranges:"},{"line_number":1188,"context_line":"            return own_shard_range, own_shard_range_from_root"},{"line_number":1189,"context_line":""},{"line_number":1190,"context_line":"        other_shard_ranges \u003d []"}],"source_content_type":"text/x-python","patch_set":4,"id":"b7a6d907_21e99c07","line":1187,"updated":"2022-09-21 06:09:55.000000000","message":"I wonder why we need this check, since both shard_ranges and own_shard_range won\u0027t be None when we call this function? btw, what\u0027s our general coding style for function parameters when we require them not to be None, can we assume they won\u0027t be None after we add them in docstring, or do we check them one by one before we do everything(like c/c++)?","commit_id":"5404d5cf10e6642c89f7edad3ae82c03a1f27b99"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"34834ffd7e0d9c3cddcf90a416361163f17de280","unresolved":false,"context_lines":[{"line_number":1184,"context_line":"    def _merge_shard_ranges_from_root(self, broker, shard_ranges,"},{"line_number":1185,"context_line":"                                      own_shard_range):"},{"line_number":1186,"context_line":"        own_shard_range_from_root \u003d None"},{"line_number":1187,"context_line":"        if not shard_ranges:"},{"line_number":1188,"context_line":"            return own_shard_range, own_shard_range_from_root"},{"line_number":1189,"context_line":""},{"line_number":1190,"context_line":"        other_shard_ranges \u003d []"}],"source_content_type":"text/x-python","patch_set":4,"id":"e7b32df2_1ab8e175","line":1187,"in_reply_to":"b7a6d907_21e99c07","updated":"2022-09-21 13:51:20.000000000","message":"good point, the check was a hangover from before the latest refactor\n\nPolymorphism allows for None to be passed, so having the param in the docstring does NOT imply that the value is not None, although a good doctsring may describe the allowed types. In my experience, checking is not rigorously applied unless None is *expected* but *not supported*, so the onus is on the author to be aware of the types that may be passed. \n\nType-checking can in some cases be in conflict with the goal of polymorphism (particularly when a method is passing args through to other methods, and should therefore be agnostic about the types of the args).","commit_id":"5404d5cf10e6642c89f7edad3ae82c03a1f27b99"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"5f748be396803277e4ebf14287fa48d83286849b","unresolved":true,"context_lines":[{"line_number":1221,"context_line":"            # shard may already have moved the own_shard_range state to"},{"line_number":1222,"context_line":"            # shrunk while another replica may still be in the process of"},{"line_number":1223,"context_line":"            # shrinking."},{"line_number":1224,"context_line":"            self.logger.debug(\u0027Updating %s other shard range(s) from root\u0027,"},{"line_number":1225,"context_line":"                              len(other_shard_ranges))"},{"line_number":1226,"context_line":"            broker.merge_shard_ranges(other_shard_ranges)"},{"line_number":1227,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"fc9098f6_3c473726","line":1224,"updated":"2022-09-21 06:09:55.000000000","message":"should we change this log from debug to info too?","commit_id":"5404d5cf10e6642c89f7edad3ae82c03a1f27b99"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"f0535081a5ce86ee0aeb4e3a0111d0f8442b9fab","unresolved":false,"context_lines":[{"line_number":1221,"context_line":"            # shard may already have moved the own_shard_range state to"},{"line_number":1222,"context_line":"            # shrunk while another replica may still be in the process of"},{"line_number":1223,"context_line":"            # shrinking."},{"line_number":1224,"context_line":"            self.logger.debug(\u0027Updating %s other shard range(s) from root\u0027,"},{"line_number":1225,"context_line":"                              len(other_shard_ranges))"},{"line_number":1226,"context_line":"            broker.merge_shard_ranges(other_shard_ranges)"},{"line_number":1227,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"c028801e_5dff4808","line":1224,"in_reply_to":"cb89e29a_5b95bc5f","updated":"2022-09-21 18:47:58.000000000","message":"Done","commit_id":"5404d5cf10e6642c89f7edad3ae82c03a1f27b99"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"34834ffd7e0d9c3cddcf90a416361163f17de280","unresolved":true,"context_lines":[{"line_number":1221,"context_line":"            # shard may already have moved the own_shard_range state to"},{"line_number":1222,"context_line":"            # shrunk while another replica may still be in the process of"},{"line_number":1223,"context_line":"            # shrinking."},{"line_number":1224,"context_line":"            self.logger.debug(\u0027Updating %s other shard range(s) from root\u0027,"},{"line_number":1225,"context_line":"                              len(other_shard_ranges))"},{"line_number":1226,"context_line":"            broker.merge_shard_ranges(other_shard_ranges)"},{"line_number":1227,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"cb89e29a_5b95bc5f","line":1224,"in_reply_to":"fc9098f6_3c473726","updated":"2022-09-21 13:51:20.000000000","message":"I think that might be too noisy: we expect to receive own_shard_range_from_root *every* time the shard DB is audited. However, we expect own_shard_range_from_root to *modify* the existing own_shard_range less often, when there has been a significant change in state (hence logging as info).","commit_id":"5404d5cf10e6642c89f7edad3ae82c03a1f27b99"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9f1f2ba2ea9095e0b589ec02c5d256de156837d6","unresolved":true,"context_lines":[{"line_number":1253,"context_line":"                        \u0027states\u0027: \u0027auditing\u0027},"},{"line_number":1254,"context_line":"                include_deleted\u003dTrue)"},{"line_number":1255,"context_line":"            if shard_ranges:"},{"line_number":1256,"context_line":"                own_shard_range, own_shard_range_from_root \u003d \\"},{"line_number":1257,"context_line":"                    self._merge_shard_ranges_from_root("},{"line_number":1258,"context_line":"                        broker, shard_ranges, own_shard_range)"},{"line_number":1259,"context_line":"                if not own_shard_range_from_root:"},{"line_number":1260,"context_line":"                    # this is not necessarily an error - some replicas of the"},{"line_number":1261,"context_line":"                    # root may not yet know about this shard container, or the"},{"line_number":1262,"context_line":"                    # shard\u0027s own shard range could become deleted and"},{"line_number":1263,"context_line":"                    # reclaimed from the root under rare conditions"},{"line_number":1264,"context_line":"                    warnings.append(\u0027root has no matching shard range\u0027)"},{"line_number":1265,"context_line":"            elif not own_shard_range.deleted:"},{"line_number":1266,"context_line":"                warnings.append(\u0027unable to get shard ranges from root\u0027)"},{"line_number":1267,"context_line":"            # else, our shard range is deleted, so root may have reclaimed it"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ac8c852_0be51ae1","line":1264,"range":{"start_line":1256,"start_character":16,"end_line":1264,"end_character":71},"updated":"2022-09-20 11:57:36.000000000","message":"I also considered passing the warnings list to _merge_shard_ranges_from_root() for it to update with the warning message, rather than returning own_shard_range_from_root and updating the warnings here.\n\nIDK which is least ugly.","commit_id":"5404d5cf10e6642c89f7edad3ae82c03a1f27b99"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2679167145a166ff34a6d2ddb8c5d3afee63373c","unresolved":false,"context_lines":[{"line_number":1253,"context_line":"                        \u0027states\u0027: \u0027auditing\u0027},"},{"line_number":1254,"context_line":"                include_deleted\u003dTrue)"},{"line_number":1255,"context_line":"            if shard_ranges:"},{"line_number":1256,"context_line":"                own_shard_range, own_shard_range_from_root \u003d \\"},{"line_number":1257,"context_line":"                    self._merge_shard_ranges_from_root("},{"line_number":1258,"context_line":"                        broker, shard_ranges, own_shard_range)"},{"line_number":1259,"context_line":"                if not own_shard_range_from_root:"},{"line_number":1260,"context_line":"                    # this is not necessarily an error - some replicas of the"},{"line_number":1261,"context_line":"                    # root may not yet know about this shard container, or the"},{"line_number":1262,"context_line":"                    # shard\u0027s own shard range could become deleted and"},{"line_number":1263,"context_line":"                    # reclaimed from the root under rare conditions"},{"line_number":1264,"context_line":"                    warnings.append(\u0027root has no matching shard range\u0027)"},{"line_number":1265,"context_line":"            elif not own_shard_range.deleted:"},{"line_number":1266,"context_line":"                warnings.append(\u0027unable to get shard ranges from root\u0027)"},{"line_number":1267,"context_line":"            # else, our shard range is deleted, so root may have reclaimed it"}],"source_content_type":"text/x-python","patch_set":4,"id":"06f6aa7d_4504b89a","line":1264,"range":{"start_line":1256,"start_character":16,"end_line":1264,"end_character":71},"in_reply_to":"5ac8c852_0be51ae1","updated":"2022-09-30 10:22:25.000000000","message":"Done","commit_id":"5404d5cf10e6642c89f7edad3ae82c03a1f27b99"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9f1f2ba2ea9095e0b589ec02c5d256de156837d6","unresolved":true,"context_lines":[{"line_number":1276,"context_line":"                self.logger.debug(\u0027Deleted shard container %s (%s)\u0027,"},{"line_number":1277,"context_line":"                                  broker.db_file, quote(broker.path))"},{"line_number":1278,"context_line":"        else:"},{"line_number":1279,"context_line":"            errors.append(\u0027missing own shard range\u0027)"},{"line_number":1280,"context_line":""},{"line_number":1281,"context_line":"        if warnings:"},{"line_number":1282,"context_line":"            self.logger.warning("}],"source_content_type":"text/x-python","patch_set":4,"id":"dc3c50b3_72f939a0","line":1279,"range":{"start_line":1279,"start_character":12,"end_line":1279,"end_character":52},"updated":"2022-09-20 11:57:36.000000000","message":"this is actually the only error - IIRC we originally imagined the audit functions growing more features and perhaps finding other errors.\n\nHowever, we need to handle possible warnings independent of the if/else, so either stick with building an errors list or set a boolean error flag - either way we cannot return False directly from this else clause without handling the potential warnings.","commit_id":"5404d5cf10e6642c89f7edad3ae82c03a1f27b99"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3340e359de6b1314cdb8134e6c0b24070863bf41","unresolved":false,"context_lines":[{"line_number":1276,"context_line":"                self.logger.debug(\u0027Deleted shard container %s (%s)\u0027,"},{"line_number":1277,"context_line":"                                  broker.db_file, quote(broker.path))"},{"line_number":1278,"context_line":"        else:"},{"line_number":1279,"context_line":"            errors.append(\u0027missing own shard range\u0027)"},{"line_number":1280,"context_line":""},{"line_number":1281,"context_line":"        if warnings:"},{"line_number":1282,"context_line":"            self.logger.warning("}],"source_content_type":"text/x-python","patch_set":4,"id":"709e99d6_35f37f03","line":1279,"range":{"start_line":1279,"start_character":12,"end_line":1279,"end_character":52},"in_reply_to":"dc3c50b3_72f939a0","updated":"2022-09-21 14:59:19.000000000","message":"Ack","commit_id":"5404d5cf10e6642c89f7edad3ae82c03a1f27b99"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9f1f2ba2ea9095e0b589ec02c5d256de156837d6","unresolved":true,"context_lines":[{"line_number":1289,"context_line":"                broker.db_file, quote(broker.path), \u0027, \u0027.join(errors))"},{"line_number":1290,"context_line":"            self._increment_stat(\u0027audit_shard\u0027, \u0027failure\u0027, statsd\u003dTrue)"},{"line_number":1291,"context_line":"            return False"},{"line_number":1292,"context_line":"        else:"},{"line_number":1293,"context_line":"            self._increment_stat(\u0027audit_shard\u0027, \u0027success\u0027, statsd\u003dTrue)"},{"line_number":1294,"context_line":"            return True"},{"line_number":1295,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"6d0a14c7_5fac8686","line":1292,"updated":"2022-09-20 11:57:36.000000000","message":"I like that we now have an else clause and the return value clarity","commit_id":"5404d5cf10e6642c89f7edad3ae82c03a1f27b99"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"34834ffd7e0d9c3cddcf90a416361163f17de280","unresolved":true,"context_lines":[{"line_number":1289,"context_line":"                broker.db_file, quote(broker.path), \u0027, \u0027.join(errors))"},{"line_number":1290,"context_line":"            self._increment_stat(\u0027audit_shard\u0027, \u0027failure\u0027, statsd\u003dTrue)"},{"line_number":1291,"context_line":"            return False"},{"line_number":1292,"context_line":"        else:"},{"line_number":1293,"context_line":"            self._increment_stat(\u0027audit_shard\u0027, \u0027success\u0027, statsd\u003dTrue)"},{"line_number":1294,"context_line":"            return True"},{"line_number":1295,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"d6d03c63_86d52d4f","line":1292,"in_reply_to":"0b5d3429_eefdee9f","updated":"2022-09-21 13:51:20.000000000","message":"problem is I cannot move line 1291 earlier:\n\n  return False\n  \nbecause the warnings need to be logged before returning.\n\nI tried:\n\n  diff --git a/swift/container/sharder.py b/swift/container/sharder.py\n  index 69a1ce7a2..7bd0a6c91 100644\n  --- a/swift/container/sharder.py\n  +++ b/swift/container/sharder.py\n  @@ -1242,9 +1242,9 @@ class ContainerSharder(ContainerSharderConf, ContainerReplicator):\n         return own_shard_range, own_shard_range_from_root\n\n     def _audit_shard_container(self, broker):\n  +        success \u003d True\n           self._increment_stat(\u0027audit_shard\u0027, \u0027attempted\u0027)\n           warnings \u003d []\n  -        errors \u003d []\n           if not broker.account.startswith(self.shards_account_prefix):\n               warnings.append(\u0027account not in shards namespace %r\u0027 %\n                               self.shards_account_prefix)\n  @@ -1290,22 +1290,20 @@ class ContainerSharder(ContainerSharderConf, ContainerReplicator):\n                 self.logger.debug(\u0027Deleted shard container %s (%s)\u0027,\n                                   broker.db_file, quote(broker.path))\n         else:\n  -            errors.append(\u0027missing own shard range\u0027)\n  +            success \u003d False\n  +            self.logger.warning(\u0027Audit failed for shard %s (%s) - skipping: \u0027\n  +                                \u0027missing own shard range\u0027,\n  +                                broker.db_file, quote(broker.path))\n\n         if warnings:\n             self.logger.warning(\n                 \u0027Audit warnings for shard %s (%s): %s\u0027,\n                 broker.db_file, quote(broker.path), \u0027, \u0027.join(warnings))\n\n  -        if errors:\n  -            self.logger.warning(\n  -                \u0027Audit failed for shard %s (%s) - skipping: %s\u0027,\n  -                broker.db_file, quote(broker.path), \u0027, \u0027.join(errors))\n  -            self._increment_stat(\u0027audit_shard\u0027, \u0027failure\u0027, statsd\u003dTrue)\n  -            return False\n  -        else:\n  -            self._increment_stat(\u0027audit_shard\u0027, \u0027success\u0027, statsd\u003dTrue)\n  -            return True\n  +        self._increment_stat(\n  +            \u0027audit_shard\u0027, \u0027success\u0027 if success else \u0027failure\u0027, statsd\u003dTrue)\n  +\n  +        return success\n\n      def _audit_cleave_contexts(self, broker):\n          now \u003d Timestamp.now()\n\nbut that reverses the asserted order of the warnings, which we can fix in tests, but I wonder if it is worth it? we still have to maintain the boolean success?","commit_id":"5404d5cf10e6642c89f7edad3ae82c03a1f27b99"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"5f748be396803277e4ebf14287fa48d83286849b","unresolved":true,"context_lines":[{"line_number":1289,"context_line":"                broker.db_file, quote(broker.path), \u0027, \u0027.join(errors))"},{"line_number":1290,"context_line":"            self._increment_stat(\u0027audit_shard\u0027, \u0027failure\u0027, statsd\u003dTrue)"},{"line_number":1291,"context_line":"            return False"},{"line_number":1292,"context_line":"        else:"},{"line_number":1293,"context_line":"            self._increment_stat(\u0027audit_shard\u0027, \u0027success\u0027, statsd\u003dTrue)"},{"line_number":1294,"context_line":"            return True"},{"line_number":1295,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"0b5d3429_eefdee9f","line":1292,"in_reply_to":"6d0a14c7_5fac8686","updated":"2022-09-21 06:09:55.000000000","message":"Yes, I agree. One further step, since there is only one possibility of \"errors\", how about removing \"if errors\" check at line 1286, and move all the logic (if errors) to line 1279?","commit_id":"5404d5cf10e6642c89f7edad3ae82c03a1f27b99"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3340e359de6b1314cdb8134e6c0b24070863bf41","unresolved":false,"context_lines":[{"line_number":1289,"context_line":"                broker.db_file, quote(broker.path), \u0027, \u0027.join(errors))"},{"line_number":1290,"context_line":"            self._increment_stat(\u0027audit_shard\u0027, \u0027failure\u0027, statsd\u003dTrue)"},{"line_number":1291,"context_line":"            return False"},{"line_number":1292,"context_line":"        else:"},{"line_number":1293,"context_line":"            self._increment_stat(\u0027audit_shard\u0027, \u0027success\u0027, statsd\u003dTrue)"},{"line_number":1294,"context_line":"            return True"},{"line_number":1295,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"fa286bf1_b64fc7d7","line":1292,"in_reply_to":"d6d03c63_86d52d4f","updated":"2022-09-21 14:59:19.000000000","message":"Done","commit_id":"5404d5cf10e6642c89f7edad3ae82c03a1f27b99"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0fdc1ff2e59da502fb310e9fed1d0280f2531b4c","unresolved":true,"context_lines":[{"line_number":1277,"context_line":"                broker.empty()):"},{"line_number":1278,"context_line":"            broker.delete_db(Timestamp.now().internal)"},{"line_number":1279,"context_line":"            self.logger.debug(\u0027Deleted shard container %s (%s)\u0027,"},{"line_number":1280,"context_line":"                              broker.db_file, quote(broker.path))"},{"line_number":1281,"context_line":""},{"line_number":1282,"context_line":"        self._increment_stat(\u0027audit_shard\u0027, \u0027success\u0027, statsd\u003dTrue)"},{"line_number":1283,"context_line":"        return True"}],"source_content_type":"text/x-python","patch_set":6,"id":"8a203d06_add8365e","side":"PARENT","line":1280,"updated":"2022-09-21 18:11:20.000000000","message":"ugh, so I didn\u0027t know that we even did this clean out - and now it sort of sticks out for possible-method-extraction under the already indented `if shard_ranges` block","commit_id":"1e8c5cf81ba429cefefb558da7c3552f137d59a4"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"2533610c1baa6dcf0bc87be698b4612a4abc8434","unresolved":false,"context_lines":[{"line_number":1277,"context_line":"                broker.empty()):"},{"line_number":1278,"context_line":"            broker.delete_db(Timestamp.now().internal)"},{"line_number":1279,"context_line":"            self.logger.debug(\u0027Deleted shard container %s (%s)\u0027,"},{"line_number":1280,"context_line":"                              broker.db_file, quote(broker.path))"},{"line_number":1281,"context_line":""},{"line_number":1282,"context_line":"        self._increment_stat(\u0027audit_shard\u0027, \u0027success\u0027, statsd\u003dTrue)"},{"line_number":1283,"context_line":"        return True"}],"source_content_type":"text/x-python","patch_set":6,"id":"f62c1fcc_137a9d27","side":"PARENT","line":1280,"in_reply_to":"8a203d06_add8365e","updated":"2022-09-30 15:25:03.000000000","message":"Done","commit_id":"1e8c5cf81ba429cefefb558da7c3552f137d59a4"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3522a8532a8d335e32f4e8bb13512a2f40611df9","unresolved":true,"context_lines":[{"line_number":1225,"context_line":"            else:"},{"line_number":1226,"context_line":"                other_shard_ranges.append(shard_range)"},{"line_number":1227,"context_line":""},{"line_number":1228,"context_line":"        if (other_shard_ranges and"},{"line_number":1229,"context_line":"                own_shard_range.state in ShardRange.SHRINKING_STATES):"},{"line_number":1230,"context_line":"            # If own_shard_range state is shrinking, save off *all* shards"},{"line_number":1231,"context_line":"            # returned because these may contain shards into which this"}],"source_content_type":"text/x-python","patch_set":6,"id":"dae46208_f8b9d865","line":1228,"updated":"2022-09-23 10:11:40.000000000","message":"so IIRC the actual behavioural change w.r.t. master is that this clause no longer depends on own_shard_range_from_root existing","commit_id":"a0a5ad571143f851729bd1774d16f53c5c5b6a76"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2679167145a166ff34a6d2ddb8c5d3afee63373c","unresolved":false,"context_lines":[{"line_number":1225,"context_line":"            else:"},{"line_number":1226,"context_line":"                other_shard_ranges.append(shard_range)"},{"line_number":1227,"context_line":""},{"line_number":1228,"context_line":"        if (other_shard_ranges and"},{"line_number":1229,"context_line":"                own_shard_range.state in ShardRange.SHRINKING_STATES):"},{"line_number":1230,"context_line":"            # If own_shard_range state is shrinking, save off *all* shards"},{"line_number":1231,"context_line":"            # returned because these may contain shards into which this"}],"source_content_type":"text/x-python","patch_set":6,"id":"b1fa64ea_4bb3218f","line":1228,"in_reply_to":"dae46208_f8b9d865","updated":"2022-09-30 10:22:25.000000000","message":"Ack","commit_id":"a0a5ad571143f851729bd1774d16f53c5c5b6a76"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0fdc1ff2e59da502fb310e9fed1d0280f2531b4c","unresolved":true,"context_lines":[{"line_number":1290,"context_line":"                self.logger.debug(\u0027Deleted shard container %s (%s)\u0027,"},{"line_number":1291,"context_line":"                                  broker.db_file, quote(broker.path))"},{"line_number":1292,"context_line":"        else:"},{"line_number":1293,"context_line":"            success \u003d False"},{"line_number":1294,"context_line":"            self.logger.warning(\u0027Audit failed for shard %s (%s) - skipping: \u0027"},{"line_number":1295,"context_line":"                                \u0027missing own shard range\u0027,"},{"line_number":1296,"context_line":"                                broker.db_file, quote(broker.path))"}],"source_content_type":"text/x-python","patch_set":6,"id":"8a584e43_ede7d10b","line":1293,"updated":"2022-09-21 18:11:20.000000000","message":"it\u0027s partly comments and other things that could maybe be extracted, but this re-set of success is kind of far apart from the init and is AFAIK the only place we set the retrun value to False\n\na guard return might be ok if we figure out how to handle the warnings","commit_id":"a0a5ad571143f851729bd1774d16f53c5c5b6a76"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2679167145a166ff34a6d2ddb8c5d3afee63373c","unresolved":false,"context_lines":[{"line_number":1290,"context_line":"                self.logger.debug(\u0027Deleted shard container %s (%s)\u0027,"},{"line_number":1291,"context_line":"                                  broker.db_file, quote(broker.path))"},{"line_number":1292,"context_line":"        else:"},{"line_number":1293,"context_line":"            success \u003d False"},{"line_number":1294,"context_line":"            self.logger.warning(\u0027Audit failed for shard %s (%s) - skipping: \u0027"},{"line_number":1295,"context_line":"                                \u0027missing own shard range\u0027,"},{"line_number":1296,"context_line":"                                broker.db_file, quote(broker.path))"}],"source_content_type":"text/x-python","patch_set":6,"id":"0cdf029b_ca750828","line":1293,"in_reply_to":"8a584e43_ede7d10b","updated":"2022-09-30 10:22:25.000000000","message":"Ack","commit_id":"a0a5ad571143f851729bd1774d16f53c5c5b6a76"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3340e359de6b1314cdb8134e6c0b24070863bf41","unresolved":true,"context_lines":[{"line_number":1293,"context_line":"            success \u003d False"},{"line_number":1294,"context_line":"            self.logger.warning(\u0027Audit failed for shard %s (%s) - skipping: \u0027"},{"line_number":1295,"context_line":"                                \u0027missing own shard range\u0027,"},{"line_number":1296,"context_line":"                                broker.db_file, quote(broker.path))"},{"line_number":1297,"context_line":""},{"line_number":1298,"context_line":"        if warnings:"},{"line_number":1299,"context_line":"            self.logger.warning("}],"source_content_type":"text/x-python","patch_set":6,"id":"3251b09b_a597db34","line":1296,"updated":"2022-09-21 14:59:19.000000000","message":"ok, I went ahead and got rid of the errors list and just log the single error case here. This does change the order of the audit errors vs warnings logs.","commit_id":"a0a5ad571143f851729bd1774d16f53c5c5b6a76"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"f0535081a5ce86ee0aeb4e3a0111d0f8442b9fab","unresolved":false,"context_lines":[{"line_number":1293,"context_line":"            success \u003d False"},{"line_number":1294,"context_line":"            self.logger.warning(\u0027Audit failed for shard %s (%s) - skipping: \u0027"},{"line_number":1295,"context_line":"                                \u0027missing own shard range\u0027,"},{"line_number":1296,"context_line":"                                broker.db_file, quote(broker.path))"},{"line_number":1297,"context_line":""},{"line_number":1298,"context_line":"        if warnings:"},{"line_number":1299,"context_line":"            self.logger.warning("}],"source_content_type":"text/x-python","patch_set":6,"id":"032782c1_945bc01a","line":1296,"in_reply_to":"3251b09b_a597db34","updated":"2022-09-21 18:47:58.000000000","message":"Done","commit_id":"a0a5ad571143f851729bd1774d16f53c5c5b6a76"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"7dbf3947fceb3205787f781187a240fa963ffcda","unresolved":true,"context_lines":[{"line_number":1258,"context_line":"                own_shard_range.timestamp \u003c delete_age and"},{"line_number":1259,"context_line":"                broker.empty()):"},{"line_number":1260,"context_line":"            broker.delete_db(Timestamp.now().internal)"},{"line_number":1261,"context_line":"            self.logger.debug(\u0027Deleted shard container %s (%s)\u0027,"},{"line_number":1262,"context_line":"                              broker.db_file, quote(broker.path))"},{"line_number":1263,"context_line":""},{"line_number":1264,"context_line":"    def _do_audit_shard_container(self, broker):"}],"source_content_type":"text/x-python","patch_set":7,"id":"06cdf3ce_32b102da","line":1261,"updated":"2022-09-28 18:44:34.000000000","message":"nit: how about change to \u0027Marked shard container %s (%s) as deleted.\u0027?","commit_id":"63e477fa2ad051d449b001dd2de5978b96730345"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2679167145a166ff34a6d2ddb8c5d3afee63373c","unresolved":false,"context_lines":[{"line_number":1258,"context_line":"                own_shard_range.timestamp \u003c delete_age and"},{"line_number":1259,"context_line":"                broker.empty()):"},{"line_number":1260,"context_line":"            broker.delete_db(Timestamp.now().internal)"},{"line_number":1261,"context_line":"            self.logger.debug(\u0027Deleted shard container %s (%s)\u0027,"},{"line_number":1262,"context_line":"                              broker.db_file, quote(broker.path))"},{"line_number":1263,"context_line":""},{"line_number":1264,"context_line":"    def _do_audit_shard_container(self, broker):"}],"source_content_type":"text/x-python","patch_set":7,"id":"078a873d_b164cc4c","line":1261,"in_reply_to":"06cdf3ce_32b102da","updated":"2022-09-30 10:22:25.000000000","message":"Done","commit_id":"63e477fa2ad051d449b001dd2de5978b96730345"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"2533610c1baa6dcf0bc87be698b4612a4abc8434","unresolved":true,"context_lines":[{"line_number":1226,"context_line":"                other_shard_ranges.append(shard_range)"},{"line_number":1227,"context_line":""},{"line_number":1228,"context_line":"        if (other_shard_ranges and own_shard_range_from_root and"},{"line_number":1229,"context_line":"                own_shard_range.state in"},{"line_number":1230,"context_line":"                (ShardRange.SHRINKING, ShardRange.SHRUNK)):"},{"line_number":1231,"context_line":"            # If own_shard_range state is shrinking, save off *all* shards"},{"line_number":1232,"context_line":"            # returned because these may contain shards into which this"}],"source_content_type":"text/x-python","patch_set":9,"id":"bd57e998_794bcc01","side":"PARENT","line":1229,"updated":"2022-09-30 15:25:03.000000000","message":"I don\u0027t think this condition was a behavior we had explicitly relied on, removing the (and osr_from_root) condition doesn\u0027t break any tests on master.","commit_id":"429702e30de95ca3f0188b89d51364f6b47d5121"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"27ddbaa938fc0c51287e2eea27154e21a1217406","unresolved":true,"context_lines":[{"line_number":1226,"context_line":"                other_shard_ranges.append(shard_range)"},{"line_number":1227,"context_line":""},{"line_number":1228,"context_line":"        if (other_shard_ranges and own_shard_range_from_root and"},{"line_number":1229,"context_line":"                own_shard_range.state in"},{"line_number":1230,"context_line":"                (ShardRange.SHRINKING, ShardRange.SHRUNK)):"},{"line_number":1231,"context_line":"            # If own_shard_range state is shrinking, save off *all* shards"},{"line_number":1232,"context_line":"            # returned because these may contain shards into which this"}],"source_content_type":"text/x-python","patch_set":9,"id":"9c2195e6_456fa84d","side":"PARENT","line":1229,"in_reply_to":"bd57e998_794bcc01","updated":"2022-09-30 16:03:41.000000000","message":"That doesn\u0027t surprise me -- I\u0027m pretty sure we had it the old way out of an abundance of caution; we always expected to find ourselves in the root\u0027s response. Now that we\u0027ve seen how that can be violated, and how painful it is to fix things while expecting to find ourselves in the response, makes sense that we\u0027d want to drop the requirement.","commit_id":"429702e30de95ca3f0188b89d51364f6b47d5121"}],"test/unit/container/test_sharder.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"2533610c1baa6dcf0bc87be698b4612a4abc8434","unresolved":true,"context_lines":[{"line_number":5947,"context_line":"                ts, lower\u003d\u0027a\u0027, upper\u003d\u0027b\u0027, state\u003down_state, state_timestamp\u003dts)"},{"line_number":5948,"context_line":"            expected \u003d [acceptor_from_root, root_from_root]"},{"line_number":5949,"context_line":"            with annotate_failure(\u0027with states %s %s %s\u0027"},{"line_number":5950,"context_line":"                                  % (own_state, acceptor_state, root_state)):"},{"line_number":5951,"context_line":"                sharder \u003d self._assert_merge_into_shard("},{"line_number":5952,"context_line":"                    own_sr, [],"},{"line_number":5953,"context_line":"                    # own sr is in ranges fetched from root"}],"source_content_type":"text/x-python","patch_set":9,"id":"cdfa4085_c0322c9e","line":5950,"updated":"2022-09-30 15:25:03.000000000","message":"annotate what what!?  you gettin\u0027 FANCY!","commit_id":"f15b92084f0f10148f887b75b031686279bab6f3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"2533610c1baa6dcf0bc87be698b4612a4abc8434","unresolved":true,"context_lines":[{"line_number":5985,"context_line":"                                  % (own_state, acceptor_state, root_state)):"},{"line_number":5986,"context_line":"                sharder \u003d self._assert_merge_into_shard("},{"line_number":5987,"context_line":"                    own_sr, [],"},{"line_number":5988,"context_line":"                    # own sr is NOT in ranges fetched from root"},{"line_number":5989,"context_line":"                    [acceptor_from_root, root_from_root],"},{"line_number":5990,"context_line":"                    expected, \u0027Quoted-Root\u0027, \u0027a/c\u0027)"},{"line_number":5991,"context_line":"                warning_lines \u003d sharder.logger.get_lines_for_level(\u0027warning\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"0cd0043c_57284631","line":5988,"updated":"2022-09-30 15:25:03.000000000","message":"so may dropping the \"own_sr_from_root\" condition WAS a change - but I don\u0027t know if it\u0027s significant\n\n\tswift/test/unit/container/test_sharder.py:6000: \n\t_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ \n\tswift/test/unit/container/test_sharder.py:5986: in do_test\n\t    sharder \u003d self._assert_merge_into_shard(\n\tswift/test/unit/container/test_sharder.py:5876: in _assert_merge_into_shard\n\t    self._assert_shard_ranges_equal(expected, broker.get_shard_ranges())\n\tswift/test/unit/container/test_sharder.py:68: in _assert_shard_ranges_equal\n\t    self.assertEqual([dict(sr) for sr in expected],\n\tE   AssertionError: with states 50 10 10 Failed with Lists differ: [{\u0027name\u0027: \u0027.shards_a/c-4a8a08f09d37b737956[543 chars] -1}] !\u003d []\n\tE   \n\tE   First list contains 2 additional elements.","commit_id":"f15b92084f0f10148f887b75b031686279bab6f3"}]}