)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f5484323ca66d2dbdfc60352043f8fa57ae0936e","unresolved":true,"context_lines":[{"line_number":14,"context_line":"containers to be very slow."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Instead, this patch only checks if there is any child shard range"},{"line_number":17,"context_line":"exists in the shard table, which can be very fast. On a benchmark"},{"line_number":18,"context_line":"setup, this brings ~10X improvement to HEAD and ~2X improvement to"},{"line_number":19,"context_line":"GET into a single container server which serves a very large container"},{"line_number":20,"context_line":"DB instance."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"e73499d9_07229163","line":17,"updated":"2023-07-17 17:01:29.000000000","message":"very fast sounds good!","commit_id":"606e8d4a0354e1d4291789805f49b35617b93e37"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"673db9f1bbe8b23393034ab861587d461797bd27","unresolved":false,"context_lines":[{"line_number":14,"context_line":"containers to be very slow."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Instead, this patch only checks if there is any child shard range"},{"line_number":17,"context_line":"exists in the shard table, which can be very fast. On a benchmark"},{"line_number":18,"context_line":"setup, this brings ~10X improvement to HEAD and ~2X improvement to"},{"line_number":19,"context_line":"GET into a single container server which serves a very large container"},{"line_number":20,"context_line":"DB instance."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"d98c6c78_82783bd6","line":17,"in_reply_to":"e73499d9_07229163","updated":"2023-07-18 05:09:40.000000000","message":"Ack","commit_id":"606e8d4a0354e1d4291789805f49b35617b93e37"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"8d4fda58b73accbfa1958acf3314dacc775c0b92","unresolved":true,"context_lines":[{"line_number":19,"context_line":"execution time reduction by a factor of ~1000. On a container server"},{"line_number":20,"context_line":"setup which serves a container with ~12000 shard ranges, this patch"},{"line_number":21,"context_line":"improves HTTP HEAD request rate by 10X, and improves HTTP GET request"},{"line_number":22,"context_line":"rate by ~2X."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"Change-Id: I01fd4f3e395c8846280f44e17a56935fc6210444"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"a14b9360_619d8e6c","line":22,"updated":"2023-07-18 13:56:57.000000000","message":"woot!","commit_id":"7c18305369bcde6ebdbd69168c1dcc954dfcfed0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"29bc41c7e2e1728777a2b32bb3fd2ebe30ca069d","unresolved":true,"context_lines":[{"line_number":19,"context_line":"execution time reduction by a factor of ~1000. On a container server"},{"line_number":20,"context_line":"setup which serves a container with ~12000 shard ranges, this patch"},{"line_number":21,"context_line":"improves HTTP HEAD request rate by 10X, and improves HTTP GET request"},{"line_number":22,"context_line":"rate by ~2X."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"Change-Id: I01fd4f3e395c8846280f44e17a56935fc6210444"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"ad7e61cc_f394bc4c","line":22,"in_reply_to":"a14b9360_619d8e6c","updated":"2023-07-19 08:54:29.000000000","message":"+1 this is just one of several optimisations that @Jianjian\u0027s profiling suggested, so this is an important contribution to hopefully a ~20x improvement","commit_id":"7c18305369bcde6ebdbd69168c1dcc954dfcfed0"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"95281a32815325754c790708ac87974a435907ed","unresolved":false,"context_lines":[{"line_number":19,"context_line":"execution time reduction by a factor of ~1000. On a container server"},{"line_number":20,"context_line":"setup which serves a container with ~12000 shard ranges, this patch"},{"line_number":21,"context_line":"improves HTTP HEAD request rate by 10X, and improves HTTP GET request"},{"line_number":22,"context_line":"rate by ~2X."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"Change-Id: I01fd4f3e395c8846280f44e17a56935fc6210444"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"7dd79f48_1a115dc9","line":22,"in_reply_to":"ad7e61cc_f394bc4c","updated":"2023-07-20 17:46:57.000000000","message":"Ack","commit_id":"7c18305369bcde6ebdbd69168c1dcc954dfcfed0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"29bc41c7e2e1728777a2b32bb3fd2ebe30ca069d","unresolved":true,"context_lines":[{"line_number":10,"context_line":"and sharding_initiated(), they query the number of shard ranges of"},{"line_number":11,"context_line":"this container by pulling out all shard ranges from shard range table,"},{"line_number":12,"context_line":"instantiating ShardRange objects and then counting how many they are."},{"line_number":13,"context_line":"Those operations are very expansive, and causing HEAD/GET into large"},{"line_number":14,"context_line":"containers to be very slow."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Instead, this patch only checks if there is any child shard range"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"6e14de5f_3f071a85","line":13,"updated":"2023-07-19 08:54:29.000000000","message":"s/expansive/expensive/","commit_id":"7e1fc226487cdf8ab285bc689961e10efee956bc"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"a50b575a7445792c9e462a679916bd62fc674ca6","unresolved":false,"context_lines":[{"line_number":10,"context_line":"and sharding_initiated(), they query the number of shard ranges of"},{"line_number":11,"context_line":"this container by pulling out all shard ranges from shard range table,"},{"line_number":12,"context_line":"instantiating ShardRange objects and then counting how many they are."},{"line_number":13,"context_line":"Those operations are very expansive, and causing HEAD/GET into large"},{"line_number":14,"context_line":"containers to be very slow."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Instead, this patch only checks if there is any child shard range"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"5724a926_8130556a","line":13,"in_reply_to":"6e14de5f_3f071a85","updated":"2023-07-19 18:03:58.000000000","message":"Done","commit_id":"7e1fc226487cdf8ab285bc689961e10efee956bc"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"bd3f9d0fc348fe79108f338db355220edb32b61d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"7dfb547b_384cd978","updated":"2023-07-13 13:52:55.000000000","message":"I measure a 10x-15x improvement in get_db_state timing with this patch.\n\nHowever, I get ~1000x improvement by using a LIMIT on the existing get_shard_ranges  to (a) optimise when we only care \"are there *any* shard ranges, but also (b) optimise get_own_shard_range()\n\n```\ndiff --git a/swift/container/backend.py b/swift/container/backend.py\nindex 6cbee63e8..7e77f4f35 100644\n--- a/swift/container/backend.py\n+++ b/swift/container/backend.py\n@@ -446,7 +446,7 @@ class ContainerBroker(DatabaseBroker):\n             return UNSHARDED\n         if self.db_epoch !\u003d self.get_own_shard_range().epoch:\n             return UNSHARDED\n-        if not self.get_shard_ranges():\n+        if not self.get_shard_ranges(limit\u003d1):\n             return COLLAPSED\n         return SHARDED\n\n@@ -1687,7 +1687,8 @@ class ContainerBroker(DatabaseBroker):\n\n     def _get_shard_range_rows(self, connection\u003dNone, includes\u003dNone,\n                               include_deleted\u003dFalse, states\u003dNone,\n-                              include_own\u003dFalse, exclude_others\u003dFalse):\n+                              include_own\u003dFalse, exclude_others\u003dFalse,\n+                              limit\u003dNone):\n         \"\"\"\n         Returns a list of shard range rows.\n\n@@ -1746,6 +1747,8 @@ class ContainerBroker(DatabaseBroker):\n                 params.extend((includes, includes))\n             if conditions:\n                 condition \u003d \u0027 WHERE \u0027 + \u0027 AND \u0027.join(conditions)\n+            if limit:\n+                condition +\u003d \u0027 LIMIT %d\u0027 % limit\n             columns \u003d SHARD_RANGE_KEYS[:-2]\n             for column in SHARD_RANGE_KEYS[-2:]:\n                 if column in defaults:\n@@ -1820,7 +1823,7 @@ class ContainerBroker(DatabaseBroker):\n     def get_shard_ranges(self, marker\u003dNone, end_marker\u003dNone, includes\u003dNone,\n                          reverse\u003dFalse, include_deleted\u003dFalse, states\u003dNone,\n                          include_own\u003dFalse,\n-                         exclude_others\u003dFalse, fill_gaps\u003dFalse):\n+                         exclude_others\u003dFalse, fill_gaps\u003dFalse, limit\u003dNone):\n         \"\"\"\n         Returns a list of persisted shard ranges.\n\n@@ -1860,7 +1863,7 @@ class ContainerBroker(DatabaseBroker):\n             for row in self._get_shard_range_rows(\n                 includes\u003dincludes, include_deleted\u003dinclude_deleted,\n                 states\u003dstates, include_own\u003dinclude_own,\n-                exclude_others\u003dexclude_others)]\n+                exclude_others\u003dexclude_others, limit\u003dlimit)]\n\n         shard_ranges.sort(key\u003dShardRange.sort_key)\n         if includes:\n@@ -1907,7 +1910,8 @@ class ContainerBroker(DatabaseBroker):\n         \"\"\"\n         shard_ranges \u003d self.get_shard_ranges(include_own\u003dTrue,\n                                              include_deleted\u003dTrue,\n-                                             exclude_others\u003dTrue)\n+                                             exclude_others\u003dTrue,\n+                                             limit\u003d1)\n         if shard_ranges:\n             own_shard_range \u003d shard_ranges[0]\n         elif no_default:\n```","commit_id":"68fa2720ad3c747143b52ea861bd288e6699dde8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3e1bd69de64753e278374bf0a7c83300ccc30095","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"908064cd_d462fc99","updated":"2023-07-13 13:59:29.000000000","message":"some of the unit test failures are because get_shards_count() is being called before the DB has been migrated to create a shard table (see the try/except in _get_shard_range_rows) - similar will need to be implemented for get_shards_count(), but the alternative of using a LIMIT avoids this extra code.","commit_id":"68fa2720ad3c747143b52ea861bd288e6699dde8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"5c8c2a023e23a268b46f693093a53afa4db04beb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c3473c04_70584e17","updated":"2023-07-14 13:41:26.000000000","message":"you may (or may not) like to build on this patch (adds the limit arg for get_shard_ranges) 888575: container-server: use LIMIT in get_own_shard_range() query | https://review.opendev.org/c/openstack/swift/+/888575","commit_id":"68fa2720ad3c747143b52ea861bd288e6699dde8"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"45025faa273e07f5cceaccd6920c0f684a5e16f1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c28fb395_1dd15e5f","in_reply_to":"7dfb547b_384cd978","updated":"2023-07-13 15:45:28.000000000","message":"wow, thanks! I also wanted to just query sequence id in SQLITE_SEQUENCE.seq, which should be also much faster. will try benchmarking it.","commit_id":"68fa2720ad3c747143b52ea861bd288e6699dde8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f5484323ca66d2dbdfc60352043f8fa57ae0936e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"404c4cbb_101be1b0","updated":"2023-07-17 17:01:29.000000000","message":"i guess there\u0027s no new behavior change here - so maybe we don\u0027t need tests.  Maybe we could do something with a CountingShardRage mock, but that\u0027s a little on the nose - the point is: \"same behavior, but faster\" which we can verify empirically with review.","commit_id":"606e8d4a0354e1d4291789805f49b35617b93e37"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"8d4fda58b73accbfa1958acf3314dacc775c0b92","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"d4c29355_efb8b98d","updated":"2023-07-18 13:56:57.000000000","message":"FWIW i like the query and method name","commit_id":"7c18305369bcde6ebdbd69168c1dcc954dfcfed0"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"4e7035398473cbdf6ef1f23efdc8301e81f43c27","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"5d7ebc7c_92052917","updated":"2023-07-18 09:18:47.000000000","message":"Nice one Jianjian, now we just need some tests!","commit_id":"7c18305369bcde6ebdbd69168c1dcc954dfcfed0"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"673db9f1bbe8b23393034ab861587d461797bd27","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"87c49ff4_0516733b","updated":"2023-07-18 05:09:40.000000000","message":"thanks for the review!","commit_id":"7c18305369bcde6ebdbd69168c1dcc954dfcfed0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"19da7b5b06a3520c857a6c1424f6c5dffaba99d3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"2f6da5a7_f123e348","updated":"2023-07-19 13:27:51.000000000","message":"This is great! I measure get_db_state() is 35x faster with this patch vs master for a DB with ~12000 shard ranges.","commit_id":"7e1fc226487cdf8ab285bc689961e10efee956bc"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"26dd8e3dff298a3dfa616803abfee09f451c6497","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"548a4c04_fdf2ef8b","updated":"2023-07-18 21:10:25.000000000","message":"thanks for the reviews!","commit_id":"7e1fc226487cdf8ab285bc689961e10efee956bc"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"a50b575a7445792c9e462a679916bd62fc674ca6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"665d5139_4132d23a","updated":"2023-07-19 18:03:58.000000000","message":"thanks for the reviews!","commit_id":"7e1fc226487cdf8ab285bc689961e10efee956bc"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"82473c96ac6f21ab9e5b6c549a709258497ffd8e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"ee3b2e8a_56c4b150","updated":"2023-07-20 09:39:20.000000000","message":"Thanks @Jianjian. LGTM.","commit_id":"cbabb7cf9c35a8723d8eb3a26993d21fcf20643f"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"95281a32815325754c790708ac87974a435907ed","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"0c94a642_74f000bf","updated":"2023-07-20 17:46:57.000000000","message":"Thanks a lot for the reviews!","commit_id":"bfbe8f909ff502b4c2cd60a17301602f6fcb15e1"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"be38e03bb3a9768c867ec7c47774d4bd2e524637","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"e43a8df4_1ef45d45","updated":"2023-07-21 08:56:51.000000000","message":"recheck\n\nPOST FAILURE\nCoverage report was not found even though tests succeeded","commit_id":"bfbe8f909ff502b4c2cd60a17301602f6fcb15e1"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"767daf2f4bd92a402ae2862c8e763b24e929acf0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"2b66bfaf_7acdce62","updated":"2023-07-20 19:37:16.000000000","message":"recheck\nSocket timed-out issues from grenade.","commit_id":"bfbe8f909ff502b4c2cd60a17301602f6fcb15e1"}],"swift/container/backend.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"bd3f9d0fc348fe79108f338db355220edb32b61d","unresolved":true,"context_lines":[{"line_number":444,"context_line":"        if self.db_epoch is None:"},{"line_number":445,"context_line":"            # never been sharded"},{"line_number":446,"context_line":"            return UNSHARDED"},{"line_number":447,"context_line":"        if self.db_epoch !\u003d self.get_own_shard_range().epoch:"},{"line_number":448,"context_line":"            return UNSHARDED"},{"line_number":449,"context_line":"        if not self.get_shards_count():"},{"line_number":450,"context_line":"            return COLLAPSED"}],"source_content_type":"text/x-python","patch_set":1,"id":"de9d1231_b793f4c5","line":447,"range":{"start_line":447,"start_character":33,"end_line":447,"end_character":52},"updated":"2023-07-13 13:52:55.000000000","message":"this call is also expensive - we only expect one row to be found but the sql query is not constrained by a limit - I found that adding \"LIMIT 1\" made this much faster","commit_id":"68fa2720ad3c747143b52ea861bd288e6699dde8"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"0b9795905f403ec76a4f3aa661f7e13683ba934c","unresolved":false,"context_lines":[{"line_number":444,"context_line":"        if self.db_epoch is None:"},{"line_number":445,"context_line":"            # never been sharded"},{"line_number":446,"context_line":"            return UNSHARDED"},{"line_number":447,"context_line":"        if self.db_epoch !\u003d self.get_own_shard_range().epoch:"},{"line_number":448,"context_line":"            return UNSHARDED"},{"line_number":449,"context_line":"        if not self.get_shards_count():"},{"line_number":450,"context_line":"            return COLLAPSED"}],"source_content_type":"text/x-python","patch_set":1,"id":"e2566719_8697a032","line":447,"range":{"start_line":447,"start_character":33,"end_line":447,"end_character":52},"in_reply_to":"de9d1231_b793f4c5","updated":"2023-07-17 06:15:08.000000000","message":"Ack","commit_id":"68fa2720ad3c747143b52ea861bd288e6699dde8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"bd3f9d0fc348fe79108f338db355220edb32b61d","unresolved":true,"context_lines":[{"line_number":1958,"context_line":""},{"line_number":1959,"context_line":"    def get_shards_count(self):"},{"line_number":1960,"context_line":"        \"\"\""},{"line_number":1961,"context_line":"        Get total number of shard ranges in states ACTIVE,"},{"line_number":1962,"context_line":"        SHARDING or SHRINKING."},{"line_number":1963,"context_line":""},{"line_number":1964,"context_line":"        :return: number of shard ranges"},{"line_number":1965,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"c9cc27bc_a63c8bd3","line":1962,"range":{"start_line":1961,"start_character":51,"end_line":1962,"end_character":29},"updated":"2023-07-13 13:52:55.000000000","message":"this is different from the current query: by default get_shard_ranges() returns all states, and I think that is what we need when determining db state","commit_id":"68fa2720ad3c747143b52ea861bd288e6699dde8"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"0b9795905f403ec76a4f3aa661f7e13683ba934c","unresolved":false,"context_lines":[{"line_number":1958,"context_line":""},{"line_number":1959,"context_line":"    def get_shards_count(self):"},{"line_number":1960,"context_line":"        \"\"\""},{"line_number":1961,"context_line":"        Get total number of shard ranges in states ACTIVE,"},{"line_number":1962,"context_line":"        SHARDING or SHRINKING."},{"line_number":1963,"context_line":""},{"line_number":1964,"context_line":"        :return: number of shard ranges"},{"line_number":1965,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"0809cad8_3c7ff7e4","line":1962,"range":{"start_line":1961,"start_character":51,"end_line":1962,"end_character":29},"in_reply_to":"c9cc27bc_a63c8bd3","updated":"2023-07-17 06:15:08.000000000","message":"Ack","commit_id":"68fa2720ad3c747143b52ea861bd288e6699dde8"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"6014766882aeb051107a2570bf19d6499aba5c18","unresolved":true,"context_lines":[{"line_number":1959,"context_line":"    def get_shards_count(self):"},{"line_number":1960,"context_line":"        \"\"\""},{"line_number":1961,"context_line":"        Get total number of shard ranges in states ACTIVE,"},{"line_number":1962,"context_line":"        SHARDING or SHRINKING."},{"line_number":1963,"context_line":""},{"line_number":1964,"context_line":"        :return: number of shard ranges"},{"line_number":1965,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"d79f286f_98024a12","line":1962,"updated":"2023-07-14 23:47:08.000000000","message":"will fix comments and add try/except to handle the case of no SHARD_RANGE_TABLE.","commit_id":"f86dfb6cc12a64ebabf2156921c421ad7156cd94"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"0b9795905f403ec76a4f3aa661f7e13683ba934c","unresolved":false,"context_lines":[{"line_number":1959,"context_line":"    def get_shards_count(self):"},{"line_number":1960,"context_line":"        \"\"\""},{"line_number":1961,"context_line":"        Get total number of shard ranges in states ACTIVE,"},{"line_number":1962,"context_line":"        SHARDING or SHRINKING."},{"line_number":1963,"context_line":""},{"line_number":1964,"context_line":"        :return: number of shard ranges"},{"line_number":1965,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"e4a3ed3b_5589e320","line":1962,"in_reply_to":"d79f286f_98024a12","updated":"2023-07-17 06:15:08.000000000","message":"Ack","commit_id":"f86dfb6cc12a64ebabf2156921c421ad7156cd94"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f5484323ca66d2dbdfc60352043f8fa57ae0936e","unresolved":true,"context_lines":[{"line_number":177,"context_line":"    END;"},{"line_number":178,"context_line":"\u0027\u0027\u0027"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"DB_QUERY_MAX_RETRIES \u003d 3"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"def update_new_item_from_existing(new_item, existing):"}],"source_content_type":"text/x-python","patch_set":3,"id":"76cd39ac_6c6992f5","line":180,"updated":"2023-07-17 17:01:29.000000000","message":"is this left over?","commit_id":"606e8d4a0354e1d4291789805f49b35617b93e37"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"673db9f1bbe8b23393034ab861587d461797bd27","unresolved":false,"context_lines":[{"line_number":177,"context_line":"    END;"},{"line_number":178,"context_line":"\u0027\u0027\u0027"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"DB_QUERY_MAX_RETRIES \u003d 3"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"def update_new_item_from_existing(new_item, existing):"}],"source_content_type":"text/x-python","patch_set":3,"id":"0093a8cb_f4591a3b","line":180,"in_reply_to":"76cd39ac_6c6992f5","updated":"2023-07-18 05:09:40.000000000","message":"Ack","commit_id":"606e8d4a0354e1d4291789805f49b35617b93e37"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"0b9795905f403ec76a4f3aa661f7e13683ba934c","unresolved":true,"context_lines":[{"line_number":1980,"context_line":"        :return: True if there is any child shard range exists; False"},{"line_number":1981,"context_line":"            otherwise."},{"line_number":1982,"context_line":"        \"\"\""},{"line_number":1983,"context_line":"        return True if self.get_shard_ranges(limit\u003d1) else False"},{"line_number":1984,"context_line":""},{"line_number":1985,"context_line":"    def get_all_shard_range_data(self):"},{"line_number":1986,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"683b58ef_19e56433","line":1983,"updated":"2023-07-17 06:15:08.000000000","message":"previous patch used sqlite sequence ID, which works well and faster than this, but \"collapsed\" test cases failed. Even though production doesn\u0027t shrink shard range yet, we need support future possible usages. Another alternative is to use sqlite \"EXISTS\" keyword which could be a little faster, but we need duplicating some sql query implementations and try/except logic.","commit_id":"606e8d4a0354e1d4291789805f49b35617b93e37"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"673db9f1bbe8b23393034ab861587d461797bd27","unresolved":false,"context_lines":[{"line_number":1980,"context_line":"        :return: True if there is any child shard range exists; False"},{"line_number":1981,"context_line":"            otherwise."},{"line_number":1982,"context_line":"        \"\"\""},{"line_number":1983,"context_line":"        return True if self.get_shard_ranges(limit\u003d1) else False"},{"line_number":1984,"context_line":""},{"line_number":1985,"context_line":"    def get_all_shard_range_data(self):"},{"line_number":1986,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"466c6952_d3ef2887","line":1983,"in_reply_to":"683b58ef_19e56433","updated":"2023-07-18 05:09:40.000000000","message":"Tested 5 SQL queries, selected the most performant in the latest patch.","commit_id":"606e8d4a0354e1d4291789805f49b35617b93e37"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"8d4fda58b73accbfa1958acf3314dacc775c0b92","unresolved":true,"context_lines":[{"line_number":868,"context_line":""},{"line_number":869,"context_line":"    def is_empty_enough_to_reclaim(self):"},{"line_number":870,"context_line":"        if self.is_root_container() and (self.child_shard_ranges_exist() or"},{"line_number":871,"context_line":"                                         self.get_db_state() \u003d\u003d SHARDING):"},{"line_number":872,"context_line":"            return False"},{"line_number":873,"context_line":"        return self.empty()"},{"line_number":874,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9b02519a_0f426bde","line":871,"updated":"2023-07-18 13:56:57.000000000","message":"since get_db_state can call child_shard_ranges_exist it looks redundant somehow maybe","commit_id":"7c18305369bcde6ebdbd69168c1dcc954dfcfed0"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"26dd8e3dff298a3dfa616803abfee09f451c6497","unresolved":false,"context_lines":[{"line_number":868,"context_line":""},{"line_number":869,"context_line":"    def is_empty_enough_to_reclaim(self):"},{"line_number":870,"context_line":"        if self.is_root_container() and (self.child_shard_ranges_exist() or"},{"line_number":871,"context_line":"                                         self.get_db_state() \u003d\u003d SHARDING):"},{"line_number":872,"context_line":"            return False"},{"line_number":873,"context_line":"        return self.empty()"},{"line_number":874,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9a5ad5b3_c5f25dd3","line":871,"in_reply_to":"9b02519a_0f426bde","updated":"2023-07-18 21:10:25.000000000","message":"Two conditions are different. If state is SHARDING, get_db_state() will only check number of files and won\u0027t call child_shard_ranges_exist() internally.","commit_id":"7c18305369bcde6ebdbd69168c1dcc954dfcfed0"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"673db9f1bbe8b23393034ab861587d461797bd27","unresolved":true,"context_lines":[{"line_number":1956,"context_line":"        return {\u0027bytes_used\u0027: bytes_used,"},{"line_number":1957,"context_line":"                \u0027object_count\u0027: object_count}"},{"line_number":1958,"context_line":""},{"line_number":1959,"context_line":"    def child_shard_ranges_exist(self):"},{"line_number":1960,"context_line":"        \"\"\""},{"line_number":1961,"context_line":"        This function tells if there is any child shard range exists."},{"line_number":1962,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"1ca1c856_4262f5d0","line":1959,"updated":"2023-07-18 05:09:40.000000000","message":"although there are many test cases testing this indirectly, I am adding unit tests just for this function.","commit_id":"7c18305369bcde6ebdbd69168c1dcc954dfcfed0"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"26dd8e3dff298a3dfa616803abfee09f451c6497","unresolved":false,"context_lines":[{"line_number":1956,"context_line":"        return {\u0027bytes_used\u0027: bytes_used,"},{"line_number":1957,"context_line":"                \u0027object_count\u0027: object_count}"},{"line_number":1958,"context_line":""},{"line_number":1959,"context_line":"    def child_shard_ranges_exist(self):"},{"line_number":1960,"context_line":"        \"\"\""},{"line_number":1961,"context_line":"        This function tells if there is any child shard range exists."},{"line_number":1962,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"0cdde756_a051fbb2","line":1959,"in_reply_to":"1ca1c856_4262f5d0","updated":"2023-07-18 21:10:25.000000000","message":"Done","commit_id":"7c18305369bcde6ebdbd69168c1dcc954dfcfed0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"29bc41c7e2e1728777a2b32bb3fd2ebe30ca069d","unresolved":true,"context_lines":[{"line_number":1956,"context_line":"        return {\u0027bytes_used\u0027: bytes_used,"},{"line_number":1957,"context_line":"                \u0027object_count\u0027: object_count}"},{"line_number":1958,"context_line":""},{"line_number":1959,"context_line":"    def child_shard_ranges_exist(self):"},{"line_number":1960,"context_line":"        \"\"\""},{"line_number":1961,"context_line":"        This function tells if there is any child shard range exists."},{"line_number":1962,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"fcdbab4d_f5c47094","line":1959,"in_reply_to":"1ca1c856_4262f5d0","updated":"2023-07-19 08:54:29.000000000","message":"Not all shard ranges are \"children\", especially in the case of shards (vs roots). When a shard shrinks it actually gets \"sibling\" or \"neighbouring\" shard ranges. \n\nI feel like I\u0027m being pedantic but given that we have a strict definition of \"child\" enshrined in ShardRange.is_child_of(), there\u0027s a risk that this method name is mistakenly interpreted as exactly that: \"is there any shard range that satisfies is_child_of(own_shard_range)\"\n\n\"other_shard_ranges\" is an alternative that I would be comfortable with, although I have to confess in sharder.py we use other_shard_ranges for those shard ranges that are neither \"own\" nor \"child\" !! \n\nThe rest is a *nit*: I\u0027m more familiar with the is_/has_ naming convention for boolean methods (https://rules.sonarsource.com/java/tag/convention/RSPEC-2047/), and there is a tiny bit of precedent in the class with has_multiple_policies.\n\nBoth comments together lead me to humbly suggest \"has_other_shard_ranges\", but the \"other vs child\" change is most significant.","commit_id":"7c18305369bcde6ebdbd69168c1dcc954dfcfed0"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"a50b575a7445792c9e462a679916bd62fc674ca6","unresolved":false,"context_lines":[{"line_number":1956,"context_line":"        return {\u0027bytes_used\u0027: bytes_used,"},{"line_number":1957,"context_line":"                \u0027object_count\u0027: object_count}"},{"line_number":1958,"context_line":""},{"line_number":1959,"context_line":"    def child_shard_ranges_exist(self):"},{"line_number":1960,"context_line":"        \"\"\""},{"line_number":1961,"context_line":"        This function tells if there is any child shard range exists."},{"line_number":1962,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"a1de75a9_9e1b7a02","line":1959,"in_reply_to":"fcdbab4d_f5c47094","updated":"2023-07-19 18:03:58.000000000","message":"Good suggestion, even though some shard ranges are within this own_shard_range, they are not necessary to be its \"child\", \"has_other_shard_ranges\" is better!","commit_id":"7c18305369bcde6ebdbd69168c1dcc954dfcfed0"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"4e7035398473cbdf6ef1f23efdc8301e81f43c27","unresolved":true,"context_lines":[{"line_number":1965,"context_line":"        \"\"\""},{"line_number":1966,"context_line":"        with self.get() as conn:"},{"line_number":1967,"context_line":"            sql \u003d \u0027\u0027\u0027"},{"line_number":1968,"context_line":"            SELECT 1 FROM %s"},{"line_number":1969,"context_line":"            WHERE deleted \u003d 0 AND name !\u003d ? LIMIT 1"},{"line_number":1970,"context_line":"            \u0027\u0027\u0027 % (SHARD_RANGE_TABLE)"},{"line_number":1971,"context_line":"            try:"},{"line_number":1972,"context_line":"                data \u003d conn.execute(sql, [self.path])"}],"source_content_type":"text/x-python","patch_set":4,"id":"7b613f9a_b0b5309f","line":1969,"range":{"start_line":1968,"start_character":12,"end_line":1969,"end_character":51},"updated":"2023-07-18 09:18:47.000000000","message":"Nice!! is if !\u003d specidically fast or is the LIMIT 1 that makes it fast because it\u0027ll stop searching at the first occurence?\nI would\u0027ve thought just a count() and \u0027a \u003e 1\u0027 would be enough. But if \u0027LIMIT 1\u0027, speeds it or rather breaks out faster then that\u0027s pretty sweet!","commit_id":"7c18305369bcde6ebdbd69168c1dcc954dfcfed0"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"26dd8e3dff298a3dfa616803abfee09f451c6497","unresolved":false,"context_lines":[{"line_number":1965,"context_line":"        \"\"\""},{"line_number":1966,"context_line":"        with self.get() as conn:"},{"line_number":1967,"context_line":"            sql \u003d \u0027\u0027\u0027"},{"line_number":1968,"context_line":"            SELECT 1 FROM %s"},{"line_number":1969,"context_line":"            WHERE deleted \u003d 0 AND name !\u003d ? LIMIT 1"},{"line_number":1970,"context_line":"            \u0027\u0027\u0027 % (SHARD_RANGE_TABLE)"},{"line_number":1971,"context_line":"            try:"},{"line_number":1972,"context_line":"                data \u003d conn.execute(sql, [self.path])"}],"source_content_type":"text/x-python","patch_set":4,"id":"3ebaf93e_92552258","line":1969,"range":{"start_line":1968,"start_character":12,"end_line":1969,"end_character":51},"in_reply_to":"7b613f9a_b0b5309f","updated":"2023-07-18 21:10:25.000000000","message":"Yes, \"LIMIT 1\" will stop searching once sqlite finds a qualified shard range, which contributes to the performance improvement; another big factor is that this change will skip all the shard range object instantiations which are very expansive as well.","commit_id":"7c18305369bcde6ebdbd69168c1dcc954dfcfed0"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"8d4fda58b73accbfa1958acf3314dacc775c0b92","unresolved":true,"context_lines":[{"line_number":1974,"context_line":"                return True if [row for row in data] else False"},{"line_number":1975,"context_line":"            except sqlite3.OperationalError as err:"},{"line_number":1976,"context_line":"                if (\u0027no such table: %s\u0027 % SHARD_RANGE_TABLE) in str(err):"},{"line_number":1977,"context_line":"                    return False"},{"line_number":1978,"context_line":"                else:"},{"line_number":1979,"context_line":"                    raise"},{"line_number":1980,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"a4ccd68d_3717b096","line":1977,"updated":"2023-07-18 13:56:57.000000000","message":"seems legit","commit_id":"7c18305369bcde6ebdbd69168c1dcc954dfcfed0"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"26dd8e3dff298a3dfa616803abfee09f451c6497","unresolved":false,"context_lines":[{"line_number":1974,"context_line":"                return True if [row for row in data] else False"},{"line_number":1975,"context_line":"            except sqlite3.OperationalError as err:"},{"line_number":1976,"context_line":"                if (\u0027no such table: %s\u0027 % SHARD_RANGE_TABLE) in str(err):"},{"line_number":1977,"context_line":"                    return False"},{"line_number":1978,"context_line":"                else:"},{"line_number":1979,"context_line":"                    raise"},{"line_number":1980,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ce6d65bb_bf3bfd4d","line":1977,"in_reply_to":"a4ccd68d_3717b096","updated":"2023-07-18 21:10:25.000000000","message":"this is old behavior as well: get_shard_ranges() will return [] if SHARD_RANGE_TABLE doesn\u0027t exist yet. I also added new test case to cover this.","commit_id":"7c18305369bcde6ebdbd69168c1dcc954dfcfed0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"29bc41c7e2e1728777a2b32bb3fd2ebe30ca069d","unresolved":true,"context_lines":[{"line_number":1958,"context_line":""},{"line_number":1959,"context_line":"    def child_shard_ranges_exist(self):"},{"line_number":1960,"context_line":"        \"\"\""},{"line_number":1961,"context_line":"        This function tells if there is any child shard range exists."},{"line_number":1962,"context_line":""},{"line_number":1963,"context_line":"        :return: True if there is any child shard range exists; False"},{"line_number":1964,"context_line":"            otherwise."}],"source_content_type":"text/x-python","patch_set":5,"id":"a883bcb6_f2e61fbb","line":1961,"range":{"start_line":1961,"start_character":40,"end_line":1961,"end_character":61},"updated":"2023-07-19 08:54:29.000000000","message":"even if the method name remains as it is, the docstring should definitely clarify \n\n\"if there is any shard range other than the broker\u0027s own shard range, that is not marked as deleted\"","commit_id":"7e1fc226487cdf8ab285bc689961e10efee956bc"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"a50b575a7445792c9e462a679916bd62fc674ca6","unresolved":false,"context_lines":[{"line_number":1958,"context_line":""},{"line_number":1959,"context_line":"    def child_shard_ranges_exist(self):"},{"line_number":1960,"context_line":"        \"\"\""},{"line_number":1961,"context_line":"        This function tells if there is any child shard range exists."},{"line_number":1962,"context_line":""},{"line_number":1963,"context_line":"        :return: True if there is any child shard range exists; False"},{"line_number":1964,"context_line":"            otherwise."}],"source_content_type":"text/x-python","patch_set":5,"id":"327b25cf_0f8a48ea","line":1961,"range":{"start_line":1961,"start_character":40,"end_line":1961,"end_character":61},"in_reply_to":"a883bcb6_f2e61fbb","updated":"2023-07-19 18:03:58.000000000","message":"Done","commit_id":"7e1fc226487cdf8ab285bc689961e10efee956bc"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"19da7b5b06a3520c857a6c1424f6c5dffaba99d3","unresolved":true,"context_lines":[{"line_number":1971,"context_line":"            try:"},{"line_number":1972,"context_line":"                data \u003d conn.execute(sql, [self.path])"},{"line_number":1973,"context_line":"                data.row_factory \u003d None"},{"line_number":1974,"context_line":"                return True if [row for row in data] else False"},{"line_number":1975,"context_line":"            except sqlite3.OperationalError as err:"},{"line_number":1976,"context_line":"                if (\u0027no such table: %s\u0027 % SHARD_RANGE_TABLE) in str(err):"},{"line_number":1977,"context_line":"                    return False"}],"source_content_type":"text/x-python","patch_set":5,"id":"ab017a7e_98623a61","line":1974,"range":{"start_line":1974,"start_character":31,"end_line":1974,"end_character":52},"updated":"2023-07-19 13:27:51.000000000","message":"I had wondered if it would be faster to avoid creating the list but I could not measure any improvement when using:\n\n```\n                try:\n                    next(data)\n                except StopIteration:\n                    return False\n                else:\n                    return True\n```","commit_id":"7e1fc226487cdf8ab285bc689961e10efee956bc"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"ec7a7ea290a0634bf1360bd1bfbecebcc096a1cb","unresolved":false,"context_lines":[{"line_number":1971,"context_line":"            try:"},{"line_number":1972,"context_line":"                data \u003d conn.execute(sql, [self.path])"},{"line_number":1973,"context_line":"                data.row_factory \u003d None"},{"line_number":1974,"context_line":"                return True if [row for row in data] else False"},{"line_number":1975,"context_line":"            except sqlite3.OperationalError as err:"},{"line_number":1976,"context_line":"                if (\u0027no such table: %s\u0027 % SHARD_RANGE_TABLE) in str(err):"},{"line_number":1977,"context_line":"                    return False"}],"source_content_type":"text/x-python","patch_set":5,"id":"b94f7c39_6d016f0f","line":1974,"range":{"start_line":1974,"start_character":31,"end_line":1974,"end_character":52},"in_reply_to":"ab017a7e_98623a61","updated":"2023-07-20 09:38:39.000000000","message":"Ack","commit_id":"7e1fc226487cdf8ab285bc689961e10efee956bc"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"95281a32815325754c790708ac87974a435907ed","unresolved":false,"context_lines":[{"line_number":1961,"context_line":"        This function tells if there is any shard range other than the"},{"line_number":1962,"context_line":"        broker\u0027s own shard range, that is not marked as deleted."},{"line_number":1963,"context_line":""},{"line_number":1964,"context_line":"        :return: A boolean value as described above."},{"line_number":1965,"context_line":"        \"\"\""},{"line_number":1966,"context_line":"        with self.get() as conn:"},{"line_number":1967,"context_line":"            sql \u003d \u0027\u0027\u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"17525307_d043c002","line":1964,"updated":"2023-07-20 17:46:57.000000000","message":"I accidentally removed this docstring change from Alistair when updating patch commit message, now bring it back.","commit_id":"bfbe8f909ff502b4c2cd60a17301602f6fcb15e1"}],"test/unit/container/test_backend.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"19da7b5b06a3520c857a6c1424f6c5dffaba99d3","unresolved":true,"context_lines":[{"line_number":1741,"context_line":"        broker.initialize(ts.internal, 0)"},{"line_number":1742,"context_line":""},{"line_number":1743,"context_line":"        # Test the case which the \u0027shard_range\u0027 table doesn\u0027t exist yet."},{"line_number":1744,"context_line":"        self._delete_table(broker, \u0027shard_range\u0027)"},{"line_number":1745,"context_line":"        self.assertEqual(broker.child_shard_ranges_exist(), False)"},{"line_number":1746,"context_line":""},{"line_number":1747,"context_line":"        # Add the \u0027shard_range\u0027 table back to the database, but it doesn\u0027t"}],"source_content_type":"text/x-python","patch_set":5,"id":"ad671705_481b4a44","line":1744,"updated":"2023-07-19 13:27:51.000000000","message":"+1 very thorough, thank you!","commit_id":"7e1fc226487cdf8ab285bc689961e10efee956bc"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"ec7a7ea290a0634bf1360bd1bfbecebcc096a1cb","unresolved":false,"context_lines":[{"line_number":1741,"context_line":"        broker.initialize(ts.internal, 0)"},{"line_number":1742,"context_line":""},{"line_number":1743,"context_line":"        # Test the case which the \u0027shard_range\u0027 table doesn\u0027t exist yet."},{"line_number":1744,"context_line":"        self._delete_table(broker, \u0027shard_range\u0027)"},{"line_number":1745,"context_line":"        self.assertEqual(broker.child_shard_ranges_exist(), False)"},{"line_number":1746,"context_line":""},{"line_number":1747,"context_line":"        # Add the \u0027shard_range\u0027 table back to the database, but it doesn\u0027t"}],"source_content_type":"text/x-python","patch_set":5,"id":"36c49e3f_ef4ef965","line":1744,"in_reply_to":"ad671705_481b4a44","updated":"2023-07-20 09:38:39.000000000","message":"Done","commit_id":"7e1fc226487cdf8ab285bc689961e10efee956bc"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"19da7b5b06a3520c857a6c1424f6c5dffaba99d3","unresolved":true,"context_lines":[{"line_number":1742,"context_line":""},{"line_number":1743,"context_line":"        # Test the case which the \u0027shard_range\u0027 table doesn\u0027t exist yet."},{"line_number":1744,"context_line":"        self._delete_table(broker, \u0027shard_range\u0027)"},{"line_number":1745,"context_line":"        self.assertEqual(broker.child_shard_ranges_exist(), False)"},{"line_number":1746,"context_line":""},{"line_number":1747,"context_line":"        # Add the \u0027shard_range\u0027 table back to the database, but it doesn\u0027t"},{"line_number":1748,"context_line":"        # have any shard range row in it yet."}],"source_content_type":"text/x-python","patch_set":5,"id":"53b9d278_5bb47242","line":1745,"range":{"start_line":1745,"start_character":13,"end_line":1745,"end_character":24},"updated":"2023-07-19 13:27:51.000000000","message":"you can use `self.assertFalse(broker.child_shard_ranges_exist())`\n\nor if you want to be sure the returned value is a boolean (rather than just Falsey)\n\n`self.assertIs(False, broker.child_shard_ranges_exist())`\n\ne.g. see assertions in test_sharding_initiated_and_required\n\nsame comment below...","commit_id":"7e1fc226487cdf8ab285bc689961e10efee956bc"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"a50b575a7445792c9e462a679916bd62fc674ca6","unresolved":false,"context_lines":[{"line_number":1742,"context_line":""},{"line_number":1743,"context_line":"        # Test the case which the \u0027shard_range\u0027 table doesn\u0027t exist yet."},{"line_number":1744,"context_line":"        self._delete_table(broker, \u0027shard_range\u0027)"},{"line_number":1745,"context_line":"        self.assertEqual(broker.child_shard_ranges_exist(), False)"},{"line_number":1746,"context_line":""},{"line_number":1747,"context_line":"        # Add the \u0027shard_range\u0027 table back to the database, but it doesn\u0027t"},{"line_number":1748,"context_line":"        # have any shard range row in it yet."}],"source_content_type":"text/x-python","patch_set":5,"id":"ad8f9704_e43b5ca0","line":1745,"range":{"start_line":1745,"start_character":13,"end_line":1745,"end_character":24},"in_reply_to":"53b9d278_5bb47242","updated":"2023-07-19 18:03:58.000000000","message":"Done","commit_id":"7e1fc226487cdf8ab285bc689961e10efee956bc"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"19da7b5b06a3520c857a6c1424f6c5dffaba99d3","unresolved":true,"context_lines":[{"line_number":1756,"context_line":"        own_shard_range \u003d broker.get_own_shard_range()"},{"line_number":1757,"context_line":"        own_shard_range.update_state(ShardRange.SHARDING)"},{"line_number":1758,"context_line":"        own_shard_range.epoch \u003d epoch"},{"line_number":1759,"context_line":"        broker.merge_shard_ranges([own_shard_range])"},{"line_number":1760,"context_line":"        self.assertEqual(broker.child_shard_ranges_exist(), False)"},{"line_number":1761,"context_line":""},{"line_number":1762,"context_line":"        # Insert a child shard range into this test database."}],"source_content_type":"text/x-python","patch_set":5,"id":"ecc405bd_64bec82f","line":1759,"updated":"2023-07-19 13:27:51.000000000","message":"nit: one more sanity check assertion:\n\n```self.assertTrue(broker.get_shard_ranges(include_own\u003dTrue))```","commit_id":"7e1fc226487cdf8ab285bc689961e10efee956bc"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"a50b575a7445792c9e462a679916bd62fc674ca6","unresolved":false,"context_lines":[{"line_number":1756,"context_line":"        own_shard_range \u003d broker.get_own_shard_range()"},{"line_number":1757,"context_line":"        own_shard_range.update_state(ShardRange.SHARDING)"},{"line_number":1758,"context_line":"        own_shard_range.epoch \u003d epoch"},{"line_number":1759,"context_line":"        broker.merge_shard_ranges([own_shard_range])"},{"line_number":1760,"context_line":"        self.assertEqual(broker.child_shard_ranges_exist(), False)"},{"line_number":1761,"context_line":""},{"line_number":1762,"context_line":"        # Insert a child shard range into this test database."}],"source_content_type":"text/x-python","patch_set":5,"id":"ad423715_a67c7239","line":1759,"in_reply_to":"ecc405bd_64bec82f","updated":"2023-07-19 18:03:58.000000000","message":"Done","commit_id":"7e1fc226487cdf8ab285bc689961e10efee956bc"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"19da7b5b06a3520c857a6c1424f6c5dffaba99d3","unresolved":true,"context_lines":[{"line_number":1765,"context_line":"        broker.merge_shard_ranges([first_child_sr])"},{"line_number":1766,"context_line":"        self.assertEqual(broker.child_shard_ranges_exist(), True)"},{"line_number":1767,"context_line":""},{"line_number":1768,"context_line":"        # Markt the first child shard range as deleted."},{"line_number":1769,"context_line":"        first_child_sr.deleted \u003d 1"},{"line_number":1770,"context_line":"        first_child_sr.timestamp \u003d Timestamp.now()"},{"line_number":1771,"context_line":"        broker.merge_shard_ranges([first_child_sr])"}],"source_content_type":"text/x-python","patch_set":5,"id":"b0ccaf5f_e7d628c2","line":1768,"updated":"2023-07-19 13:27:51.000000000","message":"nit: typo","commit_id":"7e1fc226487cdf8ab285bc689961e10efee956bc"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"a50b575a7445792c9e462a679916bd62fc674ca6","unresolved":false,"context_lines":[{"line_number":1765,"context_line":"        broker.merge_shard_ranges([first_child_sr])"},{"line_number":1766,"context_line":"        self.assertEqual(broker.child_shard_ranges_exist(), True)"},{"line_number":1767,"context_line":""},{"line_number":1768,"context_line":"        # Markt the first child shard range as deleted."},{"line_number":1769,"context_line":"        first_child_sr.deleted \u003d 1"},{"line_number":1770,"context_line":"        first_child_sr.timestamp \u003d Timestamp.now()"},{"line_number":1771,"context_line":"        broker.merge_shard_ranges([first_child_sr])"}],"source_content_type":"text/x-python","patch_set":5,"id":"330be7aa_b3362a54","line":1768,"in_reply_to":"b0ccaf5f_e7d628c2","updated":"2023-07-19 18:03:58.000000000","message":"Ack","commit_id":"7e1fc226487cdf8ab285bc689961e10efee956bc"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"19da7b5b06a3520c857a6c1424f6c5dffaba99d3","unresolved":true,"context_lines":[{"line_number":1775,"context_line":"        second_child_sr \u003d ShardRange("},{"line_number":1776,"context_line":"            \u0027.shards_%s/%s_2\u0027 % (acct, cont), Timestamp.now())"},{"line_number":1777,"context_line":"        broker.merge_shard_ranges([second_child_sr])"},{"line_number":1778,"context_line":"        self.assertEqual(broker.child_shard_ranges_exist(), True)"},{"line_number":1779,"context_line":""},{"line_number":1780,"context_line":"    @with_tempdir"},{"line_number":1781,"context_line":"    def test_get_db_state(self, tempdir):"}],"source_content_type":"text/x-python","patch_set":5,"id":"52a3e406_68d53672","line":1778,"updated":"2023-07-19 13:27:51.000000000","message":"nit: a further case might be to delete own_shard_range and assert child_shard_ranges_exist() is still True - i.e. verify that the underlying condition is not \"there are \u003e\u003d2 shard ranges\"","commit_id":"7e1fc226487cdf8ab285bc689961e10efee956bc"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"a50b575a7445792c9e462a679916bd62fc674ca6","unresolved":false,"context_lines":[{"line_number":1775,"context_line":"        second_child_sr \u003d ShardRange("},{"line_number":1776,"context_line":"            \u0027.shards_%s/%s_2\u0027 % (acct, cont), Timestamp.now())"},{"line_number":1777,"context_line":"        broker.merge_shard_ranges([second_child_sr])"},{"line_number":1778,"context_line":"        self.assertEqual(broker.child_shard_ranges_exist(), True)"},{"line_number":1779,"context_line":""},{"line_number":1780,"context_line":"    @with_tempdir"},{"line_number":1781,"context_line":"    def test_get_db_state(self, tempdir):"}],"source_content_type":"text/x-python","patch_set":5,"id":"c007a0c9_b1b1574b","line":1778,"in_reply_to":"52a3e406_68d53672","updated":"2023-07-19 18:03:58.000000000","message":"Done","commit_id":"7e1fc226487cdf8ab285bc689961e10efee956bc"}]}