)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"8ecca0e69fab1bf572cefd7437f08d94173a1f86","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"0ff96fb0_55db36f2","updated":"2026-06-03 05:46:30.000000000","message":"recheck\n\nunrelated time out","commit_id":"c21f19f4a35c3d0573a23e68cca68627884ac28f"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"78f9a8acc0b4a987ae3067d09369d36c03d2e6c7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"2dc7bb7b_6562db03","updated":"2026-07-28 05:27:35.000000000","message":"I feel like is\u0027t weird we\u0027re recalucating the _history_epoch when we have the time it should be set to at hand..\n\nAlso an inline question about the extra number for full ring iterations we\u0027ve added to a rebalance.","commit_id":"f695fee2906c42a498ee34b595dda3caf83ab705"},{"author":{"_account_id":38496,"name":"Andressa Cabistani","display_name":"Andressa","email":"acabistani@gmail.com","username":"andressadotpy","status":"I\u0027m a Software Engineer at Red Hat and I love Open Source and connect with people! Feel free to DM through IRC, I\u0027ll be delighted to chat"},"change_message_id":"eb2114007e41919f31f32e81fa9cb67b55f0d402","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"b8f1f324_c66d5c3f","updated":"2026-07-28 14:16:06.000000000","message":"It looks good to me. Here follows a summary of what I analyzed\n\n- Good test coverage\n- Profiled rebalance performance at 2^10, 2^14, and 2^18 parts to assess the multiple full-ring iteration concern\n- Tested epoch drift behavior: advances by `whole * period`, remainder preserved\n- Tested age increment: 0→1 after one cycle\n- Tested earliest-prior-wins: second _record_voluntary_move does not overwrite first\n- Tested pass-based aging (min_part_hours\u003d0): increments by exactly 1 per pass","commit_id":"f695fee2906c42a498ee34b595dda3caf83ab705"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"1485b1c114d2a1b74a7624d97a87310e8eb9edee","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"65b4fa20_7e526125","updated":"2026-07-29 00:01:16.000000000","message":"Still doing some rebalance benchmarking but itinitally without a failed device it adds a linlar 6% to the rebalance time.\n\n```\n   Part power                   Master median    991131 median          Difference    RSS increase                                                                                                                 \n  ━━━━━━━━━━━━━━━━━━━━━━━━━━━  ━━━━━━━━━━━━━━━  ━━━━━━━━━━━━━━━  ━━━━━━━━━━━━━━━━━━  ━━━━━━━━━━━━━━                                                                                                                \n   20 (1,048,576 partitions)          17.65 s          18.70 s    +1.05 s / +5.94%       +9.46 MiB                                                                                                                 \n  ───────────────────────────  ───────────────  ───────────────  ──────────────────  ──────────────                                                                                                                \n   21 (2,097,152 partitions)          35.57 s          37.70 s    +2.13 s / +5.99%      +18.51 MiB                                                                                                                 \n                                                                                                                                                                                                                   \n  Each run used a balanced 12-device, three-replica ring, restored after its initial build to initialise history, warmed once, then measured three no-change rebalances. All samples moved zero partitions.        \n                                                                                                                                                                                                                   \n  The new full-ring passes account for most of the regression:                                                                                                                                                     \n                                                                                                                                                                                                                   \n   Part power        Age    Reconcile    Age-out    Combined                                                                                                                                                       \n  ━━━━━━━━━━━━  ━━━━━━━━━  ━━━━━━━━━━━  ━━━━━━━━━  ━━━━━━━━━━                                                                                                                                                      \n   20            0.354 s      0.242 s    0.129 s     0.725 s                                                                                                                                                       \n  ────────────  ─────────  ───────────  ─────────  ──────────                                                                                                                                                      \n   21            0.705 s      0.474 s    0.266 s     1.445 s                                                                                                                                                       \n                                                                                                                                                                                                                   \n  That is three extra full sweeps over 3.15M history cells at p20, or 6.29M at p21. Their direct time explains about 68–69% of the observed wall-clock increase; the rest is likely cache/memory pressure and      \n  surrounding bookkeeping.\n```\n\nGetting my agent minions to throw a failed-device scenario too and to continue up to pp of 24, just so I can properly confirm the linilar pattern.","commit_id":"f695fee2906c42a498ee34b595dda3caf83ab705"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"075f990eed023f94d70d7ca038fba29e7360054d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"16681b28_150ec074","updated":"2026-07-29 01:06:32.000000000","message":"```\n   Part power    Partitions      Master      991131             Increase\n  ━━━━━━━━━━━━  ━━━━━━━━━━━━  ━━━━━━━━━━  ━━━━━━━━━━  ━━━━━━━━━━━━━━━━━━━\n   20                 1.05M     17.65 s     18.70 s     +1.05 s / +5.94%\n  ────────────  ────────────  ──────────  ──────────  ───────────────────\n   21                 2.10M     35.57 s     37.70 s     +2.13 s / +5.99%\n  ────────────  ────────────  ──────────  ──────────  ───────────────────\n   22                 4.19M     71.34 s     75.00 s     +3.66 s / +5.13%\n  ────────────  ────────────  ──────────  ──────────  ───────────────────\n   23                 8.39M    140.95 s    148.93 s     +7.98 s / +5.66%\n  ────────────  ────────────  ──────────  ──────────  ───────────────────\n   24                16.78M    283.26 s    296.14 s    +12.88 s / +4.55%\n\n  p20/p21 are medians of three samples; p22–p24 are one sample each after an identical warm-up. Every run used the same saved, restored 12-device/3-replica builder, min_part_hours\u003d0, and moved zero partitions.\n\n  The direct time in the three normal history sweeps reaches:\n\n  - p22: 2.62 s\n  - p23: 5.01 s\n  - p24: 10.01 s\n\n  At p24 those three sweeps explain 10.01 of the 12.88 added seconds. The p24 history state itself holds 96 MiB of prior-device entries plus 50.6 MiB of ages—about 147 MiB of additional array payload.\n\n  I also added failed-device rebalances:\n\n   Part power    Reassigned parts     Master     991131            Increase\n  ━━━━━━━━━━━━  ━━━━━━━━━━━━━━━━━━  ━━━━━━━━━  ━━━━━━━━━  ━━━━━━━━━━━━━━━━━━\n   20                     309,806    19.69 s    20.33 s    +0.64 s / +3.24%\n  ────────────  ──────────────────  ─────────  ─────────  ──────────────────\n   21                     619,614    40.27 s    41.93 s    +1.66 s / +4.12%\n\n  Those are single samples with the branch order reversed between powers to reduce warm-cache bias. They exercise the additional _apply_failed_history_sweep, so failure handling adds a fourth whole-table\n  traversal beyond the normal three.\n```\n\nSo adding failed-devices (only tested against pp 20 and 21. did increase some more. Maybe a 6-10% rebalace time increase is fine. This is where the code with helper methods are easier to read/maintain vs optimisation becomes an interesting decision. Thoughts?","commit_id":"f695fee2906c42a498ee34b595dda3caf83ab705"}],"swift/common/ring/builder.py":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"78f9a8acc0b4a987ae3067d09369d36c03d2e6c7","unresolved":true,"context_lines":[{"line_number":571,"context_line":"                    age_row[p] \u003d 255"},{"line_number":572,"context_line":"                else:"},{"line_number":573,"context_line":"                    age_row[p] \u003d new_val"},{"line_number":574,"context_line":"        self._history_epoch +\u003d whole * period"},{"line_number":575,"context_line":""},{"line_number":576,"context_line":"    def _record_voluntary_move(self, replica, part):"},{"line_number":577,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"f5815957_ebe5659e","line":574,"range":{"start_line":574,"start_character":8,"end_line":574,"end_character":45},"updated":"2026-07-28 05:27:35.000000000","message":"This isn\u0027t what I expected. period is already defined from the difference between now and _history_epcok, so can\u0027t we just use the same value of now, ie: `int(time())`\n\n```\nperiod \u003d self.min_part_hours * 3600\nnow \u003d int(time())\nwhole \u003d (now - self._history_epoch) // period\n...\nself._history_epoch \u003d now\n``` \n\nRather then relying of `+\u003d whole * period` and just absorb any rounding issues to recalculate the now we used?","commit_id":"f695fee2906c42a498ee34b595dda3caf83ab705"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"9c1e61ed42ec0b17a559beb810a1714a4ef856c5","unresolved":true,"context_lines":[{"line_number":571,"context_line":"                    age_row[p] \u003d 255"},{"line_number":572,"context_line":"                else:"},{"line_number":573,"context_line":"                    age_row[p] \u003d new_val"},{"line_number":574,"context_line":"        self._history_epoch +\u003d whole * period"},{"line_number":575,"context_line":""},{"line_number":576,"context_line":"    def _record_voluntary_move(self, replica, part):"},{"line_number":577,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"0f0079b0_483ba2a1","line":574,"range":{"start_line":574,"start_character":8,"end_line":574,"end_character":45},"in_reply_to":"302cb0d7_98237cc2","updated":"2026-08-04 13:56:52.000000000","message":"Keeping the remainder means the ages follow the clock, not how often we rebalance. At min_part_hours\u003d24, after 8 rebalances:\n\n```\nrebalance every    cycles really passed    +\u003d whole * period    \u003d now\n12h                4.00                    4                    0\n23h                7.67                    7                    4\n25h                8.33                    8                    8\n```\n\nWith epoch \u003d now on every pass, rebalancing more often than min_part_hours means whole is always 0, so nothing ever ages. If we only set it when whole \u003e 0, we still throw away up to a whole period each time - at 23h that ages half as fast as it should. Either way history sticks around longer than max_history_cycles allows.\n\nYou did find a real bug though. The remainder has to go when min_part_hours itself changes, because it\u0027s counted in the old period. _rescale_history_for_min_part_hours rescaled the cells but not the epoch, so going 24 to 1 turned 23 hours into 23 new cycles and aged out the whole table. PS4 resets the epoch whenever min_part_hours changes to a nonzero value, and documents the remainder and the skew you asked about.","commit_id":"f695fee2906c42a498ee34b595dda3caf83ab705"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"1485b1c114d2a1b74a7624d97a87310e8eb9edee","unresolved":true,"context_lines":[{"line_number":571,"context_line":"                    age_row[p] \u003d 255"},{"line_number":572,"context_line":"                else:"},{"line_number":573,"context_line":"                    age_row[p] \u003d new_val"},{"line_number":574,"context_line":"        self._history_epoch +\u003d whole * period"},{"line_number":575,"context_line":""},{"line_number":576,"context_line":"    def _record_voluntary_move(self, replica, part):"},{"line_number":577,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"302cb0d7_98237cc2","line":574,"range":{"start_line":574,"start_character":8,"end_line":574,"end_character":45},"in_reply_to":"f5815957_ebe5659e","updated":"2026-07-29 00:01:16.000000000","message":"OK, so are we doing this solely so the \"remainer\" is preserved, ie new history_epoch can lag behind now, by a little. And that\u0027ll be wrapped up in the next rebalance? So we\u0027re basically planning on rounding to the closest period.\n\nSounds interesting, but in realality we rebalanced not strictly at the last period but now..could that skew anyting?\n\nWe are saving _history_epoch, so saving the remainder does make sense. I can get on-board, just we\u0027re storing history to the nearest period, is there a way that could skew things. so `//` should perform a floor division on integers, so if we\u0027re almost at the next period, the cell will floor to the previous... so I think worst case is we\u0027ll end up tracking a previous history for a bit longer then expected, I guess that\u0027s ok. But it could potentially be an entire period.. at a min_part_hours of 24 hours, that could mean 23 hours 59 minutes and 59 seconds, it\u0027ll still floor and so we\u0027d still be reaching out to the old primary.\n\nFact that it\u0027s floor\u0027ing is probably better then rounding, where we could have the opposite problem, so not saying it\u0027s bad, just interesting. There might be a skew in metrics (when and if we track this sometimes).\n\nEitherway, maybe worth a comment about the remainder and maybe also about potential edgecase skew.","commit_id":"f695fee2906c42a498ee34b595dda3caf83ab705"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"78f9a8acc0b4a987ae3067d09369d36c03d2e6c7","unresolved":true,"context_lines":[{"line_number":926,"context_line":"        # Reconcile fresh placements before the age-out so newly-placed cells"},{"line_number":927,"context_line":"        # (age\u003d0, history\u003dcurrent) are not collapsed twice."},{"line_number":928,"context_line":"        self._reconcile_fresh_placements(old_replica2part2dev)"},{"line_number":929,"context_line":"        self._age_out_history_cells()"},{"line_number":930,"context_line":""},{"line_number":931,"context_line":"        self.devs_changed \u003d False"},{"line_number":932,"context_line":"        changed_parts \u003d self._build_dispersion_graph(old_replica2part2dev)"}],"source_content_type":"text/x-python","patch_set":3,"id":"210d6d70_dff8e44e","line":929,"updated":"2026-07-28 05:27:35.000000000","message":"I like how all these passes to gather/update/clean the history are all refactored into helper methods, but this the size of rings we\u0027re dealing with, I do feel we now check evey cell again and again and again.\nHow much longer have we increased rebalances? Surely we can combine some loops instead and reduce the number for full ring iterations?","commit_id":"f695fee2906c42a498ee34b595dda3caf83ab705"}]}