)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"325b68d32c9af31e90799bcd4a1df52712289d2e","unresolved":true,"context_lines":[{"line_number":7,"context_line":"ring: store actual ring replica count to better deserialize"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The current ring deserialize code when dealing with fractional replicas"},{"line_number":10,"context_line":"only works because a read past the end of the ring file is truncated,"},{"line_number":11,"context_line":"giving us the correct fractional replica. This is because we always"},{"line_number":12,"context_line":"store the number of replicas in the ring dict on disk as"},{"line_number":13,"context_line":"len(replica2part2dev_id) rather then the actual replica count, which in"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"fc1a0e67_23d06466","line":10,"updated":"2021-08-12 19:36:16.000000000","message":"of course it is :eyeroll:","commit_id":"03e2721887906faaada9657047f4ee9e89f7c686"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"325b68d32c9af31e90799bcd4a1df52712289d2e","unresolved":true,"context_lines":[{"line_number":16,"context_line":"This currently works, but if we ever want to store anything beyond the"},{"line_number":17,"context_line":"replica2part2dev_id structure on disk, deserialize with a ring with"},{"line_number":18,"context_line":"fractional replicas will pull extra data into the last replica,"},{"line_number":19,"context_line":"potentially crashing deserialization."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"This patch corrects serialization and deserialization by storing the"},{"line_number":22,"context_line":"correct replica count and correctly pulling the right amount of data off"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"48ffb444_0fba5f64","line":19,"updated":"2021-08-12 19:36:16.000000000","message":"Oh wow!  So old code is *really* not going to like new rings!\n\nI\u0027m sure we\u0027ve done this before, but still: UPGRADE IMPACT","commit_id":"03e2721887906faaada9657047f4ee9e89f7c686"}],"swift/common/ring/ring.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"325b68d32c9af31e90799bcd4a1df52712289d2e","unresolved":true,"context_lines":[{"line_number":152,"context_line":"        \"\"\""},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"        json_len, \u003d struct.unpack(\u0027!I\u0027, gz_file.read(4))"},{"line_number":155,"context_line":"        ring_dict \u003d json.loads(gz_file.read(json_len))"},{"line_number":156,"context_line":"        ring_dict[\u0027replica2part2dev_id\u0027] \u003d []"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"        if metadata_only:"}],"source_content_type":"text/x-python","patch_set":1,"id":"7b3fc9e3_7feae2c1","side":"PARENT","line":155,"updated":"2021-08-12 19:36:16.000000000","message":"we should think really hard right now about what we want the serialized format to actually look like - it doesn\u0027t seem like an unbounded read until the end of the file is ideal","commit_id":"901d2e15b7fa7c7a923b885cd4d369d063357075"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"cf9ec2ac3c3f61b3430815264a5b56dd838a1f67","unresolved":true,"context_lines":[{"line_number":152,"context_line":"        \"\"\""},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"        json_len, \u003d struct.unpack(\u0027!I\u0027, gz_file.read(4))"},{"line_number":155,"context_line":"        ring_dict \u003d json.loads(gz_file.read(json_len))"},{"line_number":156,"context_line":"        ring_dict[\u0027replica2part2dev_id\u0027] \u003d []"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"        if metadata_only:"}],"source_content_type":"text/x-python","patch_set":1,"id":"80ffbd88_cbccdf39","side":"PARENT","line":155,"in_reply_to":"7b3fc9e3_7feae2c1","updated":"2021-08-13 20:19:11.000000000","message":"Huh? It\u0027s not unbounded -- we\u0027re always specifying a read length, both before and after this change.\n\nStill, might be good/useful to switch to something more like a TLV scheme: https://en.wikipedia.org/wiki/Type%E2%80%93length%E2%80%93value","commit_id":"901d2e15b7fa7c7a923b885cd4d369d063357075"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"2df967e6464c4f44fa257f69e27eb58fdc9b1fa4","unresolved":true,"context_lines":[{"line_number":152,"context_line":"        \"\"\""},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"        json_len, \u003d struct.unpack(\u0027!I\u0027, gz_file.read(4))"},{"line_number":155,"context_line":"        ring_dict \u003d json.loads(gz_file.read(json_len))"},{"line_number":156,"context_line":"        ring_dict[\u0027replica2part2dev_id\u0027] \u003d []"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"        if metadata_only:"}],"source_content_type":"text/x-python","patch_set":1,"id":"cd59c8ac_633bc7bb","side":"PARENT","line":155,"in_reply_to":"80ffbd88_cbccdf39","updated":"2021-08-18 20:12:38.000000000","message":"that\u0027s what i\u0027m saying; the json bit has always been a length value schema\n\nthe larger part of the ring is a lot more brittle and that\u0027s where we\u0027re having problems; because the reads have a length, but no offset and the last one can be short","commit_id":"901d2e15b7fa7c7a923b885cd4d369d063357075"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0a1eafab16a44ffa0e9ed09279072f1815c04c81","unresolved":true,"context_lines":[{"line_number":152,"context_line":"        \"\"\""},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"        json_len, \u003d struct.unpack(\u0027!I\u0027, gz_file.read(4))"},{"line_number":155,"context_line":"        ring_dict \u003d json.loads(gz_file.read(json_len))"},{"line_number":156,"context_line":"        ring_dict[\u0027replica2part2dev_id\u0027] \u003d []"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"        if metadata_only:"}],"source_content_type":"text/x-python","patch_set":1,"id":"2f959652_fd7eda21","side":"PARENT","line":155,"in_reply_to":"cd59c8ac_633bc7bb","updated":"2021-08-19 22:14:07.000000000","message":"😞 Apparently we can\u0027t even pad it out with NONE_DEVs or something -- old code is going to just blindly do a self.devs[dev_id] and get an IndexError.","commit_id":"901d2e15b7fa7c7a923b885cd4d369d063357075"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"325b68d32c9af31e90799bcd4a1df52712289d2e","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":"        for x in range(ring_dict[\u0027replica_count\u0027]):"},{"line_number":165,"context_line":"            part2dev \u003d array.array(\u0027H\u0027, gz_file.read(2 * partition_count))"},{"line_number":166,"context_line":"            if byteswap:"},{"line_number":167,"context_line":"                part2dev.byteswap()"}],"source_content_type":"text/x-python","patch_set":1,"id":"dd7c5f1a_b69e10f0","side":"PARENT","line":164,"updated":"2021-08-12 19:36:16.000000000","message":"old code really expects this to be an int\n\n  \u003e\u003e\u003e r \u003d Ring(\u0027/etc/swift/object.ring.gz\u0027)\n  Traceback (most recent call last):\n    File \"\u003cstdin\u003e\", line 1, in \u003cmodule\u003e\n    File \"/vagrant/swift/swift/common/ring/ring.py\", line 290, in __init__\n      self._reload(force\u003dTrue)\n    File \"/vagrant/swift/swift/common/ring/ring.py\", line 295, in _reload\n      ring_data \u003d RingData.load(self.serialized_path)\n    File \"/vagrant/swift/swift/common/ring/ring.py\", line 188, in load\n      gz_file, metadata_only\u003dmetadata_only)\n    File \"/vagrant/swift/swift/common/ring/ring.py\", line 164, in deserialize_v1\n      for x in range(ring_dict[\u0027replica_count\u0027]):\n  TypeError: integer argument expected, got float\n\n... so you can\u0027t upgrade your ring-building node until *after* you\u0027ve upgraded your nodes","commit_id":"901d2e15b7fa7c7a923b885cd4d369d063357075"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"cf9ec2ac3c3f61b3430815264a5b56dd838a1f67","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":"        for x in range(ring_dict[\u0027replica_count\u0027]):"},{"line_number":165,"context_line":"            part2dev \u003d array.array(\u0027H\u0027, gz_file.read(2 * partition_count))"},{"line_number":166,"context_line":"            if byteswap:"},{"line_number":167,"context_line":"                part2dev.byteswap()"}],"source_content_type":"text/x-python","patch_set":1,"id":"bc09837f_e84bedcb","side":"PARENT","line":164,"in_reply_to":"dd7c5f1a_b69e10f0","updated":"2021-08-13 20:19:11.000000000","message":"We could at least do something like\n\n replica_count \u003d self.replica_count\n if replica_count \u003d\u003d int(replica_count):\n     replica_count \u003d int(replica_count)\n\nduring serialization so old code will be OK in the common case. Still worth an upgrade note, of course (and maybe still a ring version bump?) -- but I feel like fractional replica counts are kinda like PPIs: it should inherently be a transitional state, and telling ops \"hey, you shouldn\u0027t upgrade mid-replica-increase; finish your maintenance THEN upgrade\" is pretty reasonable.","commit_id":"901d2e15b7fa7c7a923b885cd4d369d063357075"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"325b68d32c9af31e90799bcd4a1df52712289d2e","unresolved":true,"context_lines":[{"line_number":200,"context_line":"                                 ring_data.get(\u0027next_part_power\u0027),"},{"line_number":201,"context_line":"                                 ring_data.get(\u0027version\u0027))"},{"line_number":202,"context_line":"        for attr in (\u0027md5\u0027, \u0027size\u0027, \u0027raw_size\u0027):"},{"line_number":203,"context_line":"            setattr(ring_data, attr, getattr(gz_file, attr))"},{"line_number":204,"context_line":"        return ring_data"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"    def serialize_v1(self, file_obj):"}],"source_content_type":"text/x-python","patch_set":1,"id":"d26d9225_ba1f2af0","side":"PARENT","line":203,"updated":"2021-08-12 19:36:16.000000000","message":"maybe setattr .replica_count","commit_id":"901d2e15b7fa7c7a923b885cd4d369d063357075"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"bbf31f542f2c6f1cc267e558785a6973ee043243","unresolved":true,"context_lines":[{"line_number":200,"context_line":"                                 ring_data.get(\u0027next_part_power\u0027),"},{"line_number":201,"context_line":"                                 ring_data.get(\u0027version\u0027))"},{"line_number":202,"context_line":"        for attr in (\u0027md5\u0027, \u0027size\u0027, \u0027raw_size\u0027):"},{"line_number":203,"context_line":"            setattr(ring_data, attr, getattr(gz_file, attr))"},{"line_number":204,"context_line":"        return ring_data"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"    def serialize_v1(self, file_obj):"}],"source_content_type":"text/x-python","patch_set":1,"id":"e4f40eb6_8b70f83a","side":"PARENT","line":203,"in_reply_to":"d26d9225_ba1f2af0","updated":"2021-08-13 06:27:15.000000000","message":"Well replica_count is a property from replca2part2dev_id so already set by this point.","commit_id":"901d2e15b7fa7c7a923b885cd4d369d063357075"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"325b68d32c9af31e90799bcd4a1df52712289d2e","unresolved":true,"context_lines":[{"line_number":211,"context_line":"        # Only include next_part_power if it is set in the"},{"line_number":212,"context_line":"        # builder, otherwise just ignore it"},{"line_number":213,"context_line":"        _text \u003d {\u0027devs\u0027: ring[\u0027devs\u0027], \u0027part_shift\u0027: ring[\u0027part_shift\u0027],"},{"line_number":214,"context_line":"                 \u0027replica_count\u0027: len(ring[\u0027replica2part2dev_id\u0027]),"},{"line_number":215,"context_line":"                 \u0027byteorder\u0027: sys.byteorder}"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"        if ring[\u0027version\u0027] is not None:"}],"source_content_type":"text/x-python","patch_set":1,"id":"86dcc95a_df500279","side":"PARENT","line":214,"updated":"2021-08-12 19:36:16.000000000","message":"it\u0027s so weird that we write down this integer into the serialized structure; but have thrown it out by the time we finish loading the ring","commit_id":"901d2e15b7fa7c7a923b885cd4d369d063357075"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"325b68d32c9af31e90799bcd4a1df52712289d2e","unresolved":true,"context_lines":[{"line_number":409,"context_line":"    @property"},{"line_number":410,"context_line":"    def replica_count(self):"},{"line_number":411,"context_line":"        \"\"\"Number of replicas (full or partial) used in the ring.\"\"\""},{"line_number":412,"context_line":"        return calc_replica_count(self._replica2part2dev_id)"},{"line_number":413,"context_line":""},{"line_number":414,"context_line":"    @property"},{"line_number":415,"context_line":"    def partition_count(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"d9628544_dd4b13f8","side":"PARENT","line":412,"updated":"2021-08-12 19:36:16.000000000","message":"it\u0027s so odd (surprising?) tha the integer value from the serialized format is transient \n\nright now, TODAY, rings have a builders have a .replica_count attribute *and* instances of the Ring class have a .replica_count attirbute - AND THEY\u0027RE ALREADY BOTH FLOATS!?","commit_id":"901d2e15b7fa7c7a923b885cd4d369d063357075"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"bbf31f542f2c6f1cc267e558785a6973ee043243","unresolved":true,"context_lines":[{"line_number":409,"context_line":"    @property"},{"line_number":410,"context_line":"    def replica_count(self):"},{"line_number":411,"context_line":"        \"\"\"Number of replicas (full or partial) used in the ring.\"\"\""},{"line_number":412,"context_line":"        return calc_replica_count(self._replica2part2dev_id)"},{"line_number":413,"context_line":""},{"line_number":414,"context_line":"    @property"},{"line_number":415,"context_line":"    def partition_count(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"1ec2f72d_bb2fae2b","side":"PARENT","line":412,"in_reply_to":"d9628544_dd4b13f8","updated":"2021-08-13 06:27:15.000000000","message":"Yeah, as far as I can see we use the serialized replica_count and part_shift to know how much data to read out of the ring file to get the replica2part2dev structure, from there we use the structure directly as a property.. so in a way it\u0027s necessary for deserailation but not needed afterwards.","commit_id":"901d2e15b7fa7c7a923b885cd4d369d063357075"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c7ab0759d365b14c05851f9f911de1dee5daf0d4","unresolved":true,"context_lines":[{"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))"},{"line_number":168,"context_line":"            if byteswap:"},{"line_number":169,"context_line":"                part2dev.byteswap()"},{"line_number":170,"context_line":"            ring_dict[\u0027replica2part2dev_id\u0027].append(part2dev)"}],"source_content_type":"text/x-python","patch_set":1,"id":"64bf5f94_71a95fac","line":167,"updated":"2021-08-10 18:35:49.000000000","message":"I wonder if we should have an assertion that\n\n len(part2dev) \u003d\u003d length\n\nhere -- I seem to remember hearing about people sometimes having trouble where a server reloads a ring that\u0027s still being written out. Write to a temp file then atomically rename into place is the way to go, but not everybody does it.","commit_id":"03e2721887906faaada9657047f4ee9e89f7c686"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"18f7418b24cc010ddbeebb80dd50ff9634227dab","unresolved":true,"context_lines":[{"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))"},{"line_number":168,"context_line":"            if byteswap:"},{"line_number":169,"context_line":"                part2dev.byteswap()"},{"line_number":170,"context_line":"            ring_dict[\u0027replica2part2dev_id\u0027].append(part2dev)"}],"source_content_type":"text/x-python","patch_set":1,"id":"d8224210_e023118b","line":167,"in_reply_to":"64bf5f94_71a95fac","updated":"2021-08-11 06:03:40.000000000","message":"yeah, but I worry if we do this, this will fail on exising rings. If we have an existing frantional replica ring, 2.5 saved to disk, then the current replica_count in the old code would be 3. So the length of the part2dev and length wont be the same and we\u0027d assert/bail out.\n\nMaybe I do need to think abount incrementing the ring serialized version in the rest of the chain. But that doesn\u0027t change that if we can load v2 (or whatever) serialized rings in old code (well code with this patch) as it\u0027ll just wont load the last_primaries code.","commit_id":"03e2721887906faaada9657047f4ee9e89f7c686"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"325b68d32c9af31e90799bcd4a1df52712289d2e","unresolved":true,"context_lines":[{"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))"},{"line_number":168,"context_line":"            if byteswap:"},{"line_number":169,"context_line":"                part2dev.byteswap()"},{"line_number":170,"context_line":"            ring_dict[\u0027replica2part2dev_id\u0027].append(part2dev)"}],"source_content_type":"text/x-python","patch_set":1,"id":"08932ce8_d727df57","line":167,"in_reply_to":"d8224210_e023118b","updated":"2021-08-12 19:36:16.000000000","message":"generally I don\u0027t like asserts in code, but some of the ring stuff does it during rebalance cause it seemed better than getting it wrong - i\u0027m not sure what\u0027s appropriate for serialization/deserialization code.","commit_id":"03e2721887906faaada9657047f4ee9e89f7c686"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c7ab0759d365b14c05851f9f911de1dee5daf0d4","unresolved":true,"context_lines":[{"line_number":213,"context_line":"        # Only include next_part_power if it is set in the"},{"line_number":214,"context_line":"        # builder, otherwise just ignore it"},{"line_number":215,"context_line":"        _text \u003d {\u0027devs\u0027: ring[\u0027devs\u0027], \u0027part_shift\u0027: ring[\u0027part_shift\u0027],"},{"line_number":216,"context_line":"                 \u0027replica_count\u0027: self.replica_count,"},{"line_number":217,"context_line":"                 \u0027byteorder\u0027: sys.byteorder}"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"        if ring[\u0027version\u0027] is not None:"}],"source_content_type":"text/x-python","patch_set":1,"id":"2de688c9_1501aef2","line":216,"updated":"2021-08-10 18:35:49.000000000","message":"Right, so this shims into calc_replica_count so we know it\u0027ll be based on the actual _replica2part2dev_id -- which means I shouldn\u0027t need to worry about some approximation that gets truncated differently as a result of a part-power increase or something.","commit_id":"03e2721887906faaada9657047f4ee9e89f7c686"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"18f7418b24cc010ddbeebb80dd50ff9634227dab","unresolved":true,"context_lines":[{"line_number":213,"context_line":"        # Only include next_part_power if it is set in the"},{"line_number":214,"context_line":"        # builder, otherwise just ignore it"},{"line_number":215,"context_line":"        _text \u003d {\u0027devs\u0027: ring[\u0027devs\u0027], \u0027part_shift\u0027: ring[\u0027part_shift\u0027],"},{"line_number":216,"context_line":"                 \u0027replica_count\u0027: self.replica_count,"},{"line_number":217,"context_line":"                 \u0027byteorder\u0027: sys.byteorder}"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"        if ring[\u0027version\u0027] is not None:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a8d5cfc_02a10c28","line":216,"in_reply_to":"2de688c9_1501aef2","updated":"2021-08-11 06:03:40.000000000","message":"yeah, I hope not. That\u0027s why I wanted to use that code, and it\u0027s what we use when we are using the ring in code, so let\u0027s serialize it.","commit_id":"03e2721887906faaada9657047f4ee9e89f7c686"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"2df967e6464c4f44fa257f69e27eb58fdc9b1fa4","unresolved":true,"context_lines":[{"line_number":37,"context_line":"from swift.common.ring.utils import tiers_for_dev, get_replica_lengths, \\"},{"line_number":38,"context_line":"    calc_replica_count"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"DEFAULT_RING_FORMAT_VERSION \u003d 1  # this may change to 2 in the future"},{"line_number":41,"context_line":"DEFAULT_RELOAD_TIME \u003d 15"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5e40ba44_12555893","line":40,"updated":"2021-08-18 20:12:38.000000000","message":"I think this will give us some grace for make changes to the v2 format for a little bit","commit_id":"18544a31b513179e00c256a59548d225d61016fa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"2df967e6464c4f44fa257f69e27eb58fdc9b1fa4","unresolved":true,"context_lines":[{"line_number":128,"context_line":"        return calc_replica_count(self._replica2part2dev_id)"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    @classmethod"},{"line_number":131,"context_line":"    def deserialize_v2(cls, gz_file, metadata_only\u003dFalse):"},{"line_number":132,"context_line":"        \"\"\""},{"line_number":133,"context_line":"        Deserialize a v2 ring file into a dictionary with `devs`, `part_shift`,"},{"line_number":134,"context_line":"        and `replica2part2dev_id` keys."}],"source_content_type":"text/x-python","patch_set":3,"id":"3f38ce4d_a6338cfa","line":131,"updated":"2021-08-18 20:12:38.000000000","message":"N.B. we don\u0027t need/want to change deserialize_v1","commit_id":"18544a31b513179e00c256a59548d225d61016fa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"2df967e6464c4f44fa257f69e27eb58fdc9b1fa4","unresolved":true,"context_lines":[{"line_number":155,"context_line":""},{"line_number":156,"context_line":"        partition_count \u003d 1 \u003c\u003c (32 - ring_dict[\u0027part_shift\u0027])"},{"line_number":157,"context_line":"        replica_lengths \u003d get_replica_lengths(ring_dict[\u0027replica_count\u0027],"},{"line_number":158,"context_line":"                                              partition_count)"},{"line_number":159,"context_line":"        for length in replica_lengths:"},{"line_number":160,"context_line":"            part2dev \u003d array.array(\u0027H\u0027, gz_file.read(2 * length))"},{"line_number":161,"context_line":"            if byteswap:"}],"source_content_type":"text/x-python","patch_set":3,"id":"303461de_b57076e2","line":158,"updated":"2021-08-18 20:12:38.000000000","message":"this is super sketch, a serialization format shouldn\u0027t have it\u0027s data structure length implicitly calculated","commit_id":"18544a31b513179e00c256a59548d225d61016fa"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0a1eafab16a44ffa0e9ed09279072f1815c04c81","unresolved":true,"context_lines":[{"line_number":155,"context_line":""},{"line_number":156,"context_line":"        partition_count \u003d 1 \u003c\u003c (32 - ring_dict[\u0027part_shift\u0027])"},{"line_number":157,"context_line":"        replica_lengths \u003d get_replica_lengths(ring_dict[\u0027replica_count\u0027],"},{"line_number":158,"context_line":"                                              partition_count)"},{"line_number":159,"context_line":"        for length in replica_lengths:"},{"line_number":160,"context_line":"            part2dev \u003d array.array(\u0027H\u0027, gz_file.read(2 * length))"},{"line_number":161,"context_line":"            if byteswap:"}],"source_content_type":"text/x-python","patch_set":3,"id":"8037e8ee_887f5117","line":158,"in_reply_to":"303461de_b57076e2","updated":"2021-08-19 22:14:07.000000000","message":"Devil\u0027s advocate -- I kinda like the single source of truth. What should we do if/when the table length doesn\u0027t match what we expect given the part power/replica count?\n\nOr do we drop one of those two from the metadata dict?","commit_id":"18544a31b513179e00c256a59548d225d61016fa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"2df967e6464c4f44fa257f69e27eb58fdc9b1fa4","unresolved":true,"context_lines":[{"line_number":157,"context_line":"        replica_lengths \u003d get_replica_lengths(ring_dict[\u0027replica_count\u0027],"},{"line_number":158,"context_line":"                                              partition_count)"},{"line_number":159,"context_line":"        for length in replica_lengths:"},{"line_number":160,"context_line":"            part2dev \u003d array.array(\u0027H\u0027, gz_file.read(2 * length))"},{"line_number":161,"context_line":"            if byteswap:"},{"line_number":162,"context_line":"                part2dev.byteswap()"},{"line_number":163,"context_line":"            ring_dict[\u0027replica2part2dev_id\u0027].append(part2dev)"}],"source_content_type":"text/x-python","patch_set":3,"id":"a33f8ecc_843cdec9","line":160,"updated":"2021-08-18 20:12:38.000000000","message":"i\u0027d much rather see v2 do a \n\n   part2dev_len, \u003d struct.unpack(\u0027!I\u0027, gz_file.read(4))\n   part2dev \u003d array.array(\u0027H\u0027, gz_file.read(part2dev_len))","commit_id":"18544a31b513179e00c256a59548d225d61016fa"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0a1eafab16a44ffa0e9ed09279072f1815c04c81","unresolved":true,"context_lines":[{"line_number":157,"context_line":"        replica_lengths \u003d get_replica_lengths(ring_dict[\u0027replica_count\u0027],"},{"line_number":158,"context_line":"                                              partition_count)"},{"line_number":159,"context_line":"        for length in replica_lengths:"},{"line_number":160,"context_line":"            part2dev \u003d array.array(\u0027H\u0027, gz_file.read(2 * length))"},{"line_number":161,"context_line":"            if byteswap:"},{"line_number":162,"context_line":"                part2dev.byteswap()"},{"line_number":163,"context_line":"            ring_dict[\u0027replica2part2dev_id\u0027].append(part2dev)"}],"source_content_type":"text/x-python","patch_set":3,"id":"518708e2_79831a43","line":160,"in_reply_to":"27227ce6_4b5c7ba1","updated":"2021-08-19 22:14:07.000000000","message":"I think we need to start documenting the (unzipped) format somewhat like you\u0027d see in an RFC. So currently, we\u0027ve got v1\u0027s unzipped format looking something like\n\n +---------------+-------+---------------+------------...---+\n |\u0027R\u0027 \u00271\u0027 \u0027N\u0027 \u0027G\u0027| 00 01 | \u003cJSON length\u003e | \u003cJSON text ... \u003e |\n +---------------+-------+---------------+------------...---+\n\nfollowed by N replica tables that look like\n\n +-------+-------+--...--+-------+\n | \u003cdev\u003e | \u003cdev\u003e |  ...  | \u003cdev\u003e |\n +-------+-------+--...--+-------+\n\nwhere the last table may be truncated to accommodate fractional replicas.\n\nIt sounds like Clay\u0027s in favor of redefining the replica tables to be more like\n\n +---------------+-------+-------+--...--+-------+\n |  \u003ctable len\u003e  | \u003cdev\u003e | \u003cdev\u003e |  ...  | \u003cdev\u003e |\n +---------------+-------+-------+--...--+-------+\n\nwhere each table is prefixed by a length in bytes, similar to the \u003cJSON length\u003e above.\n\nMatt is justifiably concerned that our theoretical maximum part power is 32, leading to 2**32 entries in the replica table, whose length overflows a 32-bit int *even with one byte device ids*! While we\u0027d never recommend that anyone have a part power that large (*especially* while we\u0027re still limited to 16-bit device ids!), we can\u0027t rely on it always being lower, so we need to account for it somehow. Going up to a 64-bit \u003ctable len\u003e seems reasonable. I\u0027d avoid trying to have it smaller -- getting off 2/4/8-byte ints seems like a fairly serious maintenance burden and the stream is going to get gzip\u0027ed anyway. I\u0027ll be glad when we can drop py2 and start using !Q instead of !II and manually combining. I\u0027d vote against doing much bit-packing for similar maintainability reasons.\n\nSince the first N-1 tables should all be the same length, I wonder if it\u0027d be worth doing something more like\n\n +-------------------------------+----------...---+-...-+----------...---+\n | \u003ctotal length of all tables\u003e  | \u003ctable 1 ... \u003e | ... | \u003ctable N ... \u003e |\n +-------------------------------+----------...---+-...-+----------...---+\n\nwhere each replica\u0027s table is defined similar to how we have it now. That total length should still fit quite comfortably in 64 bits.\n\n---\n\nI also wonder if it\u0027d be good to throw a type byte in front of our length-value pairs, so we get something like\n\n +---------------+-------+\n |\u0027R\u0027 \u00271\u0027 \u0027N\u0027 \u0027G\u0027| 00 02 |\n +---------------+-------+\n\n +---+---------------+------------...---+\n + 00+ \u003cJSON length\u003e | \u003cJSON text ... \u003e |\n +---+---------------+------------...---+\n\n +---+-------------------------------+-------------------...---+\n + 01+ \u003ctotal length\u003e                | \u003cseries of tables ... \u003e |\n +---+-------------------------------+-------------------...---+\n\n +---+-------------------------------+---------------------...---+\n + 02+ \u003ctable length\u003e                | \u003clast primary table ... \u003e |\n +---+-------------------------------+---------------------...---+\n\nwhere, for example, we know upon seeing the 0 that (1) we\u0027ve got a four-byte length to read and (2) it\u0027ll be some JSON ring metadata.\n\nIf we later add still-more data to the ring, it\u0027d also allow us to drop the last-primary table if the ring\u0027s been balanced for a while but we still wanted to send out updated type-03 data.\n\n---\n\nAs for the possibility of \u003cJSON length\u003e overflowing 32 bits, it looks like each dev in my saio adds ~170 bytes to the JSON metadata -- let\u0027s round it up to 200 to accommodate IPv6 (and for rounder numbers). Leaving some space for non-devs metadata, let\u0027s say we\u0027ve got ~4B bytes to play with -- so we\u0027d expect to be able to handle ~20M devices. I think we\u0027ll need to rethink a lot of things before we get close to that limit.\n\nOTOH, I could see an argument that it\u0027d be nice to standardize on 64-bit lengths so we can continue parsing in the face of unrecognized types. Though presumably we\u0027d want to order data in the ring by type code, so encountering an unknown type would be an indication that you can just halt parsing since there\u0027s nothing left you understand.","commit_id":"18544a31b513179e00c256a59548d225d61016fa"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"6efb1fa00262a2b6d155a8f332b547668182660a","unresolved":true,"context_lines":[{"line_number":157,"context_line":"        replica_lengths \u003d get_replica_lengths(ring_dict[\u0027replica_count\u0027],"},{"line_number":158,"context_line":"                                              partition_count)"},{"line_number":159,"context_line":"        for length in replica_lengths:"},{"line_number":160,"context_line":"            part2dev \u003d array.array(\u0027H\u0027, gz_file.read(2 * length))"},{"line_number":161,"context_line":"            if byteswap:"},{"line_number":162,"context_line":"                part2dev.byteswap()"},{"line_number":163,"context_line":"            ring_dict[\u0027replica2part2dev_id\u0027].append(part2dev)"}],"source_content_type":"text/x-python","patch_set":3,"id":"beaedfdd_7b1c10ea","line":160,"in_reply_to":"6b754585_26cefc94","updated":"2021-08-19 03:36:01.000000000","message":"No doubt there probably already is some module for this somewhere, but the code seems pretty straight forward, if we want to pack over 2 4-byte values because of the py33+ 8 byte limitation: https://paste.opendev.org/show/808192/","commit_id":"18544a31b513179e00c256a59548d225d61016fa"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"d8e92f24efdf81688650e9d907129d1d8b37e113","unresolved":true,"context_lines":[{"line_number":157,"context_line":"        replica_lengths \u003d get_replica_lengths(ring_dict[\u0027replica_count\u0027],"},{"line_number":158,"context_line":"                                              partition_count)"},{"line_number":159,"context_line":"        for length in replica_lengths:"},{"line_number":160,"context_line":"            part2dev \u003d array.array(\u0027H\u0027, gz_file.read(2 * length))"},{"line_number":161,"context_line":"            if byteswap:"},{"line_number":162,"context_line":"                part2dev.byteswap()"},{"line_number":163,"context_line":"            ring_dict[\u0027replica2part2dev_id\u0027].append(part2dev)"}],"source_content_type":"text/x-python","patch_set":3,"id":"6b754585_26cefc94","line":160,"in_reply_to":"a33f8ecc_843cdec9","updated":"2021-08-19 02:20:27.000000000","message":"I\u0027m a little curius about this. doing some quick maths, \u0027I\u0027 representing UINT of 4 bytes (as you denote there). If we want to allow for a max size of a part2dev would be 2^32 (32 partpower, which is crazy) but that\u0027s already a 4 byte number.\nIf we then do for each dev_id being currently 2 bytes (not even taking into account Tim\u0027s change for this to possibly become 4 bytes) means the potentual number of bytes we need to store for a single part2dev:\n\n  In [3]: 2 ** 32 * 2\n  Out[3]: 8589934592\n\nAt 4 bytes a dev_id:\n\n  In [4]: 2 ** 32 * 4\n  Out[4]: 17179869184\n\nBoth of which we\u0027ll need 5 bytes to store these numbers into. So maybe we\u0027d need to store 8 bytes per counter. So the ring will grow 8 * replica count (+1 when we add the last_part table) bytes in version 2.\nBut Tim mentions here (https://review.opendev.org/c/openstack/swift/+/761794/4/swift/common/ring/utils.py) that \u0027Q\u0027 or 8 bytes is only available in py33+ which could be a problem. So I guess I could have a play with packing them into multiple smaller units, like over 2 4 byte units.\n\nIf we continue that thought, if dev_ids potentually becoming 4 bytes each, I wonder if there is a chance that json_len would start being too small, and should we think about space for future growth here too.\n\nJust food for thought while thinking about v2 serialization.","commit_id":"18544a31b513179e00c256a59548d225d61016fa"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"b034b7014ddba1fa9271ac6f85d098570ded4047","unresolved":true,"context_lines":[{"line_number":157,"context_line":"        replica_lengths \u003d get_replica_lengths(ring_dict[\u0027replica_count\u0027],"},{"line_number":158,"context_line":"                                              partition_count)"},{"line_number":159,"context_line":"        for length in replica_lengths:"},{"line_number":160,"context_line":"            part2dev \u003d array.array(\u0027H\u0027, gz_file.read(2 * length))"},{"line_number":161,"context_line":"            if byteswap:"},{"line_number":162,"context_line":"                part2dev.byteswap()"},{"line_number":163,"context_line":"            ring_dict[\u0027replica2part2dev_id\u0027].append(part2dev)"}],"source_content_type":"text/x-python","patch_set":3,"id":"27227ce6_4b5c7ba1","line":160,"in_reply_to":"beaedfdd_7b1c10ea","updated":"2021-08-19 11:14:35.000000000","message":"Been thinking about this some more.. and thought of a way to really be able to properly scale to any number but only use a minimum of 5bytes and only use more as required.\n\nIf we\u0027re always going to use 4 byte chunks for a len, then no matter how big it goes we only ever need to track the bitshift of the last bytes (the rest will be 32). We can store that in 5 bits. Therefore we\u0027d have 3 bits of the first byte (if we only use one byte) to indicate the number of 4byte blocks (so max of 8 blocks [or maybe 7, but 0 can mean 1 right]).\nThis means we could have a theoredical max of (8 * 4 * 8 bits) 2^256 count (that\u0027s a big number).. but only even using storing a minimum of 5 bytes (1 for the bitwise and count + 4 bytes for the first block). And we can just add 4 byte blocks/uints as we grow.\n\nI think this packs down really nicely.\n\n... I also may have been thinking about this too much 😊","commit_id":"18544a31b513179e00c256a59548d225d61016fa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"2df967e6464c4f44fa257f69e27eb58fdc9b1fa4","unresolved":true,"context_lines":[{"line_number":236,"context_line":"            setattr(ring_data, attr, getattr(gz_file, attr))"},{"line_number":237,"context_line":"        return ring_data"},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"    def serialize_v2(self, file_obj):"},{"line_number":240,"context_line":"        # Write out new-style serialization magic and version:"},{"line_number":241,"context_line":"        file_obj.write(struct.pack(\u0027!4sH\u0027, b\u0027R1NG\u0027, 2))"},{"line_number":242,"context_line":"        ring \u003d self.to_dict()"}],"source_content_type":"text/x-python","patch_set":3,"id":"70e3ea17_506f3310","line":239,"updated":"2021-08-18 20:12:38.000000000","message":"N.B. we don\u0027t need/want to change serialize_v1 in this patch (we could refactor a common/base in a follow-up patch once we have the v2 format nailed down)","commit_id":"18544a31b513179e00c256a59548d225d61016fa"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0a1eafab16a44ffa0e9ed09279072f1815c04c81","unresolved":true,"context_lines":[{"line_number":243,"context_line":""},{"line_number":244,"context_line":"        # Only include next_part_power if it is set in the"},{"line_number":245,"context_line":"        # builder, otherwise just ignore it"},{"line_number":246,"context_line":"        _text \u003d {\u0027devs\u0027: ring[\u0027devs\u0027], \u0027part_shift\u0027: ring[\u0027part_shift\u0027],"},{"line_number":247,"context_line":"                 \u0027replica_count\u0027: self.replica_count,"},{"line_number":248,"context_line":"                 \u0027byteorder\u0027: sys.byteorder}"},{"line_number":249,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"58ed8dfd_fe0d26d8","line":246,"range":{"start_line":246,"start_character":40,"end_line":246,"end_character":50},"updated":"2021-08-19 22:14:07.000000000","message":"If we\u0027re doing a new format anyway -- I\u0027d rather we write out part_power than part_shift.","commit_id":"18544a31b513179e00c256a59548d225d61016fa"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"079bc88ae0a5b3c0a5e41720606e977025b549f5","unresolved":true,"context_lines":[{"line_number":243,"context_line":""},{"line_number":244,"context_line":"        # Only include next_part_power if it is set in the"},{"line_number":245,"context_line":"        # builder, otherwise just ignore it"},{"line_number":246,"context_line":"        _text \u003d {\u0027devs\u0027: ring[\u0027devs\u0027], \u0027part_shift\u0027: ring[\u0027part_shift\u0027],"},{"line_number":247,"context_line":"                 \u0027replica_count\u0027: self.replica_count,"},{"line_number":248,"context_line":"                 \u0027byteorder\u0027: sys.byteorder}"},{"line_number":249,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"0c27c304_261ac911","line":246,"range":{"start_line":246,"start_character":40,"end_line":246,"end_character":50},"in_reply_to":"58ed8dfd_fe0d26d8","updated":"2021-08-20 06:21:45.000000000","message":"yeah having part_shift in the ring rather then part_power is confusing. But RingData as it stands doesn\u0027t have a part_power only part_shift. We can change that I guess??","commit_id":"18544a31b513179e00c256a59548d225d61016fa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"2df967e6464c4f44fa257f69e27eb58fdc9b1fa4","unresolved":true,"context_lines":[{"line_number":244,"context_line":"        # Only include next_part_power if it is set in the"},{"line_number":245,"context_line":"        # builder, otherwise just ignore it"},{"line_number":246,"context_line":"        _text \u003d {\u0027devs\u0027: ring[\u0027devs\u0027], \u0027part_shift\u0027: ring[\u0027part_shift\u0027],"},{"line_number":247,"context_line":"                 \u0027replica_count\u0027: self.replica_count,"},{"line_number":248,"context_line":"                 \u0027byteorder\u0027: sys.byteorder}"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"        if ring[\u0027version\u0027] is not None:"}],"source_content_type":"text/x-python","patch_set":3,"id":"e0c9c955_067ce918","line":247,"updated":"2021-08-18 20:12:38.000000000","message":"this is super round-about; why not just leave len(replica2part2dev_id) alone and add a new explicit \"part2dev_lengths\" attribute?","commit_id":"18544a31b513179e00c256a59548d225d61016fa"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0a1eafab16a44ffa0e9ed09279072f1815c04c81","unresolved":true,"context_lines":[{"line_number":244,"context_line":"        # Only include next_part_power if it is set in the"},{"line_number":245,"context_line":"        # builder, otherwise just ignore it"},{"line_number":246,"context_line":"        _text \u003d {\u0027devs\u0027: ring[\u0027devs\u0027], \u0027part_shift\u0027: ring[\u0027part_shift\u0027],"},{"line_number":247,"context_line":"                 \u0027replica_count\u0027: self.replica_count,"},{"line_number":248,"context_line":"                 \u0027byteorder\u0027: sys.byteorder}"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"        if ring[\u0027version\u0027] is not None:"}],"source_content_type":"text/x-python","patch_set":3,"id":"67d4cda6_cc5b4cf3","line":247,"in_reply_to":"e0c9c955_067ce918","updated":"2021-08-19 22:14:07.000000000","message":"+1, then we don\u0027t have to worry about whether Python\u0027s using floats or doubles under the hood, or whether round-tripping it through JSON will preserve the precision.\n\n \"part2dev_lengths\": [1024, 1024, 768]","commit_id":"18544a31b513179e00c256a59548d225d61016fa"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0a1eafab16a44ffa0e9ed09279072f1815c04c81","unresolved":true,"context_lines":[{"line_number":245,"context_line":"        # builder, otherwise just ignore it"},{"line_number":246,"context_line":"        _text \u003d {\u0027devs\u0027: ring[\u0027devs\u0027], \u0027part_shift\u0027: ring[\u0027part_shift\u0027],"},{"line_number":247,"context_line":"                 \u0027replica_count\u0027: self.replica_count,"},{"line_number":248,"context_line":"                 \u0027byteorder\u0027: sys.byteorder}"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"        if ring[\u0027version\u0027] is not None:"},{"line_number":251,"context_line":"            _text[\u0027version\u0027] \u003d ring[\u0027version\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"4d4c4a26_1d0219e8","line":248,"range":{"start_line":248,"start_character":18,"end_line":248,"end_character":27},"updated":"2021-08-19 22:14:07.000000000","message":"I think we could get away from needing to include the \"byteorder\" key in v2 -- we just need to be sure to always write network-order to disk.","commit_id":"18544a31b513179e00c256a59548d225d61016fa"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"079bc88ae0a5b3c0a5e41720606e977025b549f5","unresolved":true,"context_lines":[{"line_number":245,"context_line":"        # builder, otherwise just ignore it"},{"line_number":246,"context_line":"        _text \u003d {\u0027devs\u0027: ring[\u0027devs\u0027], \u0027part_shift\u0027: ring[\u0027part_shift\u0027],"},{"line_number":247,"context_line":"                 \u0027replica_count\u0027: self.replica_count,"},{"line_number":248,"context_line":"                 \u0027byteorder\u0027: sys.byteorder}"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"        if ring[\u0027version\u0027] is not None:"},{"line_number":251,"context_line":"            _text[\u0027version\u0027] \u003d ring[\u0027version\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"31f25915_1b008264","line":248,"range":{"start_line":248,"start_character":18,"end_line":248,"end_character":27},"in_reply_to":"4d4c4a26_1d0219e8","updated":"2021-08-20 06:21:45.000000000","message":"Yeah this would be great, except we are serialising Array\u0027s which do their own struct packing. But we could write the array ourselves to disk with struct, then we can use network-order (!). And when we deserialize we unpack into an list and load that into the array as the constructor.","commit_id":"18544a31b513179e00c256a59548d225d61016fa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"2df967e6464c4f44fa257f69e27eb58fdc9b1fa4","unresolved":true,"context_lines":[{"line_number":265,"context_line":"                # doesn\u0027t count as an \u0027open file\u0027"},{"line_number":266,"context_line":"                file_obj.write(part2dev_id.tostring())"},{"line_number":267,"context_line":"            else:"},{"line_number":268,"context_line":"                part2dev_id.tofile(file_obj)"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"    def serialize_v1(self, file_obj):"},{"line_number":271,"context_line":"        # Write out new-style serialization magic and version:"}],"source_content_type":"text/x-python","patch_set":3,"id":"e1e55496_77046e22","line":268,"updated":"2021-08-18 20:12:38.000000000","message":"maybe we should just:\n\n    file_obj.write(struct.pack(\u0027!I\u0027, len(part2dev_string)))","commit_id":"18544a31b513179e00c256a59548d225d61016fa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"2df967e6464c4f44fa257f69e27eb58fdc9b1fa4","unresolved":true,"context_lines":[{"line_number":299,"context_line":"                part2dev_id.tofile(file_obj)"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"    def save(self, filename, mtime\u003d1300507380.0,"},{"line_number":302,"context_line":"             format_version\u003dDEFAULT_RING_FORMAT_VERSION):"},{"line_number":303,"context_line":"        \"\"\""},{"line_number":304,"context_line":"        Serialize this RingData instance to disk."},{"line_number":305,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"00f48544_79769494","line":302,"updated":"2021-08-18 20:12:38.000000000","message":"we need this option because we rebalance rings using a newer swift version on the controller \n\nalso it just seems like good practice","commit_id":"18544a31b513179e00c256a59548d225d61016fa"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0a1eafab16a44ffa0e9ed09279072f1815c04c81","unresolved":true,"context_lines":[{"line_number":299,"context_line":"                part2dev_id.tofile(file_obj)"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"    def save(self, filename, mtime\u003d1300507380.0,"},{"line_number":302,"context_line":"             format_version\u003dDEFAULT_RING_FORMAT_VERSION):"},{"line_number":303,"context_line":"        \"\"\""},{"line_number":304,"context_line":"        Serialize this RingData instance to disk."},{"line_number":305,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"15752d38_aa8c56c5","line":302,"in_reply_to":"00f48544_79769494","updated":"2021-08-19 22:14:07.000000000","message":"+100\n\nBut how\u0027d we handle the \"old-style pickled ring\" -\u003e v1 transition?","commit_id":"18544a31b513179e00c256a59548d225d61016fa"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"a5e6b7b1ceb39260e7c15c76831d697eafac2164","unresolved":true,"context_lines":[{"line_number":173,"context_line":"                part2dev \u003d array.array(\u0027H\u0027, gz_file.read(p2d_len))"},{"line_number":174,"context_line":"                if byteswap:"},{"line_number":175,"context_line":"                    part2dev.byteswap()"},{"line_number":176,"context_line":"                ring_dict[\u0027replica2part2dev_id\u0027].append(part2dev)"},{"line_number":177,"context_line":"            else:"},{"line_number":178,"context_line":"                raise RingLoadError("},{"line_number":179,"context_line":"                    \u0027JSON serialization type mismatch %d !\u003d %d\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"b81ae086_88abf945","line":176,"updated":"2021-08-23 03:09:09.000000000","message":"We just need to add an elif here for last_primaries","commit_id":"ac9d129bbe560803afb948b5ed9be61d8003f5a4"}],"swift/common/ring/utils.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"325b68d32c9af31e90799bcd4a1df52712289d2e","unresolved":true,"context_lines":[{"line_number":712,"context_line":"    desired_lengths \u003d [num_parts] * int(whole_replicas)"},{"line_number":713,"context_line":"    if fractional_replicas:"},{"line_number":714,"context_line":"        desired_lengths.append(int(num_parts * fractional_replicas))"},{"line_number":715,"context_line":"    return desired_lengths"}],"source_content_type":"text/x-python","patch_set":1,"id":"a650e8af_01980130","line":715,"updated":"2021-08-12 19:36:16.000000000","message":"I think this would look better next to common.ring.calc_replica_count\n\nmaybe that method should be moved in here to utils?","commit_id":"03e2721887906faaada9657047f4ee9e89f7c686"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"bbf31f542f2c6f1cc267e558785a6973ee043243","unresolved":false,"context_lines":[{"line_number":712,"context_line":"    desired_lengths \u003d [num_parts] * int(whole_replicas)"},{"line_number":713,"context_line":"    if fractional_replicas:"},{"line_number":714,"context_line":"        desired_lengths.append(int(num_parts * fractional_replicas))"},{"line_number":715,"context_line":"    return desired_lengths"}],"source_content_type":"text/x-python","patch_set":1,"id":"fb5f4369_c5348056","line":715,"in_reply_to":"a650e8af_01980130","updated":"2021-08-13 06:27:15.000000000","message":"Done","commit_id":"03e2721887906faaada9657047f4ee9e89f7c686"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"2df967e6464c4f44fa257f69e27eb58fdc9b1fa4","unresolved":true,"context_lines":[{"line_number":719,"context_line":"    fractional_replicas, whole_replicas \u003d math.modf(replicas)"},{"line_number":720,"context_line":"    desired_lengths \u003d [num_parts] * int(whole_replicas)"},{"line_number":721,"context_line":"    if fractional_replicas:"},{"line_number":722,"context_line":"        desired_lengths.append(int(num_parts * fractional_replicas))"},{"line_number":723,"context_line":"    return desired_lengths"}],"source_content_type":"text/x-python","patch_set":3,"id":"427b6142_96362f27","line":722,"updated":"2021-08-18 20:12:38.000000000","message":"i don\u0027t know how those mantissas work but this seems fudgy when we\u0027re trying to get to something concrete like \"how many bytes long is this datastructure\"","commit_id":"18544a31b513179e00c256a59548d225d61016fa"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0a1eafab16a44ffa0e9ed09279072f1815c04c81","unresolved":true,"context_lines":[{"line_number":719,"context_line":"    fractional_replicas, whole_replicas \u003d math.modf(replicas)"},{"line_number":720,"context_line":"    desired_lengths \u003d [num_parts] * int(whole_replicas)"},{"line_number":721,"context_line":"    if fractional_replicas:"},{"line_number":722,"context_line":"        desired_lengths.append(int(num_parts * fractional_replicas))"},{"line_number":723,"context_line":"    return desired_lengths"}],"source_content_type":"text/x-python","patch_set":3,"id":"2896fdc0_aed6d71d","line":722,"in_reply_to":"427b6142_96362f27","updated":"2021-08-19 22:14:07.000000000","message":"Fair concern -- looks like Python\u0027s using doubles, so 52-bit mantissa. Floats would be a real pain -- their 23-bit mantissa would have us already running into trouble.\n\nPlaying with something crazy like 48 replicas:\n\n \u003e\u003e\u003e 48.0 + 1./2**46\n 48.000000000000014\n \u003e\u003e\u003e 48.0 + 1./2**47\n 48.00000000000001\n \u003e\u003e\u003e 48.0 + 1./2**48\n 48.0\n\nYeah, seems like this ought to be exact for a full part-power-32 ring (assuming a not-insane replica count). It even round-trips well through JSON:\n\n \u003e\u003e\u003e json.loads(json.dumps(48.0 + 1./2**47))\n 48.00000000000001","commit_id":"18544a31b513179e00c256a59548d225d61016fa"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"594d3398c5677705b076eebbab3250efd8482d72","unresolved":true,"context_lines":[{"line_number":720,"context_line":"    desired_lengths \u003d [num_parts] * int(whole_replicas)"},{"line_number":721,"context_line":"    if fractional_replicas:"},{"line_number":722,"context_line":"        desired_lengths.append(int(num_parts * fractional_replicas))"},{"line_number":723,"context_line":"    return desired_lengths"}],"source_content_type":"text/x-python","patch_set":4,"id":"8d8c755c_d23f2d42","line":723,"updated":"2021-08-20 06:23:01.000000000","message":"I wonder if we need to do all this anymore if we just end up going down the current patchset\u0027s line of writing the length down.","commit_id":"ac9d129bbe560803afb948b5ed9be61d8003f5a4"}],"test/unit/common/ring/test_utils.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c7ab0759d365b14c05851f9f911de1dee5daf0d4","unresolved":true,"context_lines":[{"line_number":796,"context_line":"                (3.5, 100, [100, 100, 100, 50]),"},{"line_number":797,"context_line":"                (1.75, 100, [100, 75]),"},{"line_number":798,"context_line":"                (0.5, 100, [50])):"},{"line_number":799,"context_line":"            do_test(r, p, expected)"},{"line_number":800,"context_line":""},{"line_number":801,"context_line":""},{"line_number":802,"context_line":"if __name__ \u003d\u003d \u0027__main__\u0027:"}],"source_content_type":"text/x-python","patch_set":1,"id":"464a1309_f86aa2e7","line":799,"updated":"2021-08-10 18:35:49.000000000","message":"Should we also have a test with a real ring with too much or too little data?","commit_id":"03e2721887906faaada9657047f4ee9e89f7c686"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"325b68d32c9af31e90799bcd4a1df52712289d2e","unresolved":true,"context_lines":[{"line_number":796,"context_line":"                (3.5, 100, [100, 100, 100, 50]),"},{"line_number":797,"context_line":"                (1.75, 100, [100, 75]),"},{"line_number":798,"context_line":"                (0.5, 100, [50])):"},{"line_number":799,"context_line":"            do_test(r, p, expected)"},{"line_number":800,"context_line":""},{"line_number":801,"context_line":""},{"line_number":802,"context_line":"if __name__ \u003d\u003d \u0027__main__\u0027:"}],"source_content_type":"text/x-python","patch_set":1,"id":"76ccde53_40a1e574","line":799,"in_reply_to":"464a1309_f86aa2e7","updated":"2021-08-12 19:36:16.000000000","message":"YES\n\n  vagrant@saio:/etc/swift$ swift-ring-builder object.builder \n  object.builder, build version 8, id 91f198ad23604879be14e447e9730ace\n  64 partitions, 3.200000 replicas, 1 regions, 4 zones, 4 devices, 0.39 balance, 0.00 dispersion\n  ...\n  vagrant@saio:/etc/swift$ python\n  ...\n  \u003e\u003e\u003e from swift.common.ring import Ring\n  \u003e\u003e\u003e r \u003d Ring(\u0027/etc/swift/object.ring.gz\u0027)\n  \u003e\u003e\u003e r.replica_count\n  3.1875\n\n^ I don\u0027t see any tests demonstrating why these numbers are different and what that means.\n\nAFAIK this is \"expected\" - at least old code behaves the same way - even Ring instances loaded from old .ring.gz (that have the integer replica_count in the json struct) still report a floating .replica_count attribute when using calc_replica_count\n\nit\u0027s like the attribute replica_count that we write down in .ring.gz is totally divorced from the attribute .replica_count on the hydrated Ring instance","commit_id":"03e2721887906faaada9657047f4ee9e89f7c686"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"5329192ceac0b5c1ed18e1809916af1d373f3aa8","unresolved":true,"context_lines":[{"line_number":796,"context_line":"                (3.5, 100, [100, 100, 100, 50]),"},{"line_number":797,"context_line":"                (1.75, 100, [100, 75]),"},{"line_number":798,"context_line":"                (0.5, 100, [50])):"},{"line_number":799,"context_line":"            do_test(r, p, expected)"},{"line_number":800,"context_line":""},{"line_number":801,"context_line":""},{"line_number":802,"context_line":"if __name__ \u003d\u003d \u0027__main__\u0027:"}],"source_content_type":"text/x-python","patch_set":1,"id":"b010fe38_3ef07985","line":799,"in_reply_to":"76ccde53_40a1e574","updated":"2021-08-13 03:44:02.000000000","message":"yeah it totally is, it\u0027s just the len of the replica2part2dev_id structure, so in essence a ceil(ring.replica_count).","commit_id":"03e2721887906faaada9657047f4ee9e89f7c686"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"bbf31f542f2c6f1cc267e558785a6973ee043243","unresolved":true,"context_lines":[{"line_number":796,"context_line":"                (3.5, 100, [100, 100, 100, 50]),"},{"line_number":797,"context_line":"                (1.75, 100, [100, 75]),"},{"line_number":798,"context_line":"                (0.5, 100, [50])):"},{"line_number":799,"context_line":"            do_test(r, p, expected)"},{"line_number":800,"context_line":""},{"line_number":801,"context_line":""},{"line_number":802,"context_line":"if __name__ \u003d\u003d \u0027__main__\u0027:"}],"source_content_type":"text/x-python","patch_set":1,"id":"f722ff31_a0ec839b","line":799,"in_reply_to":"b010fe38_3ef07985","updated":"2021-08-13 06:27:15.000000000","message":"The more I read of what get_replica_legths is used for. its for calulating what the lengths of a replica2part2dev_id should be. In deserialise_v1 it\u0027s used to figure out how big the arrays are to read out of the file, and in RingBuilder._adjust_replica2part2dev_size when we rebalance and we either need to create the inital or adjust based on replica count changes. From there the Ring/Ringdata.replica_count is based off the replica2part2dev_id atructure, or in the case of Ringbuilder.replica_count it\u0027s the count passed in.\n\nSo I guess we could build a ring, then take it\u0027s parts and replica count, run it though this function and see if the lengths match. But it\u0027s hard to put too much or too little data into a ring as the replica_count is a property. I guess I could serialise with a bad replica_count value so it doesn\u0027t load as much or attempt to load too much, but that would more test how we handle currupt or bad ring serializations. hmm... interesting.","commit_id":"03e2721887906faaada9657047f4ee9e89f7c686"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"cf9ec2ac3c3f61b3430815264a5b56dd838a1f67","unresolved":true,"context_lines":[{"line_number":796,"context_line":"                (3.5, 100, [100, 100, 100, 50]),"},{"line_number":797,"context_line":"                (1.75, 100, [100, 75]),"},{"line_number":798,"context_line":"                (0.5, 100, [50])):"},{"line_number":799,"context_line":"            do_test(r, p, expected)"},{"line_number":800,"context_line":""},{"line_number":801,"context_line":""},{"line_number":802,"context_line":"if __name__ \u003d\u003d \u0027__main__\u0027:"}],"source_content_type":"text/x-python","patch_set":1,"id":"485abb24_5b468f30","line":799,"in_reply_to":"f722ff31_a0ec839b","updated":"2021-08-13 20:19:11.000000000","message":"Clay: FWIW, I remember doing a patch a while ago involving write_builder (so, taking a ring and doing what we can to recreate the builder) and thinking about this a decent bit -- I eventually decided I was OK with the builder having a replica_count that doesn\u0027t match the replica2part2dev table to make sure that any already-written tooling for replica increase would be guaranteed to make progress. A loop like\n\n while rb.replica_count \u003c 4:\n     rb.replica_count +\u003d 0.01\n     # write ring\n     # push ring out\n     # wait for replication\n\nturns into an infinite loop if\n\n rb.replica_count +\u003d 0.01\n\nmay leave replica_count unchanged. Just some food for thought.\n\nMatt: I\u0027m not opposed to using ring code to write something out, then modify the .ring.gz to suit the test. It may be challenging to truncate and still have a valid gz, but validating that we get some kind of IOError seems like a good idea. For *too much* data, we should be able to just append another gz stream:\n\n $ echo foo | gzip \u003e foo.gz\n $ echo bar | gzip \u003e bar.gz\n $ cat foo.gz bar.gz | gunzip\n foo\n bar\n\nThe on-disk bytes will be a little different from what we\u0027d get when we start writing last primaries, but not at a level that I think we\u0027re likely to care about","commit_id":"03e2721887906faaada9657047f4ee9e89f7c686"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"a62e29810e331e67e4e2bde1f81726eaed185d9c","unresolved":true,"context_lines":[{"line_number":815,"context_line":"            rb_lengths \u003d get_replica_lengths(rb.replicas, rb.parts)"},{"line_number":816,"context_line":"            ring_lengths \u003d get_replica_lengths(r.replica_count,"},{"line_number":817,"context_line":"                                               2 ** (32 - r._part_shift))"},{"line_number":818,"context_line":"            orig_lengths \u003d get_replica_lengths(replicas, 2 ** part_power)"},{"line_number":819,"context_line":""},{"line_number":820,"context_line":"            self.assertEqual(rb_lengths, ring_lengths)"},{"line_number":821,"context_line":"            self.assertEqual(rb_lengths, orig_lengths)"}],"source_content_type":"text/x-python","patch_set":2,"id":"f9225e04_1c0cec2a","line":818,"updated":"2021-08-13 07:19:40.000000000","message":"It could be argued that the lengths calculated from the initial input values (orig) are really the same as what\u0027s generated by the rb.replica_count and rb.parts, but it will test incase rb starts doing something different.","commit_id":"d4415fcd5c1ea8bd220c5dcf0a13d79ffaa51880"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"a62e29810e331e67e4e2bde1f81726eaed185d9c","unresolved":true,"context_lines":[{"line_number":820,"context_line":"            self.assertEqual(rb_lengths, ring_lengths)"},{"line_number":821,"context_line":"            self.assertEqual(rb_lengths, orig_lengths)"},{"line_number":822,"context_line":""},{"line_number":823,"context_line":"        for pp, rep in ((8, 3), (8, 2.5), (6, 3.2)):"},{"line_number":824,"context_line":"            do_test(pp, rep)"},{"line_number":825,"context_line":""},{"line_number":826,"context_line":"        for fractional in range(0, 100):"}],"source_content_type":"text/x-python","patch_set":2,"id":"02bd6f42_749d70a3","line":823,"updated":"2021-08-13 07:19:40.000000000","message":"the (6, 3.2) should be the same as the example Clay gave in the comments in the last patchset.","commit_id":"d4415fcd5c1ea8bd220c5dcf0a13d79ffaa51880"}]}