)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"2c4881730e091f12b155272dd2f5f9c575240971","unresolved":true,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"A ring method `get_last_part_node(part)` now exists which will return"},{"line_number":23,"context_line":"the last primary node for a part or None if there wasn\u0027t any. The latter"},{"line_number":24,"context_line":"can occur if a device is removed or a part didn\u0027t move during the last"},{"line_number":25,"context_line":"rebalance."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"This patch also adds a \u0027for_read\u0027 optional parameter for get_more_nodes"},{"line_number":28,"context_line":"which when true will add the last primary to the front of the handoff"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"f09803d5_cb39f19d","line":25,"range":{"start_line":24,"start_character":36,"end_line":25,"end_character":9},"updated":"2021-08-03 23:33:49.000000000","message":"So we only get one rebalance cycle with the look-back? I should check what kind of threshold we use for when to kick off another rebalance... I was debating about storing it more like an array of (dev_id, ring_version, maybe replica/frag#?) tuples or something.\n\n*shrug* This is better than what we\u0027ve got currently, anyway.","commit_id":"5610b1f1a524f0bd861d9aee49b58d01bcfa1d12"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"fb395993799ee1623ae5d304c58dbd3d12ef5ab5","unresolved":true,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"A ring method `get_last_part_node(part)` now exists which will return"},{"line_number":23,"context_line":"the last primary node for a part or None if there wasn\u0027t any. The latter"},{"line_number":24,"context_line":"can occur if a device is removed or a part didn\u0027t move during the last"},{"line_number":25,"context_line":"rebalance."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"This patch also adds a \u0027for_read\u0027 optional parameter for get_more_nodes"},{"line_number":28,"context_line":"which when true will add the last primary to the front of the handoff"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5cacfbf9_0339a825","line":25,"range":{"start_line":24,"start_character":36,"end_line":25,"end_character":9},"in_reply_to":"f09803d5_cb39f19d","updated":"2021-08-04 00:31:31.000000000","message":"yeah, it just ended up being nice to add an additional part2dev array to use less space and makes look quick and easy.. and got us something we didn\u0027t have before. But of course always up for other solutions 😊","commit_id":"5610b1f1a524f0bd861d9aee49b58d01bcfa1d12"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"2c4881730e091f12b155272dd2f5f9c575240971","unresolved":true,"context_lines":[{"line_number":24,"context_line":"can occur if a device is removed or a part didn\u0027t move during the last"},{"line_number":25,"context_line":"rebalance."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"This patch also adds a \u0027for_read\u0027 optional parameter for get_more_nodes"},{"line_number":28,"context_line":"which when true will add the last primary to the front of the handoff"},{"line_number":29,"context_line":"list, if there is is one."},{"line_number":30,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"de66bd3d_f04d5e3e","line":27,"range":{"start_line":27,"start_character":24,"end_line":27,"end_character":32},"updated":"2021-08-03 23:33:49.000000000","message":"Nothing actually *uses* this yet, though, right? That\u0027s all in follow-ups.","commit_id":"5610b1f1a524f0bd861d9aee49b58d01bcfa1d12"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"62f5b7e8d691285f1937206f522abf8e14d17b38","unresolved":false,"context_lines":[{"line_number":24,"context_line":"can occur if a device is removed or a part didn\u0027t move during the last"},{"line_number":25,"context_line":"rebalance."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"This patch also adds a \u0027for_read\u0027 optional parameter for get_more_nodes"},{"line_number":28,"context_line":"which when true will add the last primary to the front of the handoff"},{"line_number":29,"context_line":"list, if there is is one."},{"line_number":30,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"67c920a9_939f031f","line":27,"range":{"start_line":27,"start_character":24,"end_line":27,"end_character":32},"in_reply_to":"c7243029_cad2116f","updated":"2021-08-04 18:51:01.000000000","message":"I like having it in this patch -- I\u0027m sure we\u0027ll have good uses for it, and I think the name is reasonable. Just wanted to confirm expectations.","commit_id":"5610b1f1a524f0bd861d9aee49b58d01bcfa1d12"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"fb395993799ee1623ae5d304c58dbd3d12ef5ab5","unresolved":true,"context_lines":[{"line_number":24,"context_line":"can occur if a device is removed or a part didn\u0027t move during the last"},{"line_number":25,"context_line":"rebalance."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"This patch also adds a \u0027for_read\u0027 optional parameter for get_more_nodes"},{"line_number":28,"context_line":"which when true will add the last primary to the front of the handoff"},{"line_number":29,"context_line":"list, if there is is one."},{"line_number":30,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"c7243029_cad2116f","line":27,"range":{"start_line":27,"start_character":24,"end_line":27,"end_character":32},"in_reply_to":"de66bd3d_f04d5e3e","updated":"2021-08-04 00:31:31.000000000","message":"Yeah, this feature is added here but not used until a follow up. Just was a helper to get the all nodes to attempt to read from.\n\nMaybe it sould really be added in a follow up patch if we even end up using it.","commit_id":"5610b1f1a524f0bd861d9aee49b58d01bcfa1d12"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"2c4881730e091f12b155272dd2f5f9c575240971","unresolved":true,"context_lines":[{"line_number":29,"context_line":"list, if there is is one."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"Finally, it also fixes deserialize_v1 to deal with fractional replicas"},{"line_number":32,"context_line":"properly."},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"Change-Id: Iaa66b9fe57c924445a5ede908053e78fc0ed8817"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"bd301196_f50114cd","line":32,"updated":"2021-08-03 23:33:49.000000000","message":"Is the current behavior broken, or just non-obvious?","commit_id":"5610b1f1a524f0bd861d9aee49b58d01bcfa1d12"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"62f5b7e8d691285f1937206f522abf8e14d17b38","unresolved":false,"context_lines":[{"line_number":29,"context_line":"list, if there is is one."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"Finally, it also fixes deserialize_v1 to deal with fractional replicas"},{"line_number":32,"context_line":"properly."},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"Change-Id: Iaa66b9fe57c924445a5ede908053e78fc0ed8817"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"667895e2_397e9a38","line":32,"in_reply_to":"5bbea8a7_cb1206a7","updated":"2021-08-04 18:51:01.000000000","message":"Ack","commit_id":"5610b1f1a524f0bd861d9aee49b58d01bcfa1d12"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"fb395993799ee1623ae5d304c58dbd3d12ef5ab5","unresolved":true,"context_lines":[{"line_number":29,"context_line":"list, if there is is one."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"Finally, it also fixes deserialize_v1 to deal with fractional replicas"},{"line_number":32,"context_line":"properly."},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"Change-Id: Iaa66b9fe57c924445a5ede908053e78fc0ed8817"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5bbea8a7_cb1206a7","line":32,"in_reply_to":"bd301196_f50114cd","updated":"2021-08-04 00:31:31.000000000","message":"We pretty much just always read as full replicas. And this works because an over read only returns whats left.. but this doesn\u0027t work if we decide to write something after the replica2part2dev structure.","commit_id":"5610b1f1a524f0bd861d9aee49b58d01bcfa1d12"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"f0646f655fef58c6866d369095fcfe3516b6d552","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"edbb4a99_469b82e2","updated":"2022-04-27 23:01:19.000000000","message":"There is a newer (and maybe better?) approach to this https://review.opendev.org/c/openstack/swift/+/835001\n\nIt take advantage of also storing the index with the last part table.","commit_id":"5ab3c6aebb8b6e536de64ec646a33ecc79a4efb5"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"2bce4caf43bba76680656ca7fcaaee0a75efe601","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"407f2370_80bc9aa3","updated":"2022-04-25 21:01:37.000000000","message":"We should think about updating rebalance_missing_suppression_count to be smarter about this, too -- basically, we should default to a new \"auto\" mode, where the suppression count is the number of last_primary entries for a part. Might be better as a follow-up, though.","commit_id":"5ab3c6aebb8b6e536de64ec646a33ecc79a4efb5"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"f0646f655fef58c6866d369095fcfe3516b6d552","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"36f43420_aa260524","in_reply_to":"407f2370_80bc9aa3","updated":"2022-04-27 23:01:19.000000000","message":"Yeah, this patch is the old version though. I probably should mention that somewhere. The new approach is currently https://review.opendev.org/c/openstack/swift/+/835001\n\nI also have a wip at then end of that chain that merges the builder with the ring :)","commit_id":"5ab3c6aebb8b6e536de64ec646a33ecc79a4efb5"}],"swift/common/ring/builder.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"bcc9e63b1a321db6f9fa73238b3ae46a50e8d3cb","unresolved":true,"context_lines":[{"line_number":603,"context_line":"                {\u0027status\u0027: finish_status, \u0027count\u0027: gather_count + 1})"},{"line_number":604,"context_line":""},{"line_number":605,"context_line":"        self.devs_changed \u003d False"},{"line_number":606,"context_line":"        changed_parts \u003d self._build_dispersion_graph(old_replica2part2dev)"},{"line_number":607,"context_line":""},{"line_number":608,"context_line":"        # clean up the cache"},{"line_number":609,"context_line":"        for dev in self._iter_devs():"}],"source_content_type":"text/x-python","patch_set":1,"id":"b5444a03_7c3c6fe3","line":606,"updated":"2021-05-11 15:08:07.000000000","message":"this point in rebalance might be intresting... we have the old_replica2part2dev in hand and we\u0027ve done all the reassignments...\n\nchanged_parts is just an int - but it\u0027s come from \"old_device !\u003d dev[\u0027id\u0027]\"","commit_id":"5ae5178abc79890373c02ef4dc5509eaa17fb445"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"1044c73edbb4a6f38759d358ad6caa8b046b817a","unresolved":true,"context_lines":[{"line_number":603,"context_line":"                {\u0027status\u0027: finish_status, \u0027count\u0027: gather_count + 1})"},{"line_number":604,"context_line":""},{"line_number":605,"context_line":"        self.devs_changed \u003d False"},{"line_number":606,"context_line":"        changed_parts \u003d self._build_dispersion_graph(old_replica2part2dev)"},{"line_number":607,"context_line":""},{"line_number":608,"context_line":"        # clean up the cache"},{"line_number":609,"context_line":"        for dev in self._iter_devs():"}],"source_content_type":"text/x-python","patch_set":1,"id":"a252f962_4190698f","line":606,"in_reply_to":"b5444a03_7c3c6fe3","updated":"2021-05-11 23:43:41.000000000","message":"oh that could be a nice place to put it. The only problem was if I do it during the gather I can grab them and ignore grabbing devices that were removed.. because I don\u0027t want to point to an old node that doesn\u0027t exist anymore.... Though I guess I could, and just not return it if it isn\u0027t in the devs, in which case it\u0027s the same thing.. and we may want to have an internal diff of changes at some point... which is what it\u0027ll give us.","commit_id":"5ae5178abc79890373c02ef4dc5509eaa17fb445"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"bcc9e63b1a321db6f9fa73238b3ae46a50e8d3cb","unresolved":true,"context_lines":[{"line_number":1002,"context_line":"                    dev_id \u003d self._replica2part2dev[replica][part]"},{"line_number":1003,"context_line":"                    if dev_id in dev_ids:"},{"line_number":1004,"context_line":"                        self._replica2part2dev[replica][part] \u003d NONE_DEV"},{"line_number":1005,"context_line":"                        self._set_part_moved(part)"},{"line_number":1006,"context_line":"                        assign_parts[part].append(replica)"},{"line_number":1007,"context_line":"                        self.logger.debug("},{"line_number":1008,"context_line":"                            \"Gathered %d/%d from dev %d [dev removed]\","}],"source_content_type":"text/x-python","patch_set":1,"id":"9493c2d9_0d888747","line":1005,"updated":"2021-05-11 15:08:07.000000000","message":"I\u0027m not 100% sure about this idea of trying to \"capture\" the moves during _gather_* methods - it seems like we could easily miss one!","commit_id":"5ae5178abc79890373c02ef4dc5509eaa17fb445"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"1044c73edbb4a6f38759d358ad6caa8b046b817a","unresolved":true,"context_lines":[{"line_number":1002,"context_line":"                    dev_id \u003d self._replica2part2dev[replica][part]"},{"line_number":1003,"context_line":"                    if dev_id in dev_ids:"},{"line_number":1004,"context_line":"                        self._replica2part2dev[replica][part] \u003d NONE_DEV"},{"line_number":1005,"context_line":"                        self._set_part_moved(part)"},{"line_number":1006,"context_line":"                        assign_parts[part].append(replica)"},{"line_number":1007,"context_line":"                        self.logger.debug("},{"line_number":1008,"context_line":"                            \"Gathered %d/%d from dev %d [dev removed]\","}],"source_content_type":"text/x-python","patch_set":1,"id":"396b60c5_cd70157f","line":1005,"in_reply_to":"9493c2d9_0d888747","updated":"2021-05-11 23:43:41.000000000","message":"Oh I thought it was the gathering stages that selected all the parts that needed to move, in which case it was where we had access to the old dev_ids without having to look through anything.","commit_id":"5ae5178abc79890373c02ef4dc5509eaa17fb445"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"f9d496f3b52909e242b942a5acd3f6819030eec3","unresolved":true,"context_lines":[{"line_number":673,"context_line":"                if old_device !\u003d dev[\u0027id\u0027]:"},{"line_number":674,"context_line":"                    changed_parts +\u003d 1"},{"line_number":675,"context_line":"                    if old_replica2part2dev:"},{"line_number":676,"context_line":"                        self._last_part2dev[part_id] \u003d old_device"},{"line_number":677,"context_line":"            # update running totals for each tiers\u0027 number of parts with a"},{"line_number":678,"context_line":"            # given replica count"},{"line_number":679,"context_line":"            part_risk_depth \u003d defaultdict(int)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ce7adc05_9b3da2b7","line":676,"updated":"2021-05-12 21:01:42.000000000","message":"This method can be run without the old_replica2part2dev.. so we only want to store last part devs when it exists.","commit_id":"5d6d68bb645d3f0142d7eb87a76e281f1e49201d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"2c4881730e091f12b155272dd2f5f9c575240971","unresolved":true,"context_lines":[{"line_number":111,"context_line":"        # a while ago, code-wise, when I last tried it)."},{"line_number":112,"context_line":"        self._replica2part2dev \u003d None"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"        # This is another array containing, like a single part2dev that holds"},{"line_number":115,"context_line":"        # the details of the last primary to own a part. This helps us to"},{"line_number":116,"context_line":"        # know what device used to own a part before the last rebalance."},{"line_number":117,"context_line":"        self._last_part2dev \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"0e675173_39f865e3","line":114,"range":{"start_line":114,"start_character":32,"end_line":114,"end_character":42},"updated":"2021-08-03 23:33:49.000000000","message":"\"containing old primaries\" yeah?","commit_id":"5610b1f1a524f0bd861d9aee49b58d01bcfa1d12"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"fb395993799ee1623ae5d304c58dbd3d12ef5ab5","unresolved":true,"context_lines":[{"line_number":111,"context_line":"        # a while ago, code-wise, when I last tried it)."},{"line_number":112,"context_line":"        self._replica2part2dev \u003d None"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"        # This is another array containing, like a single part2dev that holds"},{"line_number":115,"context_line":"        # the details of the last primary to own a part. This helps us to"},{"line_number":116,"context_line":"        # know what device used to own a part before the last rebalance."},{"line_number":117,"context_line":"        self._last_part2dev \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"822f51e6_ff5a7383","line":114,"range":{"start_line":114,"start_character":32,"end_line":114,"end_character":42},"in_reply_to":"0e675173_39f865e3","updated":"2021-08-04 00:31:31.000000000","message":"lol, yeah, will reword it 😊","commit_id":"5610b1f1a524f0bd861d9aee49b58d01bcfa1d12"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"2c4881730e091f12b155272dd2f5f9c575240971","unresolved":true,"context_lines":[{"line_number":663,"context_line":"                for tier in (dev.get(\u0027tiers\u0027) or tiers_for_dev(dev)):"},{"line_number":664,"context_line":"                    replicas_at_tier[tier] +\u003d 1"},{"line_number":665,"context_line":"                # IndexErrors will be raised if the replicas are increased or"},{"line_number":666,"context_line":"                # decreased, and that actually means the partition has changed"},{"line_number":667,"context_line":"                try:"},{"line_number":668,"context_line":"                    old_device \u003d old_replica2part2dev[rep_id][part_id]"},{"line_number":669,"context_line":"                except IndexError:"}],"source_content_type":"text/x-python","patch_set":3,"id":"4735715e_8d8e5639","line":666,"range":{"start_line":666,"start_character":18,"end_line":666,"end_character":27},"updated":"2021-08-03 23:33:49.000000000","message":"IndexError should only get raised when we *increase* the replica count, right? Just making sure we shouldn\u0027t need to update the except block.\n\nAssuming I\u0027ve got that right, where do we record the old primary when we *decrease* replica count?","commit_id":"5610b1f1a524f0bd861d9aee49b58d01bcfa1d12"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"fb395993799ee1623ae5d304c58dbd3d12ef5ab5","unresolved":true,"context_lines":[{"line_number":663,"context_line":"                for tier in (dev.get(\u0027tiers\u0027) or tiers_for_dev(dev)):"},{"line_number":664,"context_line":"                    replicas_at_tier[tier] +\u003d 1"},{"line_number":665,"context_line":"                # IndexErrors will be raised if the replicas are increased or"},{"line_number":666,"context_line":"                # decreased, and that actually means the partition has changed"},{"line_number":667,"context_line":"                try:"},{"line_number":668,"context_line":"                    old_device \u003d old_replica2part2dev[rep_id][part_id]"},{"line_number":669,"context_line":"                except IndexError:"}],"source_content_type":"text/x-python","patch_set":3,"id":"c734ea01_f7dd2d12","line":666,"range":{"start_line":666,"start_character":18,"end_line":666,"end_character":27},"in_reply_to":"4735715e_8d8e5639","updated":"2021-08-04 00:31:31.000000000","message":"If it decreses we don\u0027t get the last primaries.. but then we\u0027re no worse off then where we are now.\n\nBut we could add some logic to track the old primaries in this case. The first version of this patch was updating the old_primary array when we removed things not here. But doing it here made the code cleaner. But we do have the old ring and new in hand at this point so we could add some replica different magic to grab them.","commit_id":"5610b1f1a524f0bd861d9aee49b58d01bcfa1d12"}],"swift/common/ring/ring.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"bcc9e63b1a321db6f9fa73238b3ae46a50e8d3cb","unresolved":true,"context_lines":[{"line_number":34,"context_line":""},{"line_number":35,"context_line":"from swift.common.exceptions import RingLoadError"},{"line_number":36,"context_line":"from swift.common.utils import hash_path, validate_configuration, md5"},{"line_number":37,"context_line":"from swift.common.ring.utils import tiers_for_dev, NONE_DEV"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"def calc_replica_count(replica2part2dev_id):"}],"source_content_type":"text/x-python","patch_set":1,"id":"d3de31a1_a4157345","line":37,"updated":"2021-05-11 15:08:07.000000000","message":"presumably NONE_DEV was moved to utils so we can use it here","commit_id":"5ae5178abc79890373c02ef4dc5509eaa17fb445"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"1044c73edbb4a6f38759d358ad6caa8b046b817a","unresolved":true,"context_lines":[{"line_number":34,"context_line":""},{"line_number":35,"context_line":"from swift.common.exceptions import RingLoadError"},{"line_number":36,"context_line":"from swift.common.utils import hash_path, validate_configuration, md5"},{"line_number":37,"context_line":"from swift.common.ring.utils import tiers_for_dev, NONE_DEV"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"def calc_replica_count(replica2part2dev_id):"}],"source_content_type":"text/x-python","patch_set":1,"id":"cdc119bd_5aaa27cc","line":37,"in_reply_to":"d3de31a1_a4157345","updated":"2021-05-11 23:43:41.000000000","message":"yeah it was a builder thing, but we\u0027re using it in the last_part2dev array for \"empty\" slots so moved it into ring.utils.","commit_id":"5ae5178abc79890373c02ef4dc5509eaa17fb445"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"62cf4028748cf2238fb52a2990d680b224301e55","unresolved":true,"context_lines":[{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        # Attempt to load the last primary array"},{"line_number":170,"context_line":"        ring_dict[\u0027last_primary2dev_id\u0027] \u003d array.array("},{"line_number":171,"context_line":"            \u0027H\u0027, gz_file.read(2 * partition_count)) or None"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"        return ring_dict"},{"line_number":174,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"319fdfad_1cb74459","line":171,"updated":"2021-05-12 00:59:42.000000000","message":"So how will this work with fractional replicas?","commit_id":"5ae5178abc79890373c02ef4dc5509eaa17fb445"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"1cde988d5c3b4fccc0f3d8080f84dd80006e5f29","unresolved":true,"context_lines":[{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        # Attempt to load the last primary array"},{"line_number":170,"context_line":"        ring_dict[\u0027last_primary2dev_id\u0027] \u003d array.array("},{"line_number":171,"context_line":"            \u0027H\u0027, gz_file.read(2 * partition_count)) or None"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"        return ring_dict"},{"line_number":174,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"d37fde95_8537081e","line":171,"in_reply_to":"319fdfad_1cb74459","updated":"2021-05-12 05:48:05.000000000","message":"Yeah turns out the way we dealt with fractional replicas when deserializing was just read more then you need.. So had to fix that. Fun times 😊","commit_id":"5ae5178abc79890373c02ef4dc5509eaa17fb445"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"bcc9e63b1a321db6f9fa73238b3ae46a50e8d3cb","unresolved":true,"context_lines":[{"line_number":555,"context_line":"        See :func:`get_nodes` for a description of the node dicts."},{"line_number":556,"context_line":"        \"\"\""},{"line_number":557,"context_line":"        if time() \u003e self._rtime:"},{"line_number":558,"context_line":"            self._reload()"},{"line_number":559,"context_line":"        primary_nodes \u003d self._get_part_nodes(part)"},{"line_number":560,"context_line":"        used \u003d set(d[\u0027id\u0027] for d in primary_nodes)"},{"line_number":561,"context_line":"        index \u003d count()"}],"source_content_type":"text/x-python","patch_set":1,"id":"3351fb09_fcbec7d5","line":558,"updated":"2021-05-11 15:08:07.000000000","message":"I like the way you\u0027ve bundled concerns in this change as in - but as a follow on I think we could have a lot of benifit updating this method to .get_more_nodes(for_read\u003dTrue) and have it automatically insert get_last_node into the front of the handoff chain...","commit_id":"5ae5178abc79890373c02ef4dc5509eaa17fb445"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"1044c73edbb4a6f38759d358ad6caa8b046b817a","unresolved":true,"context_lines":[{"line_number":555,"context_line":"        See :func:`get_nodes` for a description of the node dicts."},{"line_number":556,"context_line":"        \"\"\""},{"line_number":557,"context_line":"        if time() \u003e self._rtime:"},{"line_number":558,"context_line":"            self._reload()"},{"line_number":559,"context_line":"        primary_nodes \u003d self._get_part_nodes(part)"},{"line_number":560,"context_line":"        used \u003d set(d[\u0027id\u0027] for d in primary_nodes)"},{"line_number":561,"context_line":"        index \u003d count()"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f4dce92_6d0597ed","line":558,"in_reply_to":"3351fb09_fcbec7d5","updated":"2021-05-11 23:43:41.000000000","message":"great idea. For the reconstrutor side though, I want it easily insert it into the primary_nodes so if we find it we just push back to ssync and be done, no more rebuilding 😊\n\nI think having an interface where we can ask the ring for the last part node for a partition is useful (like in the reconstructor case) but yeah, adding that to get_more_nodes would be awesome.","commit_id":"5ae5178abc79890373c02ef4dc5509eaa17fb445"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"62cf4028748cf2238fb52a2990d680b224301e55","unresolved":true,"context_lines":[{"line_number":555,"context_line":"        See :func:`get_nodes` for a description of the node dicts."},{"line_number":556,"context_line":"        \"\"\""},{"line_number":557,"context_line":"        if time() \u003e self._rtime:"},{"line_number":558,"context_line":"            self._reload()"},{"line_number":559,"context_line":"        primary_nodes \u003d self._get_part_nodes(part)"},{"line_number":560,"context_line":"        used \u003d set(d[\u0027id\u0027] for d in primary_nodes)"},{"line_number":561,"context_line":"        index \u003d count()"}],"source_content_type":"text/x-python","patch_set":1,"id":"bdd06e67_c02bbc15","line":558,"in_reply_to":"3f4dce92_6d0597ed","updated":"2021-05-12 00:59:42.000000000","message":"FWIW, I played with adding a for_read flag in https://review.opendev.org/c/openstack/swift/+/740866 (for a separate purpose). It\u0027s not too hard, but does touch most code in the proxy.\n\nI\u0027d actually push for a different approach to using this in the reconstructor, though it\u0027d require knowing which replica the old primary was responsible for. Basically,\n\n* if the ring is less than \u003cconfigurable time period\u003e old and\n* we would try to reconstruct the recently-moved replica\n\nskip it for now. Assume that the old primary will ship it over within \u003ctime period\u003e, so don\u0027t go asking it *more* questions from a bunch of yahoos that need double the bandwidth to ship the frag. Instead, let the old primary focus on shipping parts off its disk as it sees fit.\n\nNote that we may *also* want to use this to prioritize the reconstructor jobs for the old primary -- seems preferable to do the known-old-primary partitions ahead of the general handoffs.","commit_id":"5ae5178abc79890373c02ef4dc5509eaa17fb445"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"2c4881730e091f12b155272dd2f5f9c575240971","unresolved":true,"context_lines":[{"line_number":158,"context_line":"        byteswap \u003d (ring_dict.get(\u0027byteorder\u0027, sys.byteorder) !\u003d sys.byteorder)"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"        partition_count \u003d 1 \u003c\u003c (32 - ring_dict[\u0027part_shift\u0027])"},{"line_number":161,"context_line":"        for x in range(ring_dict[\u0027replica_count\u0027]):"},{"line_number":162,"context_line":"            part2dev \u003d array.array(\u0027H\u0027, gz_file.read(2 * partition_count))"},{"line_number":163,"context_line":"            if byteswap:"},{"line_number":164,"context_line":"                part2dev.byteswap()"}],"source_content_type":"text/x-python","patch_set":3,"id":"1115d522_2c888bc7","side":"PARENT","line":161,"updated":"2021-08-03 23:33:49.000000000","message":"How\u0027s the old code going to handle new rings? Seems like it\u0027ll blow up here with a TypeError.\n\nAt the same time, adding a separate replica_count_float key won\u0027t work great, either; we\u0027ll start reading the last_part_nodes table as part of the replica2part2dev table if it\u0027s a fractional-replica ring :-/ I guess we need to bump the version number, and maybe provide an option for writing old v1 rings? Or at least include an UpgradeImpact about how you need to continue writing rings with the oldest version of Swift in your cluster.","commit_id":"6e5620944183a42b7908f24e25fe66ba3f3aec57"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"fb395993799ee1623ae5d304c58dbd3d12ef5ab5","unresolved":true,"context_lines":[{"line_number":158,"context_line":"        byteswap \u003d (ring_dict.get(\u0027byteorder\u0027, sys.byteorder) !\u003d sys.byteorder)"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"        partition_count \u003d 1 \u003c\u003c (32 - ring_dict[\u0027part_shift\u0027])"},{"line_number":161,"context_line":"        for x in range(ring_dict[\u0027replica_count\u0027]):"},{"line_number":162,"context_line":"            part2dev \u003d array.array(\u0027H\u0027, gz_file.read(2 * partition_count))"},{"line_number":163,"context_line":"            if byteswap:"},{"line_number":164,"context_line":"                part2dev.byteswap()"}],"source_content_type":"text/x-python","patch_set":3,"id":"22fd0d91_dee9b024","side":"PARENT","line":161,"in_reply_to":"1115d522_2c888bc7","updated":"2021-08-04 00:31:31.000000000","message":"Yeah, I was thinking about upgrade impact and maybe even add the float replica count change as a patch first. This can get back ported. Then we add the extra last_primary array (this one) afterwards. So if anyone needs to rollback from the latter, they can still load their rings in old code and it just wont pull out the last_primary array.","commit_id":"6e5620944183a42b7908f24e25fe66ba3f3aec57"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"2c4881730e091f12b155272dd2f5f9c575240971","unresolved":true,"context_lines":[{"line_number":161,"context_line":"        byteswap \u003d (ring_dict.get(\u0027byteorder\u0027, sys.byteorder) !\u003d sys.byteorder)"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        partition_count \u003d 1 \u003c\u003c (32 - ring_dict[\u0027part_shift\u0027])"},{"line_number":164,"context_line":"        replica_lengths \u003d get_replica_lengths(ring_dict[\u0027replica_count\u0027],"},{"line_number":165,"context_line":"                                              partition_count)"},{"line_number":166,"context_line":"        for length in replica_lengths:"},{"line_number":167,"context_line":"            part2dev \u003d array.array(\u0027H\u0027, gz_file.read(2 * length))"}],"source_content_type":"text/x-python","patch_set":3,"id":"7f592945_adf7c62f","line":164,"updated":"2021-08-03 23:33:49.000000000","message":"OK, yeah -- new code with old rings should be fine, because even if we previously wrote down\n\n \"replica_count\": 3\n\nwhen it was really more likw 2.5, the read() will come back short similar to the old code. 👍","commit_id":"5610b1f1a524f0bd861d9aee49b58d01bcfa1d12"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"2c4881730e091f12b155272dd2f5f9c575240971","unresolved":true,"context_lines":[{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        # Attempt to load the last primary array"},{"line_number":173,"context_line":"        ring_dict[\u0027last_primary2dev_id\u0027] \u003d array.array("},{"line_number":174,"context_line":"            \u0027H\u0027, gz_file.read(2 * partition_count)) or None"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"        return ring_dict"},{"line_number":177,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"98eca524_f1bab43c","line":174,"updated":"2021-08-03 23:33:49.000000000","message":"I think this needs a\n\n if ring_dict[\u0027last_primary2dev_id\u0027] and byteswap:\n     ring_dict[\u0027last_primary2dev_id\u0027].byteswap()","commit_id":"5610b1f1a524f0bd861d9aee49b58d01bcfa1d12"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"fb395993799ee1623ae5d304c58dbd3d12ef5ab5","unresolved":true,"context_lines":[{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        # Attempt to load the last primary array"},{"line_number":173,"context_line":"        ring_dict[\u0027last_primary2dev_id\u0027] \u003d array.array("},{"line_number":174,"context_line":"            \u0027H\u0027, gz_file.read(2 * partition_count)) or None"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"        return ring_dict"},{"line_number":177,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9d7a63f7_76e8d3fc","line":174,"in_reply_to":"98eca524_f1bab43c","updated":"2021-08-04 00:31:31.000000000","message":"Good call!","commit_id":"5610b1f1a524f0bd861d9aee49b58d01bcfa1d12"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"2c4881730e091f12b155272dd2f5f9c575240971","unresolved":true,"context_lines":[{"line_number":218,"context_line":"        # Only include next_part_power if it is set in the"},{"line_number":219,"context_line":"        # builder, otherwise just ignore it"},{"line_number":220,"context_line":"        _text \u003d {\u0027devs\u0027: ring[\u0027devs\u0027], \u0027part_shift\u0027: ring[\u0027part_shift\u0027],"},{"line_number":221,"context_line":"                 \u0027replica_count\u0027: self.replica_count,"},{"line_number":222,"context_line":"                 \u0027byteorder\u0027: sys.byteorder}"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"        if ring[\u0027version\u0027] is not None:"}],"source_content_type":"text/x-python","patch_set":3,"id":"9e06bb01_62b00437","line":221,"updated":"2021-08-03 23:33:49.000000000","message":"So it\u0027s going to be a float instead of an int now, yeah? And that type difference will get preserved with the JSON round-tripping. Makes me wonder if I should update calc_replica_count() so it returns an int if it can do so with no loss of fidelity...","commit_id":"5610b1f1a524f0bd861d9aee49b58d01bcfa1d12"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"62f5b7e8d691285f1937206f522abf8e14d17b38","unresolved":true,"context_lines":[{"line_number":577,"context_line":"        if for_read:"},{"line_number":578,"context_line":"            last_primary \u003d self.get_last_part_node(part)"},{"line_number":579,"context_line":"            if last_primary and last_primary[\u0027id\u0027] not in used:"},{"line_number":580,"context_line":"                yield dict(last_primary, handoff_index\u003dnext(index))"},{"line_number":581,"context_line":"                used.add(last_primary[\u0027id\u0027])"},{"line_number":582,"context_line":""},{"line_number":583,"context_line":"        parts \u003d len(self._replica2part2dev_id[0])"}],"source_content_type":"text/x-python","patch_set":3,"id":"7762b68e_dc708281","line":580,"range":{"start_line":580,"start_character":41,"end_line":580,"end_character":66},"updated":"2021-08-04 18:51:01.000000000","message":"I don\u0027t know about including a handoff index. I kind of want to consider this an authoritative response when doing GETs/HEADs in the proxy.\n\nFor example, today on master, when we get back 404s from handoffs we ignore them when calculating quorum -- handoffs aren\u0027t expected to have data, so the 404 should be unsurprising. Combined with\n\n rebalance_missing_suppression_count \u003d 1\n\nwe\u0027ll serve back a 404 on (Timeout, 404, 404) from primaries but a 503 on  (Timeout, Timeout, 404) -- assuming none of the 404s have timestamps associated with them.\n\nWith last_primary support, though, we aren\u0027t surprised if *one* of the four nodes have no data, but *two* 404s seem like a pretty good indication that we don\u0027t have anything for that name.  So rebalance_missing_suppression_count \u003d 1 still seems to make sense. Deciding on the client response code seems more like considering 4 \"primary\" responses for 3 replicas -- and that 503 would either\n\n* become a 200 if the last_primary had data,\n* become a 404 if the last_primary responded 404, or\n* stay a 503 if the last_primary also hit an error.\n\nSo maybe index\u003dlen(primary_nodes)+1 would work? It feels a little weird, but might be a reasonable option.","commit_id":"5610b1f1a524f0bd861d9aee49b58d01bcfa1d12"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"1f955b880dd534c97d2c58effd093a5b2d549dde","unresolved":true,"context_lines":[{"line_number":577,"context_line":"        if for_read:"},{"line_number":578,"context_line":"            last_primary \u003d self.get_last_part_node(part)"},{"line_number":579,"context_line":"            if last_primary and last_primary[\u0027id\u0027] not in used:"},{"line_number":580,"context_line":"                yield dict(last_primary, handoff_index\u003dnext(index))"},{"line_number":581,"context_line":"                used.add(last_primary[\u0027id\u0027])"},{"line_number":582,"context_line":""},{"line_number":583,"context_line":"        parts \u003d len(self._replica2part2dev_id[0])"}],"source_content_type":"text/x-python","patch_set":3,"id":"7df6249b_cbab43ee","line":580,"range":{"start_line":580,"start_character":41,"end_line":580,"end_character":66},"in_reply_to":"7762b68e_dc708281","updated":"2021-08-04 23:42:20.000000000","message":"In this patch we also have get_last_part_node, which will be a None if it doesn\u0027t have one. So maybe pulling in this on a read in get_part_nodes rather then in get more nodes makes sense if we want something more authoriative.. Or should it get a handoff_index of -1 or a new dict value to indicate an authorative response.\n\nget_part_nodes could include the primaries + dict(last_primary, last_primary\u003dTrue). And change our best_response code around it... or in get_more_nodes with the same (for when we move into them) or a special handoff_index value if adding more metadata is too much. Something like:\n\n HANDOFF_AUTHORTIVE_INDEX \u003d -1\nBut that could have handoff counting issues... just brainstorming 😊\n\nOr a combination, index \u003d len(primary_nodes)+1 AND last_primary\u003dTrue \n\nBut in either case it\u0027ll be about plumbing this into the proxy, so really just need to decide on an API for last_primaries.","commit_id":"5610b1f1a524f0bd861d9aee49b58d01bcfa1d12"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e2413c9b06457e9e1a67ecffb3bce70391ebf762","unresolved":true,"context_lines":[{"line_number":563,"context_line":""},{"line_number":564,"context_line":"        :param part: partition to get handoff nodes for"},{"line_number":565,"context_line":"        :param for_read: if true insert the last primary at the start of"},{"line_number":566,"context_line":"            the generator, if one exists for the part."},{"line_number":567,"context_line":"        :returns: generator of node dicts"},{"line_number":568,"context_line":""},{"line_number":569,"context_line":"        See :func:`get_nodes` for a description of the node dicts."}],"source_content_type":"text/x-python","patch_set":4,"id":"7f4adc77_9b2bb49e","line":566,"updated":"2021-08-12 17:55:29.000000000","message":"maybe instead of \"get_more_nodes(for_read\u003dTrue)\" we should update call-sites that are *interested* in the new behavior to call a different method...\n\n\"find_old_node_or_handoff\"\n\"get_best_nodes_for_read\"","commit_id":"b611c12e62834a2adfb04c2007c0d24000d71ded"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e2413c9b06457e9e1a67ecffb3bce70391ebf762","unresolved":true,"context_lines":[{"line_number":583,"context_line":"        if for_read:"},{"line_number":584,"context_line":"            last_primary \u003d self.get_last_part_node(part)"},{"line_number":585,"context_line":"            if last_primary and last_primary[\u0027id\u0027] not in used:"},{"line_number":586,"context_line":"                yield dict(last_primary, handoff_index\u003dnext(index))"},{"line_number":587,"context_line":"                used.add(last_primary[\u0027id\u0027])"},{"line_number":588,"context_line":""},{"line_number":589,"context_line":"        parts \u003d len(self._replica2part2dev_id[0])"}],"source_content_type":"text/x-python","patch_set":4,"id":"d979af13_fa0a4026","line":586,"updated":"2021-08-12 17:55:29.000000000","message":"maybe consider yielding a primary_index of... -1 or replica+1 instead of a \"handoff_index\"\n\ni\u0027m sure we have code that assumes everything coming out of get_more_nodes has a handoff_index 😞","commit_id":"b611c12e62834a2adfb04c2007c0d24000d71ded"}],"swift/common/ring/utils.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"bcc9e63b1a321db6f9fa73238b3ae46a50e8d3cb","unresolved":true,"context_lines":[{"line_number":24,"context_line":"# we can\u0027t store None\u0027s in the replica2part2dev array, so we high-jack"},{"line_number":25,"context_line":"# the max value for magic to represent the part is not currently"},{"line_number":26,"context_line":"# assigned to any device."},{"line_number":27,"context_line":"NONE_DEV \u003d 2 ** 16 - 1"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"def tiers_for_dev(dev):"}],"source_content_type":"text/x-python","patch_set":1,"id":"ac3d12d7_2344337e","line":27,"updated":"2021-05-11 15:08:07.000000000","message":"oic, this was MOVED from over in builder","commit_id":"5ae5178abc79890373c02ef4dc5509eaa17fb445"}],"test/unit/common/ring/test_ring.py":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"f9d496f3b52909e242b942a5acd3f6819030eec3","unresolved":true,"context_lines":[{"line_number":545,"context_line":"        self.ring._rebuild_tier_data()"},{"line_number":546,"context_line":""},{"line_number":547,"context_line":"    @unittest.skipIf(sys.version_info \u003e\u003d (3,),"},{"line_number":548,"context_line":"                     \"Seed-specific tests don\u0027t work well on py3\")"},{"line_number":549,"context_line":"    def test_get_more_nodes(self):"},{"line_number":550,"context_line":"        # Yes, these tests are deliberately very fragile. We want to make sure"},{"line_number":551,"context_line":"        # that if someone changes the results the ring produces, they know it."}],"source_content_type":"text/x-python","patch_set":2,"id":"687d2497_4a17e91c","line":548,"updated":"2021-05-12 21:01:42.000000000","message":"This skips on py3.. I might attempt a follow up to create a seperate py3 version of this and make it:\n\n @unittest.skipIf(sys.version_info \u003c (3,),\n\nAssuming of course we can find a consistant seed that\u0027ll always work on py3. I know the same seed wont work for py2 and py3. thus 2 different tests.","commit_id":"5d6d68bb645d3f0142d7eb87a76e281f1e49201d"}]}
