)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"e078d4bcd0ea514ec471a9dc00ad26b3eb0c3a60","unresolved":true,"context_lines":[{"line_number":11,"context_line":"whole cache structure and the hashes.invalid stuff half-way exist just"},{"line_number":12,"context_line":"to avoid a listdir in a part_dir (srly, go `time ls` in a part_dir in"},{"line_number":13,"context_line":"prod).  But at some point we noticed in a fresh cluster before you get"},{"line_number":14,"context_line":"all 4096 suffix dirs in every part when you rsync a whole new suffix"},{"line_number":15,"context_line":"behind a part but the remote-end crashes before sending the post-rsync"},{"line_number":16,"context_line":"REPLICATE request you could end up missing a suffix in the hashes.pkl"},{"line_number":17,"context_line":"(at least until it gets invalidated), and then you constantly appear out"},{"line_number":18,"context_line":"of sync to your neighbors (it was a whole thing); and the original"},{"line_number":19,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"c06459f1_8b0ec03b","line":16,"range":{"start_line":14,"start_character":35,"end_line":16,"end_character":17},"updated":"2025-05-16 03:49:13.000000000","message":"https://review.opendev.org/c/openstack/swift/+/839649 anyone? ;-)","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fb98042cf3c32c2f4b7dd96a8d8daadf9a1d5a83","unresolved":false,"context_lines":[{"line_number":11,"context_line":"whole cache structure and the hashes.invalid stuff half-way exist just"},{"line_number":12,"context_line":"to avoid a listdir in a part_dir (srly, go `time ls` in a part_dir in"},{"line_number":13,"context_line":"prod).  But at some point we noticed in a fresh cluster before you get"},{"line_number":14,"context_line":"all 4096 suffix dirs in every part when you rsync a whole new suffix"},{"line_number":15,"context_line":"behind a part but the remote-end crashes before sending the post-rsync"},{"line_number":16,"context_line":"REPLICATE request you could end up missing a suffix in the hashes.pkl"},{"line_number":17,"context_line":"(at least until it gets invalidated), and then you constantly appear out"},{"line_number":18,"context_line":"of sync to your neighbors (it was a whole thing); and the original"},{"line_number":19,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"574fa97a_8b7788f6","line":16,"range":{"start_line":14,"start_character":35,"end_line":16,"end_character":17},"in_reply_to":"c06459f1_8b0ec03b","updated":"2025-05-21 16:14:23.000000000","message":"Acknowledged","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"e078d4bcd0ea514ec471a9dc00ad26b3eb0c3a60","unresolved":true,"context_lines":[{"line_number":15,"context_line":"behind a part but the remote-end crashes before sending the post-rsync"},{"line_number":16,"context_line":"REPLICATE request you could end up missing a suffix in the hashes.pkl"},{"line_number":17,"context_line":"(at least until it gets invalidated), and then you constantly appear out"},{"line_number":18,"context_line":"of sync to your neighbors (it was a whole thing); and the original"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"    \"ok, *fine* we\u0027ll do the listdir *occasionally*\""},{"line_number":21,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"bf3737bc_9c0c8152","line":18,"range":{"start_line":18,"start_character":27,"end_line":18,"end_character":47},"updated":"2025-05-16 03:49:13.000000000","message":"I feel like we could link to some related change...","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"a7f53eef67541d34ec4b278d0e36bc4c8f2789fa","unresolved":true,"context_lines":[{"line_number":15,"context_line":"behind a part but the remote-end crashes before sending the post-rsync"},{"line_number":16,"context_line":"REPLICATE request you could end up missing a suffix in the hashes.pkl"},{"line_number":17,"context_line":"(at least until it gets invalidated), and then you constantly appear out"},{"line_number":18,"context_line":"of sync to your neighbors (it was a whole thing); and the original"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"    \"ok, *fine* we\u0027ll do the listdir *occasionally*\""},{"line_number":21,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"f69825f2_497c6108","line":18,"range":{"start_line":18,"start_character":27,"end_line":18,"end_character":47},"in_reply_to":"0adc2524_ac809033","updated":"2025-05-29 19:30:04.000000000","message":"https://github.com/openstack/swift/commit/8dcdaff64eee37fdbaf2e83bbe6fa07dbfe36bd8 seems pretty darn related.","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fb98042cf3c32c2f4b7dd96a8d8daadf9a1d5a83","unresolved":true,"context_lines":[{"line_number":15,"context_line":"behind a part but the remote-end crashes before sending the post-rsync"},{"line_number":16,"context_line":"REPLICATE request you could end up missing a suffix in the hashes.pkl"},{"line_number":17,"context_line":"(at least until it gets invalidated), and then you constantly appear out"},{"line_number":18,"context_line":"of sync to your neighbors (it was a whole thing); and the original"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"    \"ok, *fine* we\u0027ll do the listdir *occasionally*\""},{"line_number":21,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"0adc2524_ac809033","line":18,"range":{"start_line":18,"start_character":27,"end_line":18,"end_character":47},"in_reply_to":"bf3737bc_9c0c8152","updated":"2025-05-21 16:14:23.000000000","message":"IIRC the argument between redbo and letterj happened in the rackspace offices pre-openstack/bzr - AFAIK the relevant change doesn\u0027t exist in gerrit.\n\nWe could certainly omit the ancdata and stick to claims we can back-up with links to diffs; like Pavel\u0027s fix:\n\n402324: Fix non-deterministic suffix updates in hashes.pkl | https://review.opendev.org/c/openstack/swift/+/402324","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"e078d4bcd0ea514ec471a9dc00ad26b3eb0c3a60","unresolved":true,"context_lines":[{"line_number":20,"context_line":"    \"ok, *fine* we\u0027ll do the listdir *occasionally*\""},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"fix actually went through a couple of iterations before we got a"},{"line_number":23,"context_line":"working/fair \"10% is *more* than good enough\" implementation.  But in"},{"line_number":24,"context_line":"reality on most full-ish or busy clusters this is mostly wasted IO and"},{"line_number":25,"context_line":"you can turn this down to 1 or even .1 and be fine."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"The current implementation won\u0027t go lower than 1/10K, but you can set it"},{"line_number":28,"context_line":"to zero to disable this \"sanity check\" io penalty entirely."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"b1318c16_bd98ee43","line":25,"range":{"start_line":23,"start_character":67,"end_line":25,"end_character":50},"updated":"2025-05-16 03:49:13.000000000","message":"So why are we leaving the default at 10? Do we hate our operators or something?\n\nIf we *didn\u0027t* make it a config option, would we give it a second thought to change the hard-coded 10% to 1%? I feel like I\u0027d be OK with that and call it a day.","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fb98042cf3c32c2f4b7dd96a8d8daadf9a1d5a83","unresolved":true,"context_lines":[{"line_number":20,"context_line":"    \"ok, *fine* we\u0027ll do the listdir *occasionally*\""},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"fix actually went through a couple of iterations before we got a"},{"line_number":23,"context_line":"working/fair \"10% is *more* than good enough\" implementation.  But in"},{"line_number":24,"context_line":"reality on most full-ish or busy clusters this is mostly wasted IO and"},{"line_number":25,"context_line":"you can turn this down to 1 or even .1 and be fine."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"The current implementation won\u0027t go lower than 1/10K, but you can set it"},{"line_number":28,"context_line":"to zero to disable this \"sanity check\" io penalty entirely."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"594391c2_e9c5f204","line":25,"range":{"start_line":23,"start_character":67,"end_line":25,"end_character":50},"in_reply_to":"b1318c16_bd98ee43","updated":"2025-05-21 16:14:23.000000000","message":"\u003e So why are we leaving the default at 10?\n\nBecause it\u0027s been that way for a decade and no one has complained.\n\n\u003e Do we hate our operators or something?\n\nLOL\n\n\u003e would we give it a second thought to change the hard-coded 10% to 1%?\n\nI would hate that.  Any time I see a magic number that\u0027s making some choice that could easily be configurable I feel like making it configurable is a reasonable endeavor.  expirer_task_container_per_day, partition timeout... seems like everytime we decide we know the right number some new set of circumstances makes a different number look better.  There\u0027s only 3 numbers in computer software: 0, 1 and N.","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"e078d4bcd0ea514ec471a9dc00ad26b3eb0c3a60","unresolved":true,"context_lines":[{"line_number":27,"context_line":"The current implementation won\u0027t go lower than 1/10K, but you can set it"},{"line_number":28,"context_line":"to zero to disable this \"sanity check\" io penalty entirely."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Drive-by: the reconstructor did this listdir on EVERY part dir EVERY"},{"line_number":31,"context_line":"cycle - and it doesn\u0027t even use rsync!?"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"Change-Id: I275df03ac24839e9946781b0927bf1992f80137d"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"e8a7f595_3a44b764","line":31,"range":{"start_line":30,"start_character":0,"end_line":31,"end_character":39},"updated":"2025-05-16 03:49:13.000000000","message":"I thought this was the driving motivation for the change -- why\u0027s it just a little two-line drive-by?","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fb98042cf3c32c2f4b7dd96a8d8daadf9a1d5a83","unresolved":true,"context_lines":[{"line_number":27,"context_line":"The current implementation won\u0027t go lower than 1/10K, but you can set it"},{"line_number":28,"context_line":"to zero to disable this \"sanity check\" io penalty entirely."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Drive-by: the reconstructor did this listdir on EVERY part dir EVERY"},{"line_number":31,"context_line":"cycle - and it doesn\u0027t even use rsync!?"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"Change-Id: I275df03ac24839e9946781b0927bf1992f80137d"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"85d8ccdd_457a9bd9","line":31,"range":{"start_line":30,"start_character":0,"end_line":31,"end_character":39},"in_reply_to":"e8a7f595_3a44b764","updated":"2025-05-21 16:14:23.000000000","message":"after I wrote the change and looked at what it was doing it seemed like it was making the existing behavior configurable and then copying the pattern to the reconstructor.\n\nI\u0027m not attached to the commit message if you want to re-write it.","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"e078d4bcd0ea514ec471a9dc00ad26b3eb0c3a60","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"3b9b7762_39a3a04a","updated":"2025-05-16 03:49:13.000000000","message":"Good enough to carry, anyway.","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9670da9c1ac15b737cd2fd57df6f3195ee1c987c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"1addbb3c_ef50a842","updated":"2025-05-16 12:38:03.000000000","message":"Seems like a very prudent and useful option. I agree with Tim that if we think that the best default is \u003c10% then we could change what was previously hard-wired and make it an upgrade impact for ops that want to maintain 10%.\n\nThe implementation could be encapsulated in a util class that (a) reduces the duplication in replicator and reconstructor and (b) hides the seed initialisation from the caller. I played with idea here [1] but didn\u0027t take it all the way yet, just patched the reconstructor and tests.\n\n[1] https://paste.openstack.org/show/bB1bcIjdAopRJAlBGkO8/","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"794ff2a65d28d7df268d56a5ad6da5eb2e3c6a45","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"bc073758_352633ee","updated":"2025-05-16 14:55:09.000000000","message":"hopefully fixed the p3.7 test error","commit_id":"cfa589b10a01d507547ab544c5c7ca32c8c62370"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fb98042cf3c32c2f4b7dd96a8d8daadf9a1d5a83","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"c94e5534_11d29aaa","updated":"2025-05-21 16:14:23.000000000","message":"I\u0027d like to solicit ideas for what configuration exactly we want to expose; I think some kind of \"how often do you want to listdir parts\" is quite reasonable; but there\u0027s probably different ways to spell that.  Exposing it as precent but then making \"51% of cycles\" an equivalent behavior to \"99% of cycles\" in this implementation is strange.  Maybe \"how many cycles between listdir\" or \"steps\" would be an ok option name, but I feel like it leaves less flexibility to change the implementation.  If we decide percentage is the way to go we could probably take advantage of `config_percent_value` - but `should_part_listdir_decimal_fraction` is sort of an annoying variable to be carrying around.\n\nI think the \"we could use a class\" idea is promising; but functionally equivalent.\n\nI\u0027m unconvinced the implementation should be more abstract and in utils - there\u0027s lots of places we deal with parts across cycles tho; maybe if we can find another candidate for a similar \"intermittent behvioar\" it would be easier to make this helper function more of a utility.\n\nThanks for the help getting this out to SRE!","commit_id":"88888880f35ea65653ccabad45e4f8dc5bf48baa"}],"etc/object-server.conf-sample":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9670da9c1ac15b737cd2fd57df6f3195ee1c987c","unresolved":true,"context_lines":[{"line_number":349,"context_line":"#"},{"line_number":350,"context_line":"# Sanity check a partitions suffixes in hashes.pkl some small percentage of"},{"line_number":351,"context_line":"# cycles. Lower values are fine for busy or full clusters.  Minimum real value"},{"line_number":352,"context_line":"# is will floor at .01, but you can disable entirely by setting it to zero."},{"line_number":353,"context_line":"# part_listdir_percent \u003d 10"},{"line_number":354,"context_line":"#"},{"line_number":355,"context_line":"# handoffs_first and handoff_delete are options for a special case"}],"source_content_type":"application/octet-stream","patch_set":1,"id":"4a38aaab_2f274544","line":352,"range":{"start_line":352,"start_character":2,"end_line":352,"end_character":9},"updated":"2025-05-16 12:38:03.000000000","message":"typo: delete ``is``","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fb98042cf3c32c2f4b7dd96a8d8daadf9a1d5a83","unresolved":false,"context_lines":[{"line_number":349,"context_line":"#"},{"line_number":350,"context_line":"# Sanity check a partitions suffixes in hashes.pkl some small percentage of"},{"line_number":351,"context_line":"# cycles. Lower values are fine for busy or full clusters.  Minimum real value"},{"line_number":352,"context_line":"# is will floor at .01, but you can disable entirely by setting it to zero."},{"line_number":353,"context_line":"# part_listdir_percent \u003d 10"},{"line_number":354,"context_line":"#"},{"line_number":355,"context_line":"# handoffs_first and handoff_delete are options for a special case"}],"source_content_type":"application/octet-stream","patch_set":1,"id":"3566d4ea_07ba5618","line":352,"range":{"start_line":352,"start_character":2,"end_line":352,"end_character":9},"in_reply_to":"4a38aaab_2f274544","updated":"2025-05-21 16:14:23.000000000","message":"Done","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9670da9c1ac15b737cd2fd57df6f3195ee1c987c","unresolved":true,"context_lines":[{"line_number":453,"context_line":"#"},{"line_number":454,"context_line":"# Sanity check a partitions suffixes in hashes.pkl some small percentage of"},{"line_number":455,"context_line":"# cycles. Lower values are fine for busy or full clusters.  Minimum real value"},{"line_number":456,"context_line":"# is will floor at .01, but you can disable entirely by setting it to zero."},{"line_number":457,"context_line":"# part_listdir_percent \u003d 10"},{"line_number":458,"context_line":"#"},{"line_number":459,"context_line":"# You can set scheduling priority of processes. Niceness values range from -20"}],"source_content_type":"application/octet-stream","patch_set":1,"id":"678c255f_6269d03e","line":456,"range":{"start_line":456,"start_character":2,"end_line":456,"end_character":4},"updated":"2025-05-16 12:38:03.000000000","message":"ditto typo","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fb98042cf3c32c2f4b7dd96a8d8daadf9a1d5a83","unresolved":false,"context_lines":[{"line_number":453,"context_line":"#"},{"line_number":454,"context_line":"# Sanity check a partitions suffixes in hashes.pkl some small percentage of"},{"line_number":455,"context_line":"# cycles. Lower values are fine for busy or full clusters.  Minimum real value"},{"line_number":456,"context_line":"# is will floor at .01, but you can disable entirely by setting it to zero."},{"line_number":457,"context_line":"# part_listdir_percent \u003d 10"},{"line_number":458,"context_line":"#"},{"line_number":459,"context_line":"# You can set scheduling priority of processes. Niceness values range from -20"}],"source_content_type":"application/octet-stream","patch_set":1,"id":"b6664787_0e2f06eb","line":456,"range":{"start_line":456,"start_character":2,"end_line":456,"end_character":4},"in_reply_to":"678c255f_6269d03e","updated":"2025-05-21 16:14:23.000000000","message":"Done","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"}],"swift/obj/diskfile.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"e078d4bcd0ea514ec471a9dc00ad26b3eb0c3a60","unresolved":true,"context_lines":[{"line_number":378,"context_line":"    return hashes"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":""},{"line_number":381,"context_line":"def should_part_listdir(seed, cycle, part, pct):"},{"line_number":382,"context_line":"    if pct \u003c\u003d 0:"},{"line_number":383,"context_line":"        # bold choice, but ok"},{"line_number":384,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"840d8290_8d771d6e","line":381,"range":{"start_line":381,"start_character":24,"end_line":381,"end_character":46},"updated":"2025-05-16 03:49:13.000000000","message":"We really need a docstring, if only to give expected types. At first glance I expected `seed` to be an int (like `PYTHONHASHSEED`) and thought it would have no effect at all.","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9670da9c1ac15b737cd2fd57df6f3195ee1c987c","unresolved":true,"context_lines":[{"line_number":378,"context_line":"    return hashes"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":""},{"line_number":381,"context_line":"def should_part_listdir(seed, cycle, part, pct):"},{"line_number":382,"context_line":"    if pct \u003c\u003d 0:"},{"line_number":383,"context_line":"        # bold choice, but ok"},{"line_number":384,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"c7126ed6_b86bb385","line":381,"range":{"start_line":381,"start_character":24,"end_line":381,"end_character":46},"in_reply_to":"840d8290_8d771d6e","updated":"2025-05-16 12:38:03.000000000","message":"I\u0027m not sure I understand why this is in diskfile.py\n\nThis seems to be a utility function that returns True for 1 in N values of a cycle counter, with provision for offsets into the counter.\n\nIt reminds of some of the stuff we have to distribute expirer containers over processes. I think it could be in utils: https://paste.openstack.org/show/bB1bcIjdAopRJAlBGkO8/","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fb98042cf3c32c2f4b7dd96a8d8daadf9a1d5a83","unresolved":true,"context_lines":[{"line_number":378,"context_line":"    return hashes"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":""},{"line_number":381,"context_line":"def should_part_listdir(seed, cycle, part, pct):"},{"line_number":382,"context_line":"    if pct \u003c\u003d 0:"},{"line_number":383,"context_line":"        # bold choice, but ok"},{"line_number":384,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"508dc4bc_77af9122","line":381,"range":{"start_line":381,"start_character":24,"end_line":381,"end_character":46},"in_reply_to":"c7126ed6_b86bb385","updated":"2025-05-21 16:14:23.000000000","message":"\u003e We really need a docstring\n\nsounds right to me!\n\n\u003e I\u0027m not sure I understand why this is in diskfile.py\n\nbecause of how it\u0027s used and how it\u0027s named\n\n\u003e that returns True for 1 in N values of a cycle counter, with provision for offsets into the counter.\n\nOR \"it\u0027s a function that returns when you should listdir\"... which has some rules and some tests in test_diskfile\n\n\u003e I think it could be in utils\n\nyeah I think it could, maybe even *should* ... if it ever gets another use-case.\n\nFWIW I like the class hiding the seed, but don\u0027t want to encapsulate the process_cycle attribute which I think has a lot of potential for other intermittent activities.\n\nMaybe:\n\n```\nself.should_part_listdir \u003d PartCycler(cycle_pct)\n\ndo_listdir\u003dself.should_part_listdir(part, self.process_cycle)\n\nself.process_cycle +\u003d 1\n```","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9670da9c1ac15b737cd2fd57df6f3195ee1c987c","unresolved":true,"context_lines":[{"line_number":381,"context_line":"def should_part_listdir(seed, cycle, part, pct):"},{"line_number":382,"context_line":"    if pct \u003c\u003d 0:"},{"line_number":383,"context_line":"        # bold choice, but ok"},{"line_number":384,"context_line":"        return False"},{"line_number":385,"context_line":"    index \u003d int(seed * 10_000 + cycle + int(part)) % 10_000"},{"line_number":386,"context_line":"    steps \u003d max(1, int(1 / pct * 100))"},{"line_number":387,"context_line":"    return index % steps \u003d\u003d 0"}],"source_content_type":"text/x-python","patch_set":1,"id":"3769c661_74f087fa","line":384,"updated":"2025-05-16 12:38:03.000000000","message":"maybe \n\n```\nelif pct \u003e\u003d 100:\n    return True\nelse:\n    index \u003d int(seed * 10_000 + cycle + int(part)) % 10_000\n    steps \u003d int(1 / pct * 100)\n    return index % steps \u003d\u003d 0\n```\n\n...easier than needing to grok the significance of the max at line 386","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fb98042cf3c32c2f4b7dd96a8d8daadf9a1d5a83","unresolved":false,"context_lines":[{"line_number":381,"context_line":"def should_part_listdir(seed, cycle, part, pct):"},{"line_number":382,"context_line":"    if pct \u003c\u003d 0:"},{"line_number":383,"context_line":"        # bold choice, but ok"},{"line_number":384,"context_line":"        return False"},{"line_number":385,"context_line":"    index \u003d int(seed * 10_000 + cycle + int(part)) % 10_000"},{"line_number":386,"context_line":"    steps \u003d max(1, int(1 / pct * 100))"},{"line_number":387,"context_line":"    return index % steps \u003d\u003d 0"}],"source_content_type":"text/x-python","patch_set":1,"id":"b6622be3_ccba4dab","line":384,"in_reply_to":"3769c661_74f087fa","updated":"2025-05-21 16:14:23.000000000","message":"sure","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"e078d4bcd0ea514ec471a9dc00ad26b3eb0c3a60","unresolved":true,"context_lines":[{"line_number":384,"context_line":"        return False"},{"line_number":385,"context_line":"    index \u003d int(seed * 10_000 + cycle + int(part)) % 10_000"},{"line_number":386,"context_line":"    steps \u003d max(1, int(1 / pct * 100))"},{"line_number":387,"context_line":"    return index % steps \u003d\u003d 0"},{"line_number":388,"context_line":""},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"def write_hashes(partition_dir, hashes):"}],"source_content_type":"text/x-python","patch_set":1,"id":"261b575b_b3944593","line":387,"updated":"2025-05-16 03:49:13.000000000","message":"I think we want something more like\n```\n    steps \u003d max(1, int(1 / pct * 100))\n    return int(seed * steps + cycle + int(part)) % steps \u003d\u003d 0\n```\nThen ops can set `pct` arbitrarily low and you don\u0027t get into weird edge cases when `steps` turns out to be 9999 or something and you do the listdir two cycles in a row.","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"31f4c36e250d95134a3a77b044af6a3160eea8f7","unresolved":true,"context_lines":[{"line_number":384,"context_line":"        return False"},{"line_number":385,"context_line":"    index \u003d int(seed * 10_000 + cycle + int(part)) % 10_000"},{"line_number":386,"context_line":"    steps \u003d max(1, int(1 / pct * 100))"},{"line_number":387,"context_line":"    return index % steps \u003d\u003d 0"},{"line_number":388,"context_line":""},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"def write_hashes(partition_dir, hashes):"}],"source_content_type":"text/x-python","patch_set":1,"id":"8f32f8e0_1216ebc8","line":387,"in_reply_to":"261b575b_b3944593","updated":"2025-05-16 15:30:45.000000000","message":"Thinking about this more, the fact that `steps` is always the same for every meta-cycle leads to some weird edge cases anyway -- if an operator says `part_listdir_percent \u003d 90` (or any largish number down to some iota over `50`), they\u0027re likely going to be confused about why they don\u0027t see any difference from `100`. I don\u0027t think it matters _too_ much in the ranges we feel are appropriate (OK, there\u0027s no `12`, only `12.5` or `11.11` -- no big whoop), but I think we\u0027re setting operators up for confusion.\n\nI feel like `steps` should be the config option -- `forced_rehash_period` maybe?","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0279c297ace3fa61519ba995a9876d2b76cbbaee","unresolved":true,"context_lines":[{"line_number":384,"context_line":"        return False"},{"line_number":385,"context_line":"    index \u003d int(seed * 10_000 + cycle + int(part)) % 10_000"},{"line_number":386,"context_line":"    steps \u003d max(1, int(1 / pct * 100))"},{"line_number":387,"context_line":"    return index % steps \u003d\u003d 0"},{"line_number":388,"context_line":""},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"def write_hashes(partition_dir, hashes):"}],"source_content_type":"text/x-python","patch_set":1,"id":"96cd8aee_62c0ddfc","line":387,"in_reply_to":"787f1f5e_2ccb2ef6","updated":"2025-05-21 19:43:28.000000000","message":"\u003e you mean replacing the \"pct\" based approximation with a disjoint real int to better match the implementation?\n\n💯\n\n\u003e I guess the main WTF would be 51% and 99% both mean \"do the listdir every cycle\" - that kinda sucks.\n\nYes, this, exactly this. If we\u0027re going to have a fixed number of cycles between rehashes (which seems like a reasonably good idea) then we may as well make *that* the config option, rather than some percentage that doesn\u0027t actually reflect reality. That would also make it easier for ops to know how long to expect to have to wait to get everything rehashed -- that config option multiplied by cycle time.\n\nFWIW, I could imagine an implementation that allows for a variable (but very tightly bounded) number of cycles between rehashes that would make it so 90% actually means 90% -- but I don\u0027t think it would be time well-spent.","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fb98042cf3c32c2f4b7dd96a8d8daadf9a1d5a83","unresolved":true,"context_lines":[{"line_number":384,"context_line":"        return False"},{"line_number":385,"context_line":"    index \u003d int(seed * 10_000 + cycle + int(part)) % 10_000"},{"line_number":386,"context_line":"    steps \u003d max(1, int(1 / pct * 100))"},{"line_number":387,"context_line":"    return index % steps \u003d\u003d 0"},{"line_number":388,"context_line":""},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"def write_hashes(partition_dir, hashes):"}],"source_content_type":"text/x-python","patch_set":1,"id":"787f1f5e_2ccb2ef6","line":387,"in_reply_to":"8f32f8e0_1216ebc8","updated":"2025-05-21 16:14:23.000000000","message":"\u003e forced_rehash_period maybe\n\nadding a 2nd config option doesn\u0027t sound good to me; you mean replacing the \"pct\" based approximation with a disjoint real int to better match the implementation?  Seems like the tail wagging the dog - maybe my implementation just isn\u0027t very good?\n\n\u003e they\u0027re likely going to be confused about why they don\u0027t see any difference from 100\n\nI\u0027m skeptical anyone is paying that close of attention; but a test might convince me there\u0027s an awkward enough interaction that we should care?  I guess the main WTF would be 51% and 99% both mean \"do the listdir every cycle\" - that kinda sucks.\n\n\u003e  I don\u0027t think it matters too much in the ranges we feel are appropriate\n\nI agree, but perhaps making the config value a contiguous float isn\u0027t that helpful at communicating there\u0027s \"only\" N different options, and we think most of the interesting ones are small.  Or maybe this is just a bad implementation of the implied idea that the % of cycles we listdir a given part should be configurable.","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"a5f4c1fe5430fe035d49da773e6a40cfbcb1dca3","unresolved":true,"context_lines":[{"line_number":413,"context_line":"    we want to listdir on some configured pct of parts and given enough cycles"},{"line_number":414,"context_line":"    we want to listdir every part."},{"line_number":415,"context_line":""},{"line_number":416,"context_line":"    :param seed: float, random.random() will do fine"},{"line_number":417,"context_line":"    :param cycle: int, counter of how many times you\u0027ve looped in run_forever"},{"line_number":418,"context_line":"    :param part: the partition number (often from listdir, so it might be digit as a str)"},{"line_number":419,"context_line":"    :param pct: float, zero or positive value; larger values more likely to"}],"source_content_type":"text/x-python","patch_set":9,"id":"b24fbabd_b3ad616c","line":416,"updated":"2025-05-21 19:48:09.000000000","message":"\u003e ...but should be held constant while you\u0027re incrementing `cycle`\n\nor\n\n\u003e .... Choose this once during process start-up.","commit_id":"bbf4c2fbd6a30ea957dd33076a8fd473516fc897"}],"swift/obj/reconstructor.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"e078d4bcd0ea514ec471a9dc00ad26b3eb0c3a60","unresolved":true,"context_lines":[{"line_number":1152,"context_line":"        # find all the fi\u0027s in the part, and which suffixes have them"},{"line_number":1153,"context_line":"        try:"},{"line_number":1154,"context_line":"            hashes \u003d self._get_hashes(local_dev[\u0027device\u0027], partition, policy,"},{"line_number":1155,"context_line":"                                      do_listdir\u003dTrue)"},{"line_number":1156,"context_line":"        except OSError as e:"},{"line_number":1157,"context_line":"            if e.errno !\u003d errno.ENOTDIR:"},{"line_number":1158,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":1,"id":"f6a678d5_dd2de75a","side":"PARENT","line":1155,"range":{"start_line":1155,"start_character":38,"end_line":1155,"end_character":53},"updated":"2025-05-16 03:49:13.000000000","message":"Yowza! We used to do it *all the damn time!?*","commit_id":"c12aa812c9f434d4584e65d48c8f77b6c8c37862"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fb98042cf3c32c2f4b7dd96a8d8daadf9a1d5a83","unresolved":false,"context_lines":[{"line_number":1152,"context_line":"        # find all the fi\u0027s in the part, and which suffixes have them"},{"line_number":1153,"context_line":"        try:"},{"line_number":1154,"context_line":"            hashes \u003d self._get_hashes(local_dev[\u0027device\u0027], partition, policy,"},{"line_number":1155,"context_line":"                                      do_listdir\u003dTrue)"},{"line_number":1156,"context_line":"        except OSError as e:"},{"line_number":1157,"context_line":"            if e.errno !\u003d errno.ENOTDIR:"},{"line_number":1158,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf22094e_7be6ef2b","side":"PARENT","line":1155,"range":{"start_line":1155,"start_character":38,"end_line":1155,"end_character":53},"in_reply_to":"f6a678d5_dd2de75a","updated":"2025-05-21 16:14:23.000000000","message":"yup.","commit_id":"c12aa812c9f434d4584e65d48c8f77b6c8c37862"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9670da9c1ac15b737cd2fd57df6f3195ee1c987c","unresolved":true,"context_lines":[{"line_number":197,"context_line":"                    \u0027object-reconstructor/interval.\u0027)"},{"line_number":198,"context_line":"        self.process_cycle \u003d 0"},{"line_number":199,"context_line":"        self.cycle_seed \u003d random.random()"},{"line_number":200,"context_line":"        self.part_listdir_percent \u003d float(conf.get(\u0027part_listdir_percent\u0027, 10))"},{"line_number":201,"context_line":"        self.http_timeout \u003d int(conf.get(\u0027http_timeout\u0027, 60))"},{"line_number":202,"context_line":"        self.lockup_timeout \u003d int(conf.get(\u0027lockup_timeout\u0027, 1800))"},{"line_number":203,"context_line":"        self.recon_cache_path \u003d conf.get(\u0027recon_cache_path\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"2cf9faae_68fe0f14","line":200,"range":{"start_line":200,"start_character":36,"end_line":200,"end_character":41},"updated":"2025-05-16 12:38:03.000000000","message":"we have swift.common.utils.config.config_percent_value","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fb98042cf3c32c2f4b7dd96a8d8daadf9a1d5a83","unresolved":true,"context_lines":[{"line_number":197,"context_line":"                    \u0027object-reconstructor/interval.\u0027)"},{"line_number":198,"context_line":"        self.process_cycle \u003d 0"},{"line_number":199,"context_line":"        self.cycle_seed \u003d random.random()"},{"line_number":200,"context_line":"        self.part_listdir_percent \u003d float(conf.get(\u0027part_listdir_percent\u0027, 10))"},{"line_number":201,"context_line":"        self.http_timeout \u003d int(conf.get(\u0027http_timeout\u0027, 60))"},{"line_number":202,"context_line":"        self.lockup_timeout \u003d int(conf.get(\u0027lockup_timeout\u0027, 1800))"},{"line_number":203,"context_line":"        self.recon_cache_path \u003d conf.get(\u0027recon_cache_path\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"2f2e41b4_eac38ebb","line":200,"range":{"start_line":200,"start_character":36,"end_line":200,"end_character":41},"in_reply_to":"2cf9faae_68fe0f14","updated":"2025-05-21 16:14:23.000000000","message":"that\u0027s interesting...\n\n```\n159:        self.etag_validate_frac \u003d config_percent_value(\n242:        self.container_existence_skip_cache \u003d config_percent_value( \n```\n\nit seems to do the `/ 100` normalization for me, so I\u0027d have to rework the handling in the helper.","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"}],"swift/obj/replicator.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9670da9c1ac15b737cd2fd57df6f3195ee1c987c","unresolved":true,"context_lines":[{"line_number":157,"context_line":"                    \u0027Option object-replicator/run_pause is deprecated and \u0027"},{"line_number":158,"context_line":"                    \u0027will be removed in a future version. Update your \u0027"},{"line_number":159,"context_line":"                    \u0027configuration to use option object-replicator/interval.\u0027)"},{"line_number":160,"context_line":"        self.process_cycle \u003d 0"},{"line_number":161,"context_line":"        self.cycle_seed \u003d random.random()"},{"line_number":162,"context_line":"        self.part_listdir_percent \u003d float(conf.get(\u0027part_listdir_percent\u0027, 10))"},{"line_number":163,"context_line":"        self.rsync_timeout \u003d int(conf.get(\u0027rsync_timeout\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"0109d170_e56fc67c","line":160,"updated":"2025-05-16 12:38:03.000000000","message":"off-topic: shame the replicator and reconstructor don\u0027t share any common code","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fb98042cf3c32c2f4b7dd96a8d8daadf9a1d5a83","unresolved":false,"context_lines":[{"line_number":157,"context_line":"                    \u0027Option object-replicator/run_pause is deprecated and \u0027"},{"line_number":158,"context_line":"                    \u0027will be removed in a future version. Update your \u0027"},{"line_number":159,"context_line":"                    \u0027configuration to use option object-replicator/interval.\u0027)"},{"line_number":160,"context_line":"        self.process_cycle \u003d 0"},{"line_number":161,"context_line":"        self.cycle_seed \u003d random.random()"},{"line_number":162,"context_line":"        self.part_listdir_percent \u003d float(conf.get(\u0027part_listdir_percent\u0027, 10))"},{"line_number":163,"context_line":"        self.rsync_timeout \u003d int(conf.get(\u0027rsync_timeout\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"450d502c_d3feac2d","line":160,"in_reply_to":"0109d170_e56fc67c","updated":"2025-05-21 16:14:23.000000000","message":"Acknowledged","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"e078d4bcd0ea514ec471a9dc00ad26b3eb0c3a60","unresolved":true,"context_lines":[{"line_number":159,"context_line":"                    \u0027configuration to use option object-replicator/interval.\u0027)"},{"line_number":160,"context_line":"        self.process_cycle \u003d 0"},{"line_number":161,"context_line":"        self.cycle_seed \u003d random.random()"},{"line_number":162,"context_line":"        self.part_listdir_percent \u003d float(conf.get(\u0027part_listdir_percent\u0027, 10))"},{"line_number":163,"context_line":"        self.rsync_timeout \u003d int(conf.get(\u0027rsync_timeout\u0027,"},{"line_number":164,"context_line":"                                          DEFAULT_RSYNC_TIMEOUT))"},{"line_number":165,"context_line":"        self.rsync_io_timeout \u003d conf.get(\u0027rsync_io_timeout\u0027, \u002730\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"6159e417_c085a7f3","line":162,"range":{"start_line":162,"start_character":75,"end_line":162,"end_character":77},"updated":"2025-05-16 03:49:13.000000000","message":"So this matches existing behavior -- but do we have a sense that it\u0027s \"right\" for production clusters? I feel like we can get away with changing the behavior if there\u0027s a better default -- *especially* for the reconstructor which was previously at 100!\n\nProbably the sort of thing where we\u0027ll need to carry the patch for a bit and solicit feedback from SRE.","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fb98042cf3c32c2f4b7dd96a8d8daadf9a1d5a83","unresolved":true,"context_lines":[{"line_number":159,"context_line":"                    \u0027configuration to use option object-replicator/interval.\u0027)"},{"line_number":160,"context_line":"        self.process_cycle \u003d 0"},{"line_number":161,"context_line":"        self.cycle_seed \u003d random.random()"},{"line_number":162,"context_line":"        self.part_listdir_percent \u003d float(conf.get(\u0027part_listdir_percent\u0027, 10))"},{"line_number":163,"context_line":"        self.rsync_timeout \u003d int(conf.get(\u0027rsync_timeout\u0027,"},{"line_number":164,"context_line":"                                          DEFAULT_RSYNC_TIMEOUT))"},{"line_number":165,"context_line":"        self.rsync_io_timeout \u003d conf.get(\u0027rsync_io_timeout\u0027, \u002730\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"e8abd8ec_5111dbe2","line":162,"range":{"start_line":162,"start_character":75,"end_line":162,"end_character":77},"in_reply_to":"6159e417_c085a7f3","updated":"2025-05-21 16:14:23.000000000","message":"\u003e do we have a sense that it\u0027s \"right\" for production clusters\n\nit hasn\u0027t been a problem for 10 years; leave well enough alone might be justification to keep it where it is - always easier to merge something that keeps things working as is by default - in theory it\u0027s one less thing to argue about.\n\n\u003e if there\u0027s a better default\n\nI\u0027m looking forward to spinning the knob and seeing if we can see the difference in disk utilization; If I can provide compelling justification the default should change we can do that later.\n\n\u003e the reconstructor which was previously at 100\n\nwell... I *am* suggesting we change *that one* (to match the replicator)\n\nHonestly, as best I can tell once you have 4096 suffix in the part the listdir can\u0027t really do anything... We could probably eventually change the implementation to move the responsibility down into the get_hashes function and have it \"disable\" the listdir if there\u0027s already 4096 suffixes in the hash structure.\n\nIn the meantime, making the reconstructer do what the replicator does and making them both configurable to enable additional experimentation seemed like the most prudent path forward.","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0279c297ace3fa61519ba995a9876d2b76cbbaee","unresolved":true,"context_lines":[{"line_number":159,"context_line":"                    \u0027configuration to use option object-replicator/interval.\u0027)"},{"line_number":160,"context_line":"        self.process_cycle \u003d 0"},{"line_number":161,"context_line":"        self.cycle_seed \u003d random.random()"},{"line_number":162,"context_line":"        self.part_listdir_percent \u003d float(conf.get(\u0027part_listdir_percent\u0027, 10))"},{"line_number":163,"context_line":"        self.rsync_timeout \u003d int(conf.get(\u0027rsync_timeout\u0027,"},{"line_number":164,"context_line":"                                          DEFAULT_RSYNC_TIMEOUT))"},{"line_number":165,"context_line":"        self.rsync_io_timeout \u003d conf.get(\u0027rsync_io_timeout\u0027, \u002730\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"250381c5_cd2d0307","line":162,"range":{"start_line":162,"start_character":75,"end_line":162,"end_character":77},"in_reply_to":"e8abd8ec_5111dbe2","updated":"2025-05-21 19:43:28.000000000","message":"\u003e Honestly, as best I can tell once you have 4096 suffix in the part the listdir can\u0027t really do anything... We could probably eventually change the implementation to move the responsibility down into the get_hashes function and have it \"disable\" the listdir if there\u0027s already 4096 suffixes in the hash structure.\n\nI don\u0027t think that can quite be the answer, either -- there\u0027s nothing magic about 4096 as far as the listdir is concerned; 4095 or even just 4000 seem about as likely turn the listdir into a tarpit. In general, I rather had the impression that the \"solution\" to dense suffixes was a part-power increase.\n\nFWIW, I expect this to be more and more of an issue as individual disk capacity grows -- it\u0027s (your?) old observation that the LOSF problem is really a LOF problem and OVH just noticed it with small files first.","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"}],"test/unit/obj/test_diskfile.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9670da9c1ac15b737cd2fd57df6f3195ee1c987c","unresolved":true,"context_lines":[{"line_number":861,"context_line":"        # int seed unexpected, and struggles with no cycle or part"},{"line_number":862,"context_line":"        self.assertTrue(should_part_listdir(894, 0.0, 0, 1e-300))"},{"line_number":863,"context_line":"        # but random seed does ok"},{"line_number":864,"context_line":"        self.assertFalse(should_part_listdir(random(), 0.0, 0, 1e-300))"},{"line_number":865,"context_line":""},{"line_number":866,"context_line":"    def test_should_part_listdir_seed_helps_restarts(self):"},{"line_number":867,"context_line":"        # for this test it doesn\u0027t matter that these are random so much as"}],"source_content_type":"text/x-python","patch_set":1,"id":"bab1035d_744b5319","line":864,"updated":"2025-05-16 12:38:03.000000000","message":"sounds like the choice of seed is significant and might be better hidden from the caller?","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fb98042cf3c32c2f4b7dd96a8d8daadf9a1d5a83","unresolved":true,"context_lines":[{"line_number":861,"context_line":"        # int seed unexpected, and struggles with no cycle or part"},{"line_number":862,"context_line":"        self.assertTrue(should_part_listdir(894, 0.0, 0, 1e-300))"},{"line_number":863,"context_line":"        # but random seed does ok"},{"line_number":864,"context_line":"        self.assertFalse(should_part_listdir(random(), 0.0, 0, 1e-300))"},{"line_number":865,"context_line":""},{"line_number":866,"context_line":"    def test_should_part_listdir_seed_helps_restarts(self):"},{"line_number":867,"context_line":"        # for this test it doesn\u0027t matter that these are random so much as"}],"source_content_type":"text/x-python","patch_set":1,"id":"f3da3955_977cc8a0","line":864,"in_reply_to":"bab1035d_744b5319","updated":"2025-05-21 16:14:23.000000000","message":"FWIW, this was more me trying to understand what I\u0027d written than make any kinds of claims about it\u0027s virtue - if anything I\u0027d recognized it was kind of wonky and wanted to call it out...\n\nperhaps good enough reason to make it a class - somehow that hadn\u0027t occurred to me (general disdain of classes?); I had considered at least stashing some global state somewhere; but thought it was easier to test just passing it into a pure function.","commit_id":"2d428f5d6c16fb2c795026682b540b1fe8e54ace"}]}