)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"12dc1e57a8c04d91a421e5a7f438d518bc152a73","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"6bc23e19_4b6d72f2","updated":"2026-09-17 20:15:16.000000000","message":"\u003e we probably should merge them both at the same time is my opinion\n\nmy opinion is that the first patch will fix prod and this diff opens TONS of questions about `_get_shard_broker` modifying (in somecases) a primary root databases\u0027 shard metadata (!?) \n\nseperating misplaced vs cleaving operation when pre-paring the \"local_handoff\" (which could be a primary!  Or even a shard range that has no peers!?) certainly HELPS - since the bug we *currently* know of is only relevant through the network provided fallback _misplaced_ handling\n\n... but I\u0027m also not sure that we can avoid MAYBE writing AT LEAST the quoted root \"just in case\" (what if we\u0027re the only/last person who thinks this shard should exist and after it replicates off there\u0027s no one to tell it to whom it belongs!?)\n\nAS SUCH, I think we should merge the epoch.timestmap\u003dzero *fix* and work on figuring out what to do about existing stuck sharding database ... and only then invest sufficient brain cells to un-%^\u0026* `_get_shard_broker`\u0027s tendency to slam overriding non-authoratiative `timestamp.now()` facts directly into other databases with impunity.","commit_id":"3a5a2623f6b293fe525acf2fd1c76bd143753882"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"51dad9dfaec19b1acfe870543c1365372eb00d2b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"0755001d_2c3f905d","updated":"2026-09-17 13:00:00.000000000","message":"I like the split, we probably should merge them both at the same time is my opinion.\n\nThis patch seems to introduce a small edgecase where there can be a delete race due to interleaving db updates. Created a test for it https://review.opendev.org/c/openstack/swift/+/1006028\n\nSo I do wonder if the put_timestamp check that is used to have is a little better, but I guess that\u0027s only reducing the edgecase rather then solving it.\n\nThoughts?","commit_id":"3a5a2623f6b293fe525acf2fd1c76bd143753882"}],"swift/container/sharder.py":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"51dad9dfaec19b1acfe870543c1365372eb00d2b","unresolved":true,"context_lines":[{"line_number":2019,"context_line":"                self.db_logger.info("},{"line_number":2020,"context_line":"                    broker, \"Cleaving %r - zero objects found\","},{"line_number":2021,"context_line":"                    shard_range)"},{"line_number":2022,"context_line":"                if initialized:"},{"line_number":2023,"context_line":"                    # This was just created; don\u0027t need to replicate this"},{"line_number":2024,"context_line":"                    # SR because there was nothing there. So cleanup and"},{"line_number":2025,"context_line":"                    # remove the shard_broker from its hand off location."}],"source_content_type":"text/x-python","patch_set":3,"id":"9caa7381_42747409","line":2022,"updated":"2026-09-17 13:00:00.000000000","message":"Removing the put_timestamp check seems to have opened up an interleaving issue, something like:\n\n1. The cleaver creates the local destination database.                                                                                                                                                                                                       \n2. A container PUT advances its `put_timestamp`, and an object update adds a row.                                                                                                                                                                            \n3. The cleaver finds no source rows for this range.                                                                                                                                                                                                          \n4. `initialized` is still true, so it sets `CLEAVE_EMPTY` and calls `delete_db()` without replication.\n\nWhich deletes a DB but it does have a row. This is because the destination (of find_local_handoff_for_part()) can actually be local primary for that shard. If other sharders send container PUTs while creating shard containers, and those containers can already receive redirected object updates.\n\nYeah it\u0027s a small edgecase, but possible. I guess one would hope that the object rows deleted will still be recovered and replicated from elsewhere, but just something to note.\n\nThe old put_timestamp check meant the DB hadn\u0027t changed so a little better from that point of view.. I guess one could argue, even with the put_timestamp check, a PUT could come in after here and before delete_db, so maybe moot 🤷\n\nHere is a test that exposes the issue: https://review.opendev.org/c/openstack/swift/+/1006028","commit_id":"3a5a2623f6b293fe525acf2fd1c76bd143753882"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"2f2a25c270ad23f016549590644264c0edf5b6e5","unresolved":true,"context_lines":[{"line_number":2019,"context_line":"                self.db_logger.info("},{"line_number":2020,"context_line":"                    broker, \"Cleaving %r - zero objects found\","},{"line_number":2021,"context_line":"                    shard_range)"},{"line_number":2022,"context_line":"                if initialized:"},{"line_number":2023,"context_line":"                    # This was just created; don\u0027t need to replicate this"},{"line_number":2024,"context_line":"                    # SR because there was nothing there. So cleanup and"},{"line_number":2025,"context_line":"                    # remove the shard_broker from its hand off location."}],"source_content_type":"text/x-python","patch_set":3,"id":"774c2e53_486b2731","line":2022,"in_reply_to":"4fc49aac_f2080107","updated":"2026-09-17 23:16:05.000000000","message":"yeah, good instincts. If I remember correctly we added the \"just skip (and delete) empty shards\" primarily for small handoffs who once the primaries moved into sharding state, the handoffs needed to shard in order to \"replicate away\". But a small handoff is mostly nothing, so in the case of a large container sharding a small handof  are just alot of empty shards, so rather then having to wait for a replication or worse hitting the number of cleaves this cycle counter, we\u0027d just clean them up and move on the handoffs can shard quicker.\n\nBasically this was a optimisation. Maybe we just need to detect if the returned \"handoff\" from `find_local_handoff_for_part()` is actually a handoff or a primary. And we can never skip (or delete) a primary, but an empty deep handoff is ok.. I guess the question there can we tell deepness? (\u003eX deep into  get_more_nodes(\u003cshard broker part\u003e).","commit_id":"3a5a2623f6b293fe525acf2fd1c76bd143753882"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"f513f12095bd045015c7b4cb3fcb6fd45630cc4e","unresolved":true,"context_lines":[{"line_number":2019,"context_line":"                self.db_logger.info("},{"line_number":2020,"context_line":"                    broker, \"Cleaving %r - zero objects found\","},{"line_number":2021,"context_line":"                    shard_range)"},{"line_number":2022,"context_line":"                if initialized:"},{"line_number":2023,"context_line":"                    # This was just created; don\u0027t need to replicate this"},{"line_number":2024,"context_line":"                    # SR because there was nothing there. So cleanup and"},{"line_number":2025,"context_line":"                    # remove the shard_broker from its hand off location."}],"source_content_type":"text/x-python","patch_set":3,"id":"7c00fd0c_bd155a8d","line":2022,"in_reply_to":"774c2e53_486b2731","updated":"2026-09-18 02:10:14.000000000","message":"Maybe something like: https://review.opendev.org/c/openstack/swift/+/1006136 \n\nThat\u0027s just a quick whip up by me (not even an agent). Not sure a like the extra plumbing of yet another parameter or if HANDOFF_DEEP \u003d\u003d \u003e replica_count (being the deafult is 2*replica_count). But it\u0027s more of showing one option.","commit_id":"3a5a2623f6b293fe525acf2fd1c76bd143753882"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"12dc1e57a8c04d91a421e5a7f438d518bc152a73","unresolved":true,"context_lines":[{"line_number":2019,"context_line":"                self.db_logger.info("},{"line_number":2020,"context_line":"                    broker, \"Cleaving %r - zero objects found\","},{"line_number":2021,"context_line":"                    shard_range)"},{"line_number":2022,"context_line":"                if initialized:"},{"line_number":2023,"context_line":"                    # This was just created; don\u0027t need to replicate this"},{"line_number":2024,"context_line":"                    # SR because there was nothing there. So cleanup and"},{"line_number":2025,"context_line":"                    # remove the shard_broker from its hand off location."}],"source_content_type":"text/x-python","patch_set":3,"id":"4fc49aac_f2080107","line":2022,"in_reply_to":"9caa7381_42747409","updated":"2026-09-17 20:15:16.000000000","message":"\u003e initialized is still true, so it sets CLEAVE_EMPTY and calls delete_db() without replication.\n\nto me THAT sounds like the bug more than \"only delete a database we assume is empty if we can\u0027t detect it\u0027s put timestamp hasn\u0027t changed changed\" ... like is optimistic locking really NEED in this case?  how about instead after we cleave we replicate - if that doesn\u0027t have to move a bunch of stuff GREAT!  then after successful replication - we delete (and not *before* !?!?! we confirm successful handoff replication !?!?!)","commit_id":"3a5a2623f6b293fe525acf2fd1c76bd143753882"}]}