)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"37524065a32283e78c0a786488539c7065b5a715","unresolved":true,"context_lines":[{"line_number":7,"context_line":"db_replicator: merge rsync\u0027d db into existing db"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"rsync_then_merge always merges the receiver\u0027s existing db into the"},{"line_number":10,"context_line":"rsync\u0027d copy and then replaces the existing db with the copy. That\u0027s"},{"line_number":11,"context_line":"fast when the receiver has few rows, like the sharding leader\u0027s"},{"line_number":12,"context_line":"replication to a new shard\u0027s freshly created db."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"cefa5789_98bbdeb0","line":10,"range":{"start_line":10,"start_character":0,"end_line":10,"end_character":12},"updated":"2026-10-27 02:53:19.000000000","message":"I thought we did this because we can take a read lock for free on the existing container and write into the rsync\u0027ed container (into tmp/) so we\u0027re not contending with writes while we try and sync 2 databases.\n\ndoes going the otherway really solve things? except for maybe the db_id already exists so it only needs to merge from that water line? When we rsync over at what point do we reset the new DB\u0027s db_id? if it\u0027s post merge then maybe we\u0027re ok? if it\u0027s pre them maybe we should make it post?","commit_id":"8e7a9e969d0194f9ce3534329482bce8b015f9f0"},{"author":{"_account_id":38368,"name":"Christian Ohanaja","display_name":"Christian Ohanaja","email":"cohanaja@nvidia.com","username":"cohanaja"},"change_message_id":"8ab9b1504c2c37579cd151f9184d98a353033999","unresolved":true,"context_lines":[{"line_number":7,"context_line":"db_replicator: merge rsync\u0027d db into existing db"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"rsync_then_merge always merges the receiver\u0027s existing db into the"},{"line_number":10,"context_line":"rsync\u0027d copy and then replaces the existing db with the copy. That\u0027s"},{"line_number":11,"context_line":"fast when the receiver has few rows, like the sharding leader\u0027s"},{"line_number":12,"context_line":"replication to a new shard\u0027s freshly created db."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"77fe0b4f_5fcfc43c","line":10,"range":{"start_line":10,"start_character":0,"end_line":10,"end_character":12},"in_reply_to":"cefa5789_98bbdeb0","updated":"2026-10-27 10:46:45.000000000","message":"Sender into receiver merging does involve writing into the live db now, but my view is that since we\u0027re not merging nearly as many (hopefully assuming the sender/receiver haven\u0027t diverged too heavily) rows as before, it shouldn\u0027t be as much contention. The `get_objects_not_in` scan does hold a read lock for a bit though (I tried a bit of microbenchmarking on a staging node, got roughly 40ms/50k object scan) and if that seems too high I can tweak the scan size some more to get lower lock times.\n\nBasically I believe it should be significantly faster due to cutting out the redundant rows alone, to the point that it\u0027s hopefully worth it ! won\u0027t have to merge too many rows while live writes are occurring, and I think it\u0027s even an *improvement* when considering our previous (still current?) hypothesis that live writes flushing into the receiver db while we\u0027re trying to merge it into the sender db was also causing long replicate_times","commit_id":"8e7a9e969d0194f9ce3534329482bce8b015f9f0"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"97a5884893ae512ba9396586332a01dcd829e2ef","unresolved":true,"context_lines":[{"line_number":17,"context_line":"receivers rows into the copy like we do for the leader means that we"},{"line_number":18,"context_line":"merge all of the rows the receiver has over the sender, which can be a"},{"line_number":19,"context_line":"lot of unneeded work! The receiver db should mostly be a superset of the"},{"line_number":20,"context_line":"sender in these cases, so we should probably scan the smaller db and see"},{"line_number":21,"context_line":"what the receiver may be mising.."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"This patch makes it so we can merge the other way: scan the SENDER db,"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"c478f3f1_662f5708","line":20,"range":{"start_line":20,"start_character":26,"end_line":20,"end_character":65},"updated":"2026-10-26 19:05:02.000000000","message":"that seems like a perfectly natural \"invarient\" that\u0027s not obviously a \"routing\" decision - it\u0027s more like\n\n1) pick the smaller\n2) sync the rows of the smaller db into the larger\n3) make the larger db live\n\nI like the idea that this \"ordering\" of source and destination copies should be optimized - it\u0027s not obvious that needs to be \"explicit\" as much as \"make two dbs be one db\" could be \"optimized\" - to the extent that \"order matters\" I think it should be more about \"which data is authoratative for this replica\" and we should ensure that ALWAYS lands in the \"final\" db (perhaps by virtue of \"it was already there\" as an optimization)","commit_id":"8e7a9e969d0194f9ce3534329482bce8b015f9f0"},{"author":{"_account_id":38368,"name":"Christian Ohanaja","display_name":"Christian Ohanaja","email":"cohanaja@nvidia.com","username":"cohanaja"},"change_message_id":"8ab9b1504c2c37579cd151f9184d98a353033999","unresolved":true,"context_lines":[{"line_number":17,"context_line":"receivers rows into the copy like we do for the leader means that we"},{"line_number":18,"context_line":"merge all of the rows the receiver has over the sender, which can be a"},{"line_number":19,"context_line":"lot of unneeded work! The receiver db should mostly be a superset of the"},{"line_number":20,"context_line":"sender in these cases, so we should probably scan the smaller db and see"},{"line_number":21,"context_line":"what the receiver may be mising.."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"This patch makes it so we can merge the other way: scan the SENDER db,"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"b9b4f9b8_6a042541","line":20,"range":{"start_line":20,"start_character":26,"end_line":20,"end_character":65},"in_reply_to":"c478f3f1_662f5708","updated":"2026-10-27 10:46:45.000000000","message":"yup! thanks for pointing that out, helped me structure the new pset better\n\nWRT making sure the replica’s rows land in the final db: if the existing db is kept, they’re already there anyway, but if the copy is kept, the next patch does the a catch-up merge under a lock on the existing db so writes that arrive during the merge make it into the copy before it replaces the reciever dv.","commit_id":"8e7a9e969d0194f9ce3534329482bce8b015f9f0"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"37524065a32283e78c0a786488539c7065b5a715","unresolved":true,"context_lines":[{"line_number":18,"context_line":"merge all of the rows the receiver has over the sender, which can be a"},{"line_number":19,"context_line":"lot of unneeded work! The receiver db should mostly be a superset of the"},{"line_number":20,"context_line":"sender in these cases, so we should probably scan the smaller db and see"},{"line_number":21,"context_line":"what the receiver may be mising.."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"This patch makes it so we can merge the other way: scan the SENDER db,"},{"line_number":24,"context_line":"merge its rows that the receiver db has no identical copy of into the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"c32786f0_dc123ea3","line":21,"updated":"2026-10-27 02:53:19.000000000","message":"Row ids can also differ amosnts primaries, so what are we using, are we still reading every row in one and to find what to send to the other?","commit_id":"8e7a9e969d0194f9ce3534329482bce8b015f9f0"},{"author":{"_account_id":38368,"name":"Christian Ohanaja","display_name":"Christian Ohanaja","email":"cohanaja@nvidia.com","username":"cohanaja"},"change_message_id":"8ab9b1504c2c37579cd151f9184d98a353033999","unresolved":true,"context_lines":[{"line_number":18,"context_line":"merge all of the rows the receiver has over the sender, which can be a"},{"line_number":19,"context_line":"lot of unneeded work! The receiver db should mostly be a superset of the"},{"line_number":20,"context_line":"sender in these cases, so we should probably scan the smaller db and see"},{"line_number":21,"context_line":"what the receiver may be mising.."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"This patch makes it so we can merge the other way: scan the SENDER db,"},{"line_number":24,"context_line":"merge its rows that the receiver db has no identical copy of into the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"59ee1dc8_6181ed99","line":21,"in_reply_to":"c32786f0_dc123ea3","updated":"2026-10-27 10:46:45.000000000","message":"Yeah we still read + compare every row via some attach db querying introduced in the last patch. in this case we scan the smaller DB and merge into the larger, so things should abe more efficient compared to scanning the sender + merging into the receiver in follower sharder runs.","commit_id":"8e7a9e969d0194f9ce3534329482bce8b015f9f0"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"37524065a32283e78c0a786488539c7065b5a715","unresolved":true,"context_lines":[{"line_number":26,"context_line":"sender\u0027s db as synced up to the copy\u0027s max row, as newid would have, and"},{"line_number":27,"context_line":"takes the copy\u0027s sync points, shard ranges and metadata. Since the"},{"line_number":28,"context_line":"existing db is not replaced, updates that land on it during the merge"},{"line_number":29,"context_line":"are kept without any catch-up."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"We keep the old method for when the receiver db has fewer rows than the"},{"line_number":32,"context_line":"sender (like when the leader is first replicating to empty shard dbs it"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"99cceae9_144774de","line":29,"updated":"2026-10-27 02:53:19.000000000","message":"Oh so this might be the crux. The old db gets to keep it\u0027s db_id so replicas already know where they we\u0027re up to.\n\nThanks an interesting optimisation.","commit_id":"8e7a9e969d0194f9ce3534329482bce8b015f9f0"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"97a5884893ae512ba9396586332a01dcd829e2ef","unresolved":true,"context_lines":[{"line_number":34,"context_line":"rows, we do it the other way so we can still do less work."},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"similar to the last patch, only container dbs merge in place; other dbs"},{"line_number":37,"context_line":"still always merge into the copy."},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"Co-Authored-By: Claude Opus 5.5 \u003cnoreply@anthropic.com\u003e"},{"line_number":40,"context_line":"Change-Id: I4e4c286aae3ebcbdf0bb587665de6650ad4fa11e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"062982a0_1fe6a3d8","line":37,"updated":"2026-10-26 19:05:02.000000000","message":"the implied exclusion here is that \"larger account dbs still sync into smaller account dbs\" ... because ... reasons?","commit_id":"8e7a9e969d0194f9ce3534329482bce8b015f9f0"},{"author":{"_account_id":38368,"name":"Christian Ohanaja","display_name":"Christian Ohanaja","email":"cohanaja@nvidia.com","username":"cohanaja"},"change_message_id":"8ab9b1504c2c37579cd151f9184d98a353033999","unresolved":true,"context_lines":[{"line_number":34,"context_line":"rows, we do it the other way so we can still do less work."},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"similar to the last patch, only container dbs merge in place; other dbs"},{"line_number":37,"context_line":"still always merge into the copy."},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"Co-Authored-By: Claude Opus 5.5 \u003cnoreply@anthropic.com\u003e"},{"line_number":40,"context_line":"Change-Id: I4e4c286aae3ebcbdf0bb587665de6650ad4fa11e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"fe396a2f_31f8fd68","line":37,"in_reply_to":"062982a0_1fe6a3d8","updated":"2026-10-27 10:46:45.000000000","message":"I wanted to avoid messing with the account db setup since I wasn\u0027t much aware of the specifics and the original duplicate row filtering logic was kind of married more with the earlier patch, the smaller db merging into larger path works for account dbs now.","commit_id":"8e7a9e969d0194f9ce3534329482bce8b015f9f0"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"97a5884893ae512ba9396586332a01dcd829e2ef","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"4df100d5_a3c9dc57","updated":"2026-10-26 19:05:02.000000000","message":"interesting idea - some discussion of observational justification is warrented\n\nthe implementation isn\u0027t as obvious as I would expect - especially the `abort()` closure and multiple return; compard to a `finally` that always handles the 404 response in the same way w/ a single return path (after selecting merge-db ordering based on size rather than just sender/reciever)","commit_id":"8e7a9e969d0194f9ce3534329482bce8b015f9f0"}],"swift/common/db_replicator.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"97a5884893ae512ba9396586332a01dcd829e2ef","unresolved":true,"context_lines":[{"line_number":1200,"context_line":"            if (existing_broker.get_max_row() \u003e\u003d"},{"line_number":1201,"context_line":"                    new_broker.get_max_row() / 4 and"},{"line_number":1202,"context_line":"                    self._merge_new_into_existing("},{"line_number":1203,"context_line":"                        new_broker, existing_broker, abort)):"},{"line_number":1204,"context_line":"                # Unlike the copy, the existing db can start sharding while"},{"line_number":1205,"context_line":"                # we merge into it. Its rows then belong in the shards, so"},{"line_number":1206,"context_line":"                # don\u0027t record the rsync\u0027d db as synced."}],"source_content_type":"text/x-python","patch_set":2,"id":"71ebd972_5dcb0d21","line":1203,"updated":"2026-10-26 19:05:02.000000000","message":"this seems like a fudemental \"bifrication\" of the existing `rsync_then_merge` path - we now route into *either* \n\n* `_merge_new_into_existing`\n* `_merge_existing_into_new`\n\nit\u0027s not clear why `_finish_merge_into_existing` and `_post_rysnc_then_merge_hook` are fudementally different \"after the correct db file is ready make it live and quiesce\" would be diffent paths.\n\nI think this new \"routing layer\" in this method should be more obvious, withough introducing (two?) new return paths including a special case for the \"common\" `_abort_rsync_then_merge`?","commit_id":"8e7a9e969d0194f9ce3534329482bce8b015f9f0"},{"author":{"_account_id":38368,"name":"Christian Ohanaja","display_name":"Christian Ohanaja","email":"cohanaja@nvidia.com","username":"cohanaja"},"change_message_id":"8ab9b1504c2c37579cd151f9184d98a353033999","unresolved":true,"context_lines":[{"line_number":1200,"context_line":"            if (existing_broker.get_max_row() \u003e\u003d"},{"line_number":1201,"context_line":"                    new_broker.get_max_row() / 4 and"},{"line_number":1202,"context_line":"                    self._merge_new_into_existing("},{"line_number":1203,"context_line":"                        new_broker, existing_broker, abort)):"},{"line_number":1204,"context_line":"                # Unlike the copy, the existing db can start sharding while"},{"line_number":1205,"context_line":"                # we merge into it. Its rows then belong in the shards, so"},{"line_number":1206,"context_line":"                # don\u0027t record the rsync\u0027d db as synced."}],"source_content_type":"text/x-python","patch_set":2,"id":"3d88f9ed_098b1eee","line":1203,"in_reply_to":"71ebd972_5dcb0d21","updated":"2026-10-27 10:46:45.000000000","message":"I\u0027ve simplified the design a bit, now we just swap which db is the source and which is the destination, and everything after that is one path. abort_rsync_then_merge stays the same too, so the only kinda needed branching left is the newid() and the rename for the copy or recording the sender’s sync point and deleting the copy for the existing db.","commit_id":"8e7a9e969d0194f9ce3534329482bce8b015f9f0"}]}