)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"31a6461677b008783d9da6200cfab6107ee06351","unresolved":true,"context_lines":[{"line_number":12,"context_line":"for parallel evaluation.  Each expirer process is more likely to process"},{"line_number":13,"context_line":"the tasks they list."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Drive-by: remove randomized_task_container_iteration as an option, since"},{"line_number":16,"context_line":"each task_container is handled only one expirer (or a very small small"},{"line_number":17,"context_line":"subset) it no longer adds value or seems worth the effort to maintain."},{"line_number":18,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"44dbb062_005e05f5","line":15,"range":{"start_line":15,"start_character":10,"end_line":15,"end_character":65},"updated":"2024-05-09 16:58:34.000000000","message":"That change hasn\u0027t landed yet -- these should be squashed together, so the option never exists on master. That will also make it more clear that the sample configs shouldn\u0027t mention the non-existent option.","commit_id":"d7987e1dffe9d606a14a39afc4477563216c1291"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"1d1308cf04a454c509115a60d67be827bf949d9b","unresolved":false,"context_lines":[{"line_number":12,"context_line":"for parallel evaluation.  Each expirer process is more likely to process"},{"line_number":13,"context_line":"the tasks they list."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Drive-by: remove randomized_task_container_iteration as an option, since"},{"line_number":16,"context_line":"each task_container is handled only one expirer (or a very small small"},{"line_number":17,"context_line":"subset) it no longer adds value or seems worth the effort to maintain."},{"line_number":18,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ea988051_e59d4e42","line":15,"range":{"start_line":15,"start_character":10,"end_line":15,"end_character":65},"in_reply_to":"44dbb062_005e05f5","updated":"2024-05-15 03:14:53.000000000","message":"I don\u0027t want to squash into the parent until we\u0027re ready to carry this strategy/experiment - I agree we don\u0027t have to land them in the order/shape we\u0027re carrying them.","commit_id":"d7987e1dffe9d606a14a39afc4477563216c1291"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"84eb3687a8998770892eaea3a96dda97ddb8ab92","unresolved":true,"context_lines":[{"line_number":10,"context_line":"a subset of deletes, we evaluate the expiring_objects container listings"},{"line_number":11,"context_line":"in order to distribute the iteration of task_containers to all processes"},{"line_number":12,"context_line":"for parallel evaluation. Each expirer process is more likely to process"},{"line_number":13,"context_line":"the tasks they list."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Co-Authored-By: Alistair Coles \u003calistairncoles@gmail.com\u003e"},{"line_number":16,"context_line":"Co-Authored-By: Jianjian Huo \u003cjhuo@nvidia.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":28,"id":"807904c9_55a360df","line":13,"updated":"2024-11-01 21:06:35.000000000","message":"deserves an UpgradeImpact because of the change in behavior and a note about the new \"get of jail free/keep legacy behavior\" option","commit_id":"75fd0ddd1798398d20ee37b6e9c80596b1192e76"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"6b554579556cd5f491058e5f3e052ad491ccae3a","unresolved":false,"context_lines":[{"line_number":10,"context_line":"a subset of deletes, we evaluate the expiring_objects container listings"},{"line_number":11,"context_line":"in order to distribute the iteration of task_containers to all processes"},{"line_number":12,"context_line":"for parallel evaluation. Each expirer process is more likely to process"},{"line_number":13,"context_line":"the tasks they list."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Co-Authored-By: Alistair Coles \u003calistairncoles@gmail.com\u003e"},{"line_number":16,"context_line":"Co-Authored-By: Jianjian Huo \u003cjhuo@nvidia.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":28,"id":"28a8ca35_1a767140","line":13,"in_reply_to":"807904c9_55a360df","updated":"2024-11-05 00:25:56.000000000","message":"Done","commit_id":"75fd0ddd1798398d20ee37b6e9c80596b1192e76"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"5a034f2024afb5b5980d5e5f7b94e9279f666c69","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"0b4aa4fe_def7238f","updated":"2024-05-09 11:09:29.000000000","message":"@Clay sorry, not a proper review, just a thought. The idea of selecting work by container rather than by task seems pretty good to me!","commit_id":"d7987e1dffe9d606a14a39afc4477563216c1291"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"1d1308cf04a454c509115a60d67be827bf949d9b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"fa9b5f91_cb896c2f","updated":"2024-05-15 03:14:53.000000000","message":"I don\u0027t think the ability to configure EXPIRER_CONTAINER_PER_DIVISOR is going to help much until we get a `task_container_iteration_strategy \u003d parallel` so maybe we should focus on the code we HAVE rather than the code we might have to write if just doing the simplist thing that could possibly work isn\u0027t good enough.\n\nI\u0027m already streching scope here adding in the the \"_get_all_expected_task_containers\" complexity - and I could imagine other strategies for distributing the found containers amoung processes.\n\nBecause there\u0027s currently no \"jitter\" in the task_container assignment the low process number nodes will ALWAYS be more likely to share containers than the higher process number nodes.\n\nI\u0027m not sure this strategy makes the existing expirer down process \u003d N node \"robustness problem\" any WORSE, but it sure doesn\u0027t make it better.  I think if we\u0027re going to scope creep I\u0027d like to brainstorm ideas in that domain.\n\nPretty sure if we could get the pre-reqs sorted out we could carry this and it\u0027d already be loads better for our cycle-time as is.","commit_id":"cf34af352cb9db637fbcfca06272279d0406c639"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"d64c7eea64048207adc961f267fe4c07f357f77c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"c954ddb9_58d4802c","updated":"2024-06-04 19:24:37.000000000","message":"parallel is a great strategy, but I have some concerns on the current selection algorithms working with skewed delete task distributions. described one idea to help in the comments.","commit_id":"4482a4b60b1935d5f0e950d6b4d136465b2962aa"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f950c1e90422b1d2679334da66c43dd6d865e46e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"33546312_d9251623","updated":"2024-07-22 17:32:28.000000000","message":"@Clay I think I understood this and it still seems like a good experiment!\n\nIdea for a simpler (?) implementation https://review.opendev.org/c/openstack/swift/+/924667","commit_id":"f5a63a1b4f12987d22cbbf613cd3d7673ae2eba8"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"68f2e6b601cc9d499d020066d94fdc647d58cf32","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"35ae3f89_cfa685aa","updated":"2024-07-25 06:07:26.000000000","message":"Very excited to see both of you supporting this approach of distributing containers per day evenly to processes, both two implementations are correct in logic. I add some comments and refactoring based on the last patch: https://review.opendev.org/c/openstack/swift/+/924903\n\nwill finish reviewing test cases later.","commit_id":"1d6cc6936c13e3deea674d8e93caa32599097d15"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"17483026f37f04a4e6453f3263b36608c87a08d0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"18b0064b_9a825c22","updated":"2024-07-30 18:07:38.000000000","message":"A great strategy to distribute expensive container listing to all expirer processes, so each process will only scan a subset (not all) of task containers, look forward to seeing the impact in the production.","commit_id":"4bb01dd96a5da61b753baae380d1ab614f7734d8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c35cf6bde61c0905c5315a70990602a26b8fe9ea","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"590506fc_a0ef9423","updated":"2024-10-24 18:54:34.000000000","message":"I\u0027m going to push a new version of this; but nothing really is chaning here (yet?)","commit_id":"3db5c137d98880ac9e3b7d16a1302384a5930c1a"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"84eb3687a8998770892eaea3a96dda97ddb8ab92","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":28,"id":"a167b754_5be245f4","updated":"2024-11-01 21:06:35.000000000","message":"could maybe use another pass through rebase, the example configs need to change and the commit message needs an UpgradeImpact.","commit_id":"75fd0ddd1798398d20ee37b6e9c80596b1192e76"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"6b554579556cd5f491058e5f3e052ad491ccae3a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":29,"id":"b320eac6_06cf7f34","updated":"2024-11-05 00:25:56.000000000","message":"I fixed the example configs, and added an UpgradeImpact - might need some feedback on the best way to land this one.\n\nThe idea is that no matter if you set randomized_task_container_iteration or task_container_iteration_strategy or whatever stupid option names we came up with while working on this new better way to list containers - unless you set \"legacy_multi_process_task_container_iteration \u003d true\" you\u0027re just going to get better expirers after upgrade.\n\nAnd most importantly for now - our SRE keeps getting parallel task iteration in prod (which is working great!)","commit_id":"b4a718c597e55236b0bd8ca961af75108cbf2515"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c5236251078fb63c632b59afde8e20e82fcdffa4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":36,"id":"bdec0b0b_a593a653","updated":"2025-01-16 12:15:04.000000000","message":"recheck","commit_id":"89f0e804c6c5785efec7d9940aa892528e058493"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a9edde28fd538eb1d7ba33942cacab4df6325c2b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":58,"id":"3008d94b_afea2f35","updated":"2026-03-30 09:41:31.000000000","message":"recheck","commit_id":"9e9081538e263171214c616ba2bdbf05acd1bbc2"}],"etc/object-expirer.conf-sample":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"84eb3687a8998770892eaea3a96dda97ddb8ab92","unresolved":true,"context_lines":[{"line_number":109,"context_line":"# will result in MORE randomness to the order in which expired objects are"},{"line_number":110,"context_line":"# reaped and may be useful to create less hot spots listing task_containers"},{"line_number":111,"context_line":"# from many processes."},{"line_number":112,"context_line":"# task_container_iteration_strategy \u003d legacy"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"# Number of tasks objects to cache before processing.  With many nodes it may"},{"line_number":115,"context_line":"# take some time to fill a larger cache_size but may also have a better chance"}],"source_content_type":"application/octet-stream","patch_set":28,"id":"4f916ed8_829201d9","line":112,"updated":"2024-11-01 21:06:35.000000000","message":"acpacphcapc, well this is wrong!","commit_id":"75fd0ddd1798398d20ee37b6e9c80596b1192e76"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"6b554579556cd5f491058e5f3e052ad491ccae3a","unresolved":false,"context_lines":[{"line_number":109,"context_line":"# will result in MORE randomness to the order in which expired objects are"},{"line_number":110,"context_line":"# reaped and may be useful to create less hot spots listing task_containers"},{"line_number":111,"context_line":"# from many processes."},{"line_number":112,"context_line":"# task_container_iteration_strategy \u003d legacy"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"# Number of tasks objects to cache before processing.  With many nodes it may"},{"line_number":115,"context_line":"# take some time to fill a larger cache_size but may also have a better chance"}],"source_content_type":"application/octet-stream","patch_set":28,"id":"05de5742_983bf2b0","line":112,"in_reply_to":"4f916ed8_829201d9","updated":"2024-11-05 00:25:56.000000000","message":"Done","commit_id":"75fd0ddd1798398d20ee37b6e9c80596b1192e76"}],"swift/common/utils/__init__.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"31a6461677b008783d9da6200cfab6107ee06351","unresolved":true,"context_lines":[{"line_number":179,"context_line":""},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"# Most clusters use the default \"expiring_objects_container_divisor\" of 86400."},{"line_number":182,"context_line":"# The number of expirer task containers \"per day\" is not configurable."},{"line_number":183,"context_line":"EXPIRER_CONTAINER_PER_DIVISOR \u003d 100"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"857a4cd4_f3a7aec1","line":182,"range":{"start_line":182,"start_character":50,"end_line":182,"end_character":69},"updated":"2024-05-09 16:58:34.000000000","message":"Why not, though? I feel like maybe it should be. Pretty sure we\u0027ve seen users mark *tens* or even *hundreds* of millions of objects to all expire on the same second -- so no matter how small we configure our `expiring_objects_container_divisor`, they\u0027ll all end up in the same bucket. As a result we\u0027ve found ourselves needing to bump up our sharding threshold in an effort to avoid sharding expiry queues, rather than increasing the number of containers per bucket.\n\nEh, probably best as future work.","commit_id":"d7987e1dffe9d606a14a39afc4477563216c1291"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"056e25012cce479de37dad5b25f1c12afea5e4c5","unresolved":false,"context_lines":[{"line_number":179,"context_line":""},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"# Most clusters use the default \"expiring_objects_container_divisor\" of 86400."},{"line_number":182,"context_line":"# The number of expirer task containers \"per day\" is not configurable."},{"line_number":183,"context_line":"EXPIRER_CONTAINER_PER_DIVISOR \u003d 100"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"32dc9656_d1582df7","line":182,"range":{"start_line":182,"start_character":50,"end_line":182,"end_character":69},"in_reply_to":"054ad13c_44fdceb0","updated":"2024-07-09 23:58:55.000000000","message":"Acknowledged","commit_id":"d7987e1dffe9d606a14a39afc4477563216c1291"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"1d1308cf04a454c509115a60d67be827bf949d9b","unresolved":true,"context_lines":[{"line_number":179,"context_line":""},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"# Most clusters use the default \"expiring_objects_container_divisor\" of 86400."},{"line_number":182,"context_line":"# The number of expirer task containers \"per day\" is not configurable."},{"line_number":183,"context_line":"EXPIRER_CONTAINER_PER_DIVISOR \u003d 100"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"054ad13c_44fdceb0","line":182,"range":{"start_line":182,"start_character":50,"end_line":182,"end_character":69},"in_reply_to":"857a4cd4_f3a7aec1","updated":"2024-05-15 03:14:53.000000000","message":"i wasn\u0027t trying to make an opinion about this as much as call out the existance of the magic number.  I figure having it a top-level constant is one step closer to configurable - it\u0027s frustrated me before that this number isn\u0027t configurable while divisor is.\n\nAs, Al pointed out if we *really* want to move towards \"one bucket per worker per day\" in order to optimize the ratio of processing-rows/listing-rows - we should make this configurable and set it equal to \"processes\" (even if it only helps for new rows going forward)\n\nBut since this change would already improve the processing-rows/listing-rows ratio a couple of order of magnitude - I agree; better as future work.","commit_id":"d7987e1dffe9d606a14a39afc4477563216c1291"}],"swift/obj/expirer.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"5a034f2024afb5b5980d5e5f7b94e9279f666c69","unresolved":true,"context_lines":[{"line_number":338,"context_line":"            # least one container"},{"line_number":339,"context_line":"            my_containers \u003d utils.distribute_evenly("},{"line_number":340,"context_line":"                task_containers_to_share, self.processes)[self.process]"},{"line_number":341,"context_line":"            return 0, 1, my_containers"},{"line_number":342,"context_line":"        else:"},{"line_number":343,"context_line":"            # somewhat more tricky, each container will be processed by"},{"line_number":344,"context_line":"            # potentially multiple processes in parallel."}],"source_content_type":"text/x-python","patch_set":1,"id":"200adc26_19313668","line":341,"updated":"2024-05-09 11:09:29.000000000","message":"IIUC this is the preferred case i.e. no work duplicated by multiple processes.\n\nSeems like it would be desirable to scale EXPIRER_CONTAINER_PER_DIVISOR according to number of processes e.g. if EXPIRER_CONTAINER_PER_DIVISOR was equal to the number of processes, then on average each process would pick up one container\u0027s worth of work per-day-in-the-queue.\n\nIn fact, it might be desirable for the task distribution to deliberately aim for each process to get some work from each day. (Maybe it already does, my head is full of cold so I may not be reading the code right). That way, if there are \"busy\" days (lots of objects expired at same time) the work is spread across all processes.\n\nIn general, having processes select whole containers worth of tasks, rather than selecting a subset of objects from within every container, seems like a fabulous idea, so long as there are sufficient containers w.r.t. processes.","commit_id":"d7987e1dffe9d606a14a39afc4477563216c1291"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"1d1308cf04a454c509115a60d67be827bf949d9b","unresolved":true,"context_lines":[{"line_number":338,"context_line":"            # least one container"},{"line_number":339,"context_line":"            my_containers \u003d utils.distribute_evenly("},{"line_number":340,"context_line":"                task_containers_to_share, self.processes)[self.process]"},{"line_number":341,"context_line":"            return 0, 1, my_containers"},{"line_number":342,"context_line":"        else:"},{"line_number":343,"context_line":"            # somewhat more tricky, each container will be processed by"},{"line_number":344,"context_line":"            # potentially multiple processes in parallel."}],"source_content_type":"text/x-python","patch_set":1,"id":"9546c0cc_a43daa52","line":341,"in_reply_to":"200adc26_19313668","updated":"2024-05-15 03:14:53.000000000","message":"note in the \"somewhat more tricky\" case we still sub-divide the rows w/i in shared container sort of like master does today - but instead of each process skipping 1699/1700 rows it\u0027ll be shared by two maybe three proceses at most.\n\nwe can\u0027t dynamically select EXPIRER_CONTAINER_PER_DIVISOR because it\u0027s used by the proxy to select the task_container when the tasks are created by the client request.  But if it was configurable (in a future patch) we could try scaling it up towards number of processes for new tasks created going forward if necessary.","commit_id":"d7987e1dffe9d606a14a39afc4477563216c1291"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"056e25012cce479de37dad5b25f1c12afea5e4c5","unresolved":false,"context_lines":[{"line_number":338,"context_line":"            # least one container"},{"line_number":339,"context_line":"            my_containers \u003d utils.distribute_evenly("},{"line_number":340,"context_line":"                task_containers_to_share, self.processes)[self.process]"},{"line_number":341,"context_line":"            return 0, 1, my_containers"},{"line_number":342,"context_line":"        else:"},{"line_number":343,"context_line":"            # somewhat more tricky, each container will be processed by"},{"line_number":344,"context_line":"            # potentially multiple processes in parallel."}],"source_content_type":"text/x-python","patch_set":1,"id":"e6245b64_f6736d67","line":341,"in_reply_to":"3e750f26_ba4647c3","updated":"2024-07-09 23:58:55.000000000","message":"Acknowledged","commit_id":"d7987e1dffe9d606a14a39afc4477563216c1291"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"59940bed58dfa3f33cd38a4e832b1bb683fb92f4","unresolved":true,"context_lines":[{"line_number":338,"context_line":"            # least one container"},{"line_number":339,"context_line":"            my_containers \u003d utils.distribute_evenly("},{"line_number":340,"context_line":"                task_containers_to_share, self.processes)[self.process]"},{"line_number":341,"context_line":"            return 0, 1, my_containers"},{"line_number":342,"context_line":"        else:"},{"line_number":343,"context_line":"            # somewhat more tricky, each container will be processed by"},{"line_number":344,"context_line":"            # potentially multiple processes in parallel."}],"source_content_type":"text/x-python","patch_set":1,"id":"3e750f26_ba4647c3","line":341,"in_reply_to":"9546c0cc_a43daa52","updated":"2024-05-17 15:58:47.000000000","message":"Ack. I had missed the fact that each process still selects tasks from within the containers that it has selected, so no duplicate tasks.","commit_id":"d7987e1dffe9d606a14a39afc4477563216c1291"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"1d1308cf04a454c509115a60d67be827bf949d9b","unresolved":true,"context_lines":[{"line_number":371,"context_line":""},{"line_number":372,"context_line":"        # parallel strategy divides listing task containers up by process"},{"line_number":373,"context_line":"        task_containers_to_share \u003d self._get_all_expected_task_containers("},{"line_number":374,"context_line":"            container_list)"},{"line_number":375,"context_line":""},{"line_number":376,"context_line":"        if len(task_containers_to_share) \u003e\u003d self.processes:"},{"line_number":377,"context_line":"            # pigeon hole says each process will handle all of the tasks in at"}],"source_content_type":"text/x-python","patch_set":2,"id":"456ea7af_df97aa59","line":374,"updated":"2024-05-15 03:14:53.000000000","message":"this is the main idea I\u0027d like some feedback on.  It seemed like \"distribute_evenly\" could have a cascade effect that leads to more overlaps at the edge where old containers are being cleaned up...\n\nmy strategy to stabalize the task_container assignment accross all processes was to artificially hydrate the list with expected container names - I think this creates a kind of \"epoch\" where some proceses might be on either side; but they tend to move more wholesale.\n\nAm I explaining that well?  Is that a reasonable problem to concern ourselves with?  Is this a reasonable strategy?  How should I demonstrate the desirable behavior we want in tests?","commit_id":"cf34af352cb9db637fbcfca06272279d0406c639"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f6890c38f8467f4d1a59e8558e7cc35df47ef76e","unresolved":false,"context_lines":[{"line_number":371,"context_line":""},{"line_number":372,"context_line":"        # parallel strategy divides listing task containers up by process"},{"line_number":373,"context_line":"        task_containers_to_share \u003d self._get_all_expected_task_containers("},{"line_number":374,"context_line":"            container_list)"},{"line_number":375,"context_line":""},{"line_number":376,"context_line":"        if len(task_containers_to_share) \u003e\u003d self.processes:"},{"line_number":377,"context_line":"            # pigeon hole says each process will handle all of the tasks in at"}],"source_content_type":"text/x-python","patch_set":2,"id":"fd323f24_33099d7d","line":374,"in_reply_to":"057bf5df_b2c71c56","updated":"2024-07-24 15:21:36.000000000","message":"Acknowledged","commit_id":"cf34af352cb9db637fbcfca06272279d0406c639"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"59940bed58dfa3f33cd38a4e832b1bb683fb92f4","unresolved":true,"context_lines":[{"line_number":371,"context_line":""},{"line_number":372,"context_line":"        # parallel strategy divides listing task containers up by process"},{"line_number":373,"context_line":"        task_containers_to_share \u003d self._get_all_expected_task_containers("},{"line_number":374,"context_line":"            container_list)"},{"line_number":375,"context_line":""},{"line_number":376,"context_line":"        if len(task_containers_to_share) \u003e\u003d self.processes:"},{"line_number":377,"context_line":"            # pigeon hole says each process will handle all of the tasks in at"}],"source_content_type":"text/x-python","patch_set":2,"id":"46398d70_6b029e51","line":374,"in_reply_to":"456ea7af_df97aa59","updated":"2024-05-17 15:58:47.000000000","message":"let me see if I have understood:\n\n* If we have at least one container on each of 10 days, then we\u0027ll synthesize 100 containers names per day -\u003e 1000.\n* Some of those containers won\u0027t actually exist if their tasks have been processed. (But if more than 100 objects expired on that data then it\u0027s likely that they *did* exist because of tasks being hashed across 100 containers).\n* It\u0027s more likely that a missing container has been processed and cleaned up by another process that it just never having existed.\n* So we account for work that was likely to have been recently done when divvying up the containers, by putting the missing ones into the set anyway.\n\n* If we have 500 processes, they\u0027ll get 2 container *names* each, but some containers don\u0027t exist.\n* If we hadn\u0027t synthesized the \u0027missing\u0027 containers, and the set size was \u003c 500, then some processes might have got the same containers and shared the work within that container.\n\nOK, so I think I understand what the overlap-at-edge case is and how this stabilizes the distribution/keeps it more uniform across time.\n\nIt seems like a reasonable strategy.\n\nI might need some more help understanding the consequences of *not* doing this... In the extreme, we might have 500 processes all piling on to the final container, which is listing overhead (back to the legacy strategy, but only temporarily).","commit_id":"cf34af352cb9db637fbcfca06272279d0406c639"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"302e6433f7c678c03789e611715cfc6630c3f350","unresolved":true,"context_lines":[{"line_number":371,"context_line":""},{"line_number":372,"context_line":"        # parallel strategy divides listing task containers up by process"},{"line_number":373,"context_line":"        task_containers_to_share \u003d self._get_all_expected_task_containers("},{"line_number":374,"context_line":"            container_list)"},{"line_number":375,"context_line":""},{"line_number":376,"context_line":"        if len(task_containers_to_share) \u003e\u003d self.processes:"},{"line_number":377,"context_line":"            # pigeon hole says each process will handle all of the tasks in at"}],"source_content_type":"text/x-python","patch_set":2,"id":"057bf5df_b2c71c56","line":374,"in_reply_to":"46398d70_6b029e51","updated":"2024-05-17 17:26:14.000000000","message":"notes from a verbal discussion with Clay:\n\nthe concern is that if different processes see different list then they might make different decisions about who does what work, including nobody deciding to process a container :(","commit_id":"cf34af352cb9db637fbcfca06272279d0406c639"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"d64c7eea64048207adc961f267fe4c07f357f77c","unresolved":true,"context_lines":[{"line_number":173,"context_line":"            all_expected_containers.update("},{"line_number":174,"context_line":"                (num_days * expirer_divisor) - i"},{"line_number":175,"context_line":"                for i in range(task_container_per_day))"},{"line_number":176,"context_line":"        if task_container_time not in all_expected_containers:"},{"line_number":177,"context_line":"            unexpected_containers.add(task_container_time)"},{"line_number":178,"context_line":"    return sorted(all_expected_containers), unexpected_containers"},{"line_number":179,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"1659adbb_f42485dd","line":176,"updated":"2024-06-04 19:24:37.000000000","message":"if ``task_container_per_day`` is adjusted from 100 to 50 on a cluster, then the existing task containers within a day (50~100) will be in ``unexpected_containers``.","commit_id":"4482a4b60b1935d5f0e950d6b4d136465b2962aa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"056e25012cce479de37dad5b25f1c12afea5e4c5","unresolved":false,"context_lines":[{"line_number":173,"context_line":"            all_expected_containers.update("},{"line_number":174,"context_line":"                (num_days * expirer_divisor) - i"},{"line_number":175,"context_line":"                for i in range(task_container_per_day))"},{"line_number":176,"context_line":"        if task_container_time not in all_expected_containers:"},{"line_number":177,"context_line":"            unexpected_containers.add(task_container_time)"},{"line_number":178,"context_line":"    return sorted(all_expected_containers), unexpected_containers"},{"line_number":179,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ffba5710_9664ab4c","line":176,"in_reply_to":"1659adbb_f42485dd","updated":"2024-07-09 23:58:55.000000000","message":"correct!  increasing task_container_per_day never leads to unexpected containers; but decreasing it always does.","commit_id":"4482a4b60b1935d5f0e950d6b4d136465b2962aa"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"d64c7eea64048207adc961f267fe4c07f357f77c","unresolved":true,"context_lines":[{"line_number":473,"context_line":"        return int(md5("},{"line_number":474,"context_line":"            name, usedforsecurity\u003dFalse).hexdigest(), 16) % divisor"},{"line_number":475,"context_line":""},{"line_number":476,"context_line":"    def _get_all_expected_task_containers(self, task_container_ints):"},{"line_number":477,"context_line":"        # the current strategy for unexpected containers is just to tack them"},{"line_number":478,"context_line":"        # on to the end of the distribution"},{"line_number":479,"context_line":"        return list(itertools.chain(*get_all_expected_task_containers("}],"source_content_type":"text/x-python","patch_set":4,"id":"a9fc0d5f_363cd415","line":476,"updated":"2024-06-04 19:24:37.000000000","message":"``_get_all_expected_task_containers`` calls ``get_all_expected_task_containers``, from the naming point of view, it should be the other way around?","commit_id":"4482a4b60b1935d5f0e950d6b4d136465b2962aa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"056e25012cce479de37dad5b25f1c12afea5e4c5","unresolved":false,"context_lines":[{"line_number":473,"context_line":"        return int(md5("},{"line_number":474,"context_line":"            name, usedforsecurity\u003dFalse).hexdigest(), 16) % divisor"},{"line_number":475,"context_line":""},{"line_number":476,"context_line":"    def _get_all_expected_task_containers(self, task_container_ints):"},{"line_number":477,"context_line":"        # the current strategy for unexpected containers is just to tack them"},{"line_number":478,"context_line":"        # on to the end of the distribution"},{"line_number":479,"context_line":"        return list(itertools.chain(*get_all_expected_task_containers("}],"source_content_type":"text/x-python","patch_set":4,"id":"59f58616_4c31b545","line":476,"in_reply_to":"a9fc0d5f_363cd415","updated":"2024-07-09 23:58:55.000000000","message":"the private `_get_all_expected_containers` method on the ObjectExpier ended up not being needed in favor of using the public `get_all_expected_task_containers_per_day` on the new ExpirerConfig object directly in `_parallel_strategy`.","commit_id":"4482a4b60b1935d5f0e950d6b4d136465b2962aa"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"d64c7eea64048207adc961f267fe4c07f357f77c","unresolved":true,"context_lines":[{"line_number":528,"context_line":"                \u0027this message should go away in a few days.\u0027,"},{"line_number":529,"context_line":"                unexpected_task_containers[\u0027count\u0027],"},{"line_number":530,"context_line":"                \u0027 \u0027.join(unexpected_task_containers[\u0027examples\u0027]))"},{"line_number":531,"context_line":"        if self.task_container_iteration_strategy \u003d\u003d \u0027randomized\u0027:"},{"line_number":532,"context_line":"            shuffle(container_list)"},{"line_number":533,"context_line":""},{"line_number":534,"context_line":"        if self.task_container_iteration_strategy !\u003d \"parallel\":"}],"source_content_type":"text/x-python","patch_set":4,"id":"e36bd71f_951048a3","line":531,"updated":"2024-06-04 19:24:37.000000000","message":"how about breaking Line 531-570 into two functions like this:\n```\n   if self.task_container_iteration_strategy \u003d\u003d \"parallel\":\n       handle_parallel_selection(...)\n   else:\n       handle_other_selections(...)\n```","commit_id":"4482a4b60b1935d5f0e950d6b4d136465b2962aa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"056e25012cce479de37dad5b25f1c12afea5e4c5","unresolved":false,"context_lines":[{"line_number":528,"context_line":"                \u0027this message should go away in a few days.\u0027,"},{"line_number":529,"context_line":"                unexpected_task_containers[\u0027count\u0027],"},{"line_number":530,"context_line":"                \u0027 \u0027.join(unexpected_task_containers[\u0027examples\u0027]))"},{"line_number":531,"context_line":"        if self.task_container_iteration_strategy \u003d\u003d \u0027randomized\u0027:"},{"line_number":532,"context_line":"            shuffle(container_list)"},{"line_number":533,"context_line":""},{"line_number":534,"context_line":"        if self.task_container_iteration_strategy !\u003d \"parallel\":"}],"source_content_type":"text/x-python","patch_set":4,"id":"6b549724_9414e88a","line":531,"in_reply_to":"e36bd71f_951048a3","updated":"2024-07-09 23:58:55.000000000","message":"good suggestion.","commit_id":"4482a4b60b1935d5f0e950d6b4d136465b2962aa"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"d64c7eea64048207adc961f267fe4c07f357f77c","unresolved":true,"context_lines":[{"line_number":551,"context_line":"            # at least one container"},{"line_number":552,"context_line":"            my_containers \u003d utils.distribute_evenly("},{"line_number":553,"context_line":"                task_containers_to_share, self.processes)[self.process]"},{"line_number":554,"context_line":"            return 0, 1, sorted(my_containers)"},{"line_number":555,"context_line":"        else:"},{"line_number":556,"context_line":"            # somewhat more tricky, each container will be processed by"},{"line_number":557,"context_line":"            # potentially multiple processes in parallel."}],"source_content_type":"text/x-python","patch_set":4,"id":"ed646d67_63d49257","line":554,"updated":"2024-06-04 19:24:37.000000000","message":"I see, for ``task_containers_to_share) \u003e\u003d self.processes``, so each process will handle all tasks for ``my_containers`` exclusively. I think it\u0027s a good approach to spread the load evenly if days are not skewed too much: for all task containers within a day, since all tasks will be hashed into those containers, so task containers within a day should have tasks evenly; even though different days probably contain different number of delete tasks, but ``utils.distribute_evenly`` will help to spread those containers within one busy day into different processes.\n\nthe worst case probably will be this kind of skewed workload: let\u0027s say there are 10K task containers, 1000 processes, and ``task_containers_per_day`` is 100. And one super busy day (100 task containers) have 99% of all delete tasks. so there will be only 100 processes to handle those 100 super busy task containers, and rest of 900 processes probably will finish very soon and just idle. And skewed containers will affect the other case (``task_containers_to_share) \u003c self.processes``) as well. let\u0027s say we have 500 task containers (total 5 days), and 1000 processes, so each container will be assigned to two processes, but since only one day (100 containers) has 99% of all delete tasks, so only 200 processes will be very busy and rest of 800 processes will be just idle.\n\nif we will see this kind of cases in production and we need to solve them, here is one idea, and it also has the ability to unify the two branches of ``len(task_containers_to_share) \u003e\u003d self.processes``: how about we map one group of indexed processes to one group of indexed containers for any given day? ``task_containers_per_day`` is 100, if we have 50 processes, one process will handle two fixed indexed number task containers for any day; if we have 1000 processes, 10 processes (e.g. index 1 to 10) will handle one fixed indexed task container for any day. So for each day, we divide up the 100 task containers and assigned them to a group of processes, since all 100 task containers are evenly loaded within a day, this will guarantee each group of processes will receive the same amount of delete tasks. And we won\u0027t need to worry the case of node down, for the typical setup of 1000 processes and 100 containers per day, each process group will have 10 processes, that\u0027s a lot of redundancy.","commit_id":"4482a4b60b1935d5f0e950d6b4d136465b2962aa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"056e25012cce479de37dad5b25f1c12afea5e4c5","unresolved":false,"context_lines":[{"line_number":551,"context_line":"            # at least one container"},{"line_number":552,"context_line":"            my_containers \u003d utils.distribute_evenly("},{"line_number":553,"context_line":"                task_containers_to_share, self.processes)[self.process]"},{"line_number":554,"context_line":"            return 0, 1, sorted(my_containers)"},{"line_number":555,"context_line":"        else:"},{"line_number":556,"context_line":"            # somewhat more tricky, each container will be processed by"},{"line_number":557,"context_line":"            # potentially multiple processes in parallel."}],"source_content_type":"text/x-python","patch_set":4,"id":"8280a858_50f59478","line":554,"in_reply_to":"ed646d67_63d49257","updated":"2024-07-09 23:58:55.000000000","message":"i\u0027ve attempted to address this by distributing each days worth of containers to all processes.","commit_id":"4482a4b60b1935d5f0e950d6b4d136465b2962aa"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"68f2e6b601cc9d499d020066d94fdc647d58cf32","unresolved":false,"context_lines":[{"line_number":199,"context_line":"            all_expected_containers_by_day[k]"},{"line_number":200,"context_line":"            for k in sorted(all_expected_containers_by_day)"},{"line_number":201,"context_line":"        ] + ["},{"line_number":202,"context_line":"            unexpected_containers[i:i + self.task_container_per_day] for i in"},{"line_number":203,"context_line":"            range(0, len(unexpected_containers), self.task_container_per_day)"},{"line_number":204,"context_line":"        ]"},{"line_number":205,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"3b5533d3_133ae70f","line":202,"updated":"2024-07-25 06:07:26.000000000","message":"great, so unexpected_containers are processed in a similar way. the last group of unexpected_containers may be less than self.task_container_per_day and won\u0027t be evenly distributed, but we don\u0027t expect to see unexpected_containers often in the production.","commit_id":"f5a63a1b4f12987d22cbbf613cd3d7673ae2eba8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f950c1e90422b1d2679334da66c43dd6d865e46e","unresolved":true,"context_lines":[{"line_number":539,"context_line":"            i, d, selected \u003d self._other_strategy(container_list)"},{"line_number":540,"context_line":"        return i, d, [str(c) for c in selected]"},{"line_number":541,"context_line":""},{"line_number":542,"context_line":"    def _other_strategy(self, container_list):"},{"line_number":543,"context_line":"        if self.task_container_iteration_strategy \u003d\u003d \u0027randomized\u0027:"},{"line_number":544,"context_line":"            shuffle(container_list)"},{"line_number":545,"context_line":"        if self.processes \u003c\u003d 0:"}],"source_content_type":"text/x-python","patch_set":9,"id":"44951c04_d0bdadcb","line":542,"updated":"2024-07-22 17:32:28.000000000","message":"nit: could we call this \u0027legacy_strategy\u0027 ? or \u0027serial_strategy\u0027","commit_id":"f5a63a1b4f12987d22cbbf613cd3d7673ae2eba8"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"68f2e6b601cc9d499d020066d94fdc647d58cf32","unresolved":false,"context_lines":[{"line_number":539,"context_line":"            i, d, selected \u003d self._other_strategy(container_list)"},{"line_number":540,"context_line":"        return i, d, [str(c) for c in selected]"},{"line_number":541,"context_line":""},{"line_number":542,"context_line":"    def _other_strategy(self, container_list):"},{"line_number":543,"context_line":"        if self.task_container_iteration_strategy \u003d\u003d \u0027randomized\u0027:"},{"line_number":544,"context_line":"            shuffle(container_list)"},{"line_number":545,"context_line":"        if self.processes \u003c\u003d 0:"}],"source_content_type":"text/x-python","patch_set":9,"id":"57793567_b294974d","line":542,"in_reply_to":"44951c04_d0bdadcb","updated":"2024-07-25 06:07:26.000000000","message":"Acknowledged","commit_id":"f5a63a1b4f12987d22cbbf613cd3d7673ae2eba8"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"68f2e6b601cc9d499d020066d94fdc647d58cf32","unresolved":false,"context_lines":[{"line_number":551,"context_line":"            # (and \"handle\" only a subset)"},{"line_number":552,"context_line":"            return self.process, self.processes, container_list"},{"line_number":553,"context_line":""},{"line_number":554,"context_line":"    def _parallel_strategy(self, container_list):"},{"line_number":555,"context_line":"        if self.expirer_config.task_container_per_day \u003e\u003d self.processes:"},{"line_number":556,"context_line":"            # pigeon hole says each process will handle all of the tasks in"},{"line_number":557,"context_line":"            # at least one container"}],"source_content_type":"text/x-python","patch_set":9,"id":"8170e43f_fdd8ed80","line":554,"updated":"2024-07-25 06:07:26.000000000","message":"this implementation from Clay is based on branching of \"self.expirer_config.task_container_per_day \u003e\u003d self.processes\", which is more straightforward to follow through and understand.","commit_id":"f5a63a1b4f12987d22cbbf613cd3d7673ae2eba8"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"68f2e6b601cc9d499d020066d94fdc647d58cf32","unresolved":false,"context_lines":[{"line_number":564,"context_line":"            assigned_procs_per_container \u003d utils.distribute_evenly("},{"line_number":565,"context_line":"                range(self.processes),"},{"line_number":566,"context_line":"                self.expirer_config.task_container_per_day)"},{"line_number":567,"context_line":"            # This is the best way I\u0027ve found (so far!) to determine which"},{"line_number":568,"context_line":"            # expirer process is assigned to which task_containers and how"},{"line_number":569,"context_line":"            # many other assigned procs this expirer is sharing with."},{"line_number":570,"context_line":"            for assigned_group, assigned_procs in enumerate("}],"source_content_type":"text/x-python","patch_set":9,"id":"1ea8f8bb_31cac1d8","line":567,"updated":"2024-07-25 06:07:26.000000000","message":"this search method is logically correct, but can be simplified by math indeed, since the implementation of utils.distribute_evenly is deterministic.","commit_id":"f5a63a1b4f12987d22cbbf613cd3d7673ae2eba8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f950c1e90422b1d2679334da66c43dd6d865e46e","unresolved":true,"context_lines":[{"line_number":583,"context_line":"        # parallel strategy divides listing task containers up by process"},{"line_number":584,"context_line":"        my_containers \u003d []"},{"line_number":585,"context_line":"        first_len \u003d None"},{"line_number":586,"context_line":"        for task_containers_to_share in \\"},{"line_number":587,"context_line":"                self.expirer_config.get_all_expected_task_containers_per_day("},{"line_number":588,"context_line":"                    container_list):"},{"line_number":589,"context_line":"            if first_len is None:"},{"line_number":590,"context_line":"                first_len \u003d len(task_containers_to_share)"},{"line_number":591,"context_line":"            elif first_len !\u003d len(task_containers_to_share):"},{"line_number":592,"context_line":"                assert not any("}],"source_content_type":"text/x-python","patch_set":9,"id":"98115a12_5e1fef06","line":589,"range":{"start_line":586,"start_character":1,"end_line":589,"end_character":1},"updated":"2024-07-22 17:32:28.000000000","message":"note to self: ok, I got to wondering why the for loop, i.e. why we couldn\u0027t just distribute the entire set of containers in one go, but there\u0027s something about the case where number of containers *per day* is a non-integer multiple of  number of processes that prevents that simplification: Each *day* there\u0027s some procs that get 1 more containers than others (there\u0027s a test case that illustrates this)","commit_id":"f5a63a1b4f12987d22cbbf613cd3d7673ae2eba8"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"68f2e6b601cc9d499d020066d94fdc647d58cf32","unresolved":false,"context_lines":[{"line_number":583,"context_line":"        # parallel strategy divides listing task containers up by process"},{"line_number":584,"context_line":"        my_containers \u003d []"},{"line_number":585,"context_line":"        first_len \u003d None"},{"line_number":586,"context_line":"        for task_containers_to_share in \\"},{"line_number":587,"context_line":"                self.expirer_config.get_all_expected_task_containers_per_day("},{"line_number":588,"context_line":"                    container_list):"},{"line_number":589,"context_line":"            if first_len is None:"},{"line_number":590,"context_line":"                first_len \u003d len(task_containers_to_share)"},{"line_number":591,"context_line":"            elif first_len !\u003d len(task_containers_to_share):"},{"line_number":592,"context_line":"                assert not any("}],"source_content_type":"text/x-python","patch_set":9,"id":"c3a9c718_2ac4a300","line":589,"range":{"start_line":586,"start_character":1,"end_line":589,"end_character":1},"in_reply_to":"98115a12_5e1fef06","updated":"2024-07-25 06:07:26.000000000","message":"Acknowledged","commit_id":"f5a63a1b4f12987d22cbbf613cd3d7673ae2eba8"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"68f2e6b601cc9d499d020066d94fdc647d58cf32","unresolved":true,"context_lines":[{"line_number":599,"context_line":"          process 3 -\u003e index\u003d1, divisor\u003d2, containers[t-0]"},{"line_number":600,"context_line":"        \"\"\""},{"line_number":601,"context_line":"        task_container_per_day \u003d self.expirer_config.task_container_per_day"},{"line_number":602,"context_line":"        divisor \u003d self.processes // task_container_per_day"},{"line_number":603,"context_line":"        if self.process % task_container_per_day \u003c \\"},{"line_number":604,"context_line":"                self.processes % task_container_per_day:"},{"line_number":605,"context_line":"            divisor +\u003d 1"}],"source_content_type":"text/x-python","patch_set":12,"id":"e6b83539_171e8e2e","line":602,"updated":"2024-07-25 06:07:26.000000000","message":"a concise refactoring of the same approach from Alistair, thank you too for supporting this idea from me: https://review.opendev.org/c/openstack/swift/+/918366/comment/ed646d67_63d49257/\n\neven though the calculation of divisor and my_index works for both cases(``self.processes \u003e\u003d task_container_per_day`` and ``self.processes \u003c task_container_per_day``), but this makes the code harder to understand, and the case of ``self.processes \u003c task_container_per_day`` actually don\u0027t need calculation at all. I prefer to add branches as below, to make it easier to understand and maintain.\n\n```\n        if self.processes \u003e\u003d task_container_per_day:\n            divisor \u003d self.processes // task_container_per_day\n            if self.process % task_container_per_day \u003c \\\n                    self.processes % task_container_per_day:\n                divisor +\u003d 1\n            my_index \u003d self.process // task_container_per_day\n        else:\n            my_index \u003d 0\n            divisor \u003d 1\n```","commit_id":"1d6cc6936c13e3deea674d8e93caa32599097d15"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"17483026f37f04a4e6453f3263b36608c87a08d0","unresolved":false,"context_lines":[{"line_number":599,"context_line":"          process 3 -\u003e index\u003d1, divisor\u003d2, containers[t-0]"},{"line_number":600,"context_line":"        \"\"\""},{"line_number":601,"context_line":"        task_container_per_day \u003d self.expirer_config.task_container_per_day"},{"line_number":602,"context_line":"        divisor \u003d self.processes // task_container_per_day"},{"line_number":603,"context_line":"        if self.process % task_container_per_day \u003c \\"},{"line_number":604,"context_line":"                self.processes % task_container_per_day:"},{"line_number":605,"context_line":"            divisor +\u003d 1"}],"source_content_type":"text/x-python","patch_set":12,"id":"8b09ed1d_080b0190","line":602,"in_reply_to":"4d647df2_34e8fbc3","updated":"2024-07-30 18:07:38.000000000","message":"Done","commit_id":"1d6cc6936c13e3deea674d8e93caa32599097d15"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"165ac405346d68b7bc5ad898222370395988a2cb","unresolved":true,"context_lines":[{"line_number":599,"context_line":"          process 3 -\u003e index\u003d1, divisor\u003d2, containers[t-0]"},{"line_number":600,"context_line":"        \"\"\""},{"line_number":601,"context_line":"        task_container_per_day \u003d self.expirer_config.task_container_per_day"},{"line_number":602,"context_line":"        divisor \u003d self.processes // task_container_per_day"},{"line_number":603,"context_line":"        if self.process % task_container_per_day \u003c \\"},{"line_number":604,"context_line":"                self.processes % task_container_per_day:"},{"line_number":605,"context_line":"            divisor +\u003d 1"}],"source_content_type":"text/x-python","patch_set":12,"id":"4d647df2_34e8fbc3","line":602,"in_reply_to":"e6b83539_171e8e2e","updated":"2024-07-25 10:22:03.000000000","message":"I agree, let\u0027s make simple the ``my_index\u003d0`` case explicit.\n\nHowever, I would like to avoid repeating the same condition within the method ``if self.processes \u003e\u003d task_container_per_day:``. I think that\u0027s still achievable - see next patchet.","commit_id":"1d6cc6936c13e3deea674d8e93caa32599097d15"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"0b787186bab5e92707bb133859b93d40d9042326","unresolved":true,"context_lines":[{"line_number":166,"context_line":"            account_name, task_container)"},{"line_number":167,"context_line":"        return part, nodes, task_container"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    def get_all_expected_task_containers_per_day(self, task_container_ints):"},{"line_number":170,"context_line":"        \"\"\""},{"line_number":171,"context_line":"        :params task_container_ints: a list of ints, all task_containers taken"},{"line_number":172,"context_line":"                                     from listing of the expiring objects"}],"source_content_type":"text/x-python","patch_set":13,"id":"60e88355_d9350adc","line":169,"updated":"2024-07-26 05:13:11.000000000","message":"this function needs unit tests, added at here: https://review.opendev.org/c/openstack/swift/+/924903","commit_id":"c62d8ca9b49d166acbd7d4e6df68ed8cb60fe9b4"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"17483026f37f04a4e6453f3263b36608c87a08d0","unresolved":false,"context_lines":[{"line_number":166,"context_line":"            account_name, task_container)"},{"line_number":167,"context_line":"        return part, nodes, task_container"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    def get_all_expected_task_containers_per_day(self, task_container_ints):"},{"line_number":170,"context_line":"        \"\"\""},{"line_number":171,"context_line":"        :params task_container_ints: a list of ints, all task_containers taken"},{"line_number":172,"context_line":"                                     from listing of the expiring objects"}],"source_content_type":"text/x-python","patch_set":13,"id":"ee3f8d45_35c85633","line":169,"in_reply_to":"60e88355_d9350adc","updated":"2024-07-30 18:07:38.000000000","message":"squashed.","commit_id":"c62d8ca9b49d166acbd7d4e6df68ed8cb60fe9b4"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0b54ee70601ae6c9626a4d5157ba7ced7bc4fa48","unresolved":true,"context_lines":[{"line_number":147,"context_line":"        # calculate seconds offset into previous divisor window"},{"line_number":148,"context_line":"        r \u003d (task_container_int - 1) % self.expirer_divisor"},{"line_number":149,"context_line":"        # seconds offset should be no more than task_container_per_day i.e."},{"line_number":150,"context_line":"        # given % 86400, 86359 is ok (because 42 is less than 100), but 49768"},{"line_number":151,"context_line":"        # would be unexpected"},{"line_number":152,"context_line":"        return self.expirer_divisor - r \u003c\u003d self.task_container_per_day"},{"line_number":153,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"591c9dd5_c4c6a44d","line":150,"range":{"start_line":150,"start_character":46,"end_line":150,"end_character":48},"updated":"2024-07-31 14:11:01.000000000","message":"41 vs 42 depends on whether you interpret the comment referring to `r` or to `task_container_per_day`","commit_id":"4bb01dd96a5da61b753baae380d1ab614f7734d8"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"582a2b495a3bbda33b64121ad015f3d8ec06c773","unresolved":false,"context_lines":[{"line_number":147,"context_line":"        # calculate seconds offset into previous divisor window"},{"line_number":148,"context_line":"        r \u003d (task_container_int - 1) % self.expirer_divisor"},{"line_number":149,"context_line":"        # seconds offset should be no more than task_container_per_day i.e."},{"line_number":150,"context_line":"        # given % 86400, 86359 is ok (because 42 is less than 100), but 49768"},{"line_number":151,"context_line":"        # would be unexpected"},{"line_number":152,"context_line":"        return self.expirer_divisor - r \u003c\u003d self.task_container_per_day"},{"line_number":153,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"7d787916_b626efbd","line":150,"range":{"start_line":150,"start_character":46,"end_line":150,"end_character":48},"in_reply_to":"591c9dd5_c4c6a44d","updated":"2024-07-31 16:55:03.000000000","message":"Acknowledged","commit_id":"4bb01dd96a5da61b753baae380d1ab614f7734d8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0b54ee70601ae6c9626a4d5157ba7ced7bc4fa48","unresolved":true,"context_lines":[{"line_number":185,"context_line":"            if not self.is_expected_task_container(task_container_time):"},{"line_number":186,"context_line":"                unexpected_containers.append(task_container_time)"},{"line_number":187,"context_line":"                continue"},{"line_number":188,"context_line":"            # Calculate the number of days since epoch for task container time."},{"line_number":189,"context_line":"            num_days, _ \u003d divmod("},{"line_number":190,"context_line":"                task_container_time + self.task_container_per_day,"},{"line_number":191,"context_line":"                self.expirer_divisor)"}],"source_content_type":"text/x-python","patch_set":14,"id":"65bb93ca_61321287","line":188,"updated":"2024-07-31 14:11:01.000000000","message":"I think it was helpful to have a comment that \u0027days\u0027 is just the default","commit_id":"4bb01dd96a5da61b753baae380d1ab614f7734d8"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"582a2b495a3bbda33b64121ad015f3d8ec06c773","unresolved":false,"context_lines":[{"line_number":185,"context_line":"            if not self.is_expected_task_container(task_container_time):"},{"line_number":186,"context_line":"                unexpected_containers.append(task_container_time)"},{"line_number":187,"context_line":"                continue"},{"line_number":188,"context_line":"            # Calculate the number of days since epoch for task container time."},{"line_number":189,"context_line":"            num_days, _ \u003d divmod("},{"line_number":190,"context_line":"                task_container_time + self.task_container_per_day,"},{"line_number":191,"context_line":"                self.expirer_divisor)"}],"source_content_type":"text/x-python","patch_set":14,"id":"a80afc9a_495c058e","line":188,"in_reply_to":"65bb93ca_61321287","updated":"2024-07-31 16:55:03.000000000","message":"Done","commit_id":"4bb01dd96a5da61b753baae380d1ab614f7734d8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0b54ee70601ae6c9626a4d5157ba7ced7bc4fa48","unresolved":true,"context_lines":[{"line_number":559,"context_line":"        This strategy allows all processes to list all task containers."},{"line_number":560,"context_line":""},{"line_number":561,"context_line":"        :param container_list: List of task containers to be processed"},{"line_number":562,"context_line":"        :return: Tuple of (process_index, total_processes, container_list)"},{"line_number":563,"context_line":"            process_index: The index of the current process;"},{"line_number":564,"context_line":"            total_processes: Total number of processes;"},{"line_number":565,"context_line":"            container_list: List of containers this process should"}],"source_content_type":"text/x-python","patch_set":14,"id":"54418134_274b98f6","line":562,"range":{"start_line":562,"start_character":27,"end_line":562,"end_character":40},"updated":"2024-07-31 14:11:01.000000000","message":"change to my_index, divisor","commit_id":"4bb01dd96a5da61b753baae380d1ab614f7734d8"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"582a2b495a3bbda33b64121ad015f3d8ec06c773","unresolved":false,"context_lines":[{"line_number":559,"context_line":"        This strategy allows all processes to list all task containers."},{"line_number":560,"context_line":""},{"line_number":561,"context_line":"        :param container_list: List of task containers to be processed"},{"line_number":562,"context_line":"        :return: Tuple of (process_index, total_processes, container_list)"},{"line_number":563,"context_line":"            process_index: The index of the current process;"},{"line_number":564,"context_line":"            total_processes: Total number of processes;"},{"line_number":565,"context_line":"            container_list: List of containers this process should"}],"source_content_type":"text/x-python","patch_set":14,"id":"dc7093b7_eb078061","line":562,"range":{"start_line":562,"start_character":27,"end_line":562,"end_character":40},"in_reply_to":"54418134_274b98f6","updated":"2024-07-31 16:55:03.000000000","message":"Acknowledged","commit_id":"4bb01dd96a5da61b753baae380d1ab614f7734d8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"478ac8bf708a9dc60eb55ec8dcca57062cf52402","unresolved":true,"context_lines":[{"line_number":365,"context_line":"        # XXX temporary shim to support the un-released configuration option"},{"line_number":366,"context_line":"        if config_true_value(conf.get("},{"line_number":367,"context_line":"                \u0027randomized_task_container_iteration\u0027, \u0027false\u0027)):"},{"line_number":368,"context_line":"            self.task_container_iteration_strategy \u003d \"randomized\""},{"line_number":369,"context_line":"        # randomized task container iteration can be useful if there\u0027s lots of"},{"line_number":370,"context_line":"        # tasks in the queue under a configured delay_reaping"},{"line_number":371,"context_line":"        if self.task_container_iteration_strategy not in \\"}],"source_content_type":"text/x-python","patch_set":25,"id":"29bb4fd7_5ec06b79","line":368,"updated":"2024-10-28 23:09:22.000000000","message":"the plan is to change all this up:\n\n```\n * get-of-jail option\n\t- new immeidately deprecated option:\n        - legacy_multi_process_task_container_iteration \u003d false (DEFAULT)\n```","commit_id":"913a6fa0d776f518cb83394e23d3b95c5fb93575"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"84eb3687a8998770892eaea3a96dda97ddb8ab92","unresolved":false,"context_lines":[{"line_number":365,"context_line":"        # XXX temporary shim to support the un-released configuration option"},{"line_number":366,"context_line":"        if config_true_value(conf.get("},{"line_number":367,"context_line":"                \u0027randomized_task_container_iteration\u0027, \u0027false\u0027)):"},{"line_number":368,"context_line":"            self.task_container_iteration_strategy \u003d \"randomized\""},{"line_number":369,"context_line":"        # randomized task container iteration can be useful if there\u0027s lots of"},{"line_number":370,"context_line":"        # tasks in the queue under a configured delay_reaping"},{"line_number":371,"context_line":"        if self.task_container_iteration_strategy not in \\"}],"source_content_type":"text/x-python","patch_set":25,"id":"12b20761_1d8fa494","line":368,"in_reply_to":"29bb4fd7_5ec06b79","updated":"2024-11-01 21:06:35.000000000","message":"i implemented this but didn\u0027t document it.","commit_id":"913a6fa0d776f518cb83394e23d3b95c5fb93575"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"84eb3687a8998770892eaea3a96dda97ddb8ab92","unresolved":true,"context_lines":[{"line_number":145,"context_line":"        r \u003d (task_container_int - 1) % self.expirer_divisor"},{"line_number":146,"context_line":"        # seconds offset should be no more than task_container_per_day i.e."},{"line_number":147,"context_line":"        # given % 86400, r\u003d\u003d86359 is ok (because 41 is less than 100), but"},{"line_number":148,"context_line":"        # 49768 would be unexpected"},{"line_number":149,"context_line":"        return self.expirer_divisor - r \u003c\u003d self.task_container_per_day"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"    def get_delete_at_nodes(self, x_delete_at, acc, cont, obj):"}],"source_content_type":"text/x-python","patch_set":28,"id":"ebb56b2b_f62a851e","line":148,"updated":"2024-11-01 21:06:35.000000000","message":"maybe this diff belongs in 920452: Deprecate expirer options | https://review.opendev.org/c/openstack/swift/+/920452","commit_id":"75fd0ddd1798398d20ee37b6e9c80596b1192e76"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"6b554579556cd5f491058e5f3e052ad491ccae3a","unresolved":false,"context_lines":[{"line_number":145,"context_line":"        r \u003d (task_container_int - 1) % self.expirer_divisor"},{"line_number":146,"context_line":"        # seconds offset should be no more than task_container_per_day i.e."},{"line_number":147,"context_line":"        # given % 86400, r\u003d\u003d86359 is ok (because 41 is less than 100), but"},{"line_number":148,"context_line":"        # 49768 would be unexpected"},{"line_number":149,"context_line":"        return self.expirer_divisor - r \u003c\u003d self.task_container_per_day"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"    def get_delete_at_nodes(self, x_delete_at, acc, cont, obj):"}],"source_content_type":"text/x-python","patch_set":28,"id":"20cc2f36_555157ff","line":148,"in_reply_to":"ebb56b2b_f62a851e","updated":"2024-11-05 00:25:56.000000000","message":"Done","commit_id":"75fd0ddd1798398d20ee37b6e9c80596b1192e76"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"986dd14d27026920cd643b1f90a20cfe773f7ddb","unresolved":true,"context_lines":[{"line_number":183,"context_line":"            # Calculate the number of days since epoch for task container time."},{"line_number":184,"context_line":"            # N.B. \"days\" refers to the default configured divisor of 86400"},{"line_number":185,"context_line":"            num_days, _ \u003d divmod("},{"line_number":186,"context_line":"                task_container_time + self.task_container_per_day,"},{"line_number":187,"context_line":"                self.expirer_divisor)"},{"line_number":188,"context_line":"            if num_days not in all_expected_containers_by_day:"},{"line_number":189,"context_line":"                # N.B. if we observe *any* task container for this \"day\" we"}],"source_content_type":"text/x-python","patch_set":38,"id":"13211304_ec4bc467","line":186,"updated":"2025-01-16 16:48:22.000000000","message":"Can you configure `expirer_divisor` \u003c `task_container_per_day`?","commit_id":"883a65beff6b49caf5b085abb42f8339885f5a6f"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"986dd14d27026920cd643b1f90a20cfe773f7ddb","unresolved":true,"context_lines":[{"line_number":197,"context_line":"                ]"},{"line_number":198,"context_line":"            if (task_container_time not in"},{"line_number":199,"context_line":"                    all_expected_containers_by_day[num_days]):"},{"line_number":200,"context_line":"                raise RuntimeError()"},{"line_number":201,"context_line":"            if not all(self.is_expected_task_container(c)"},{"line_number":202,"context_line":"                       for c in all_expected_containers_by_day[num_days]):"},{"line_number":203,"context_line":"                raise RuntimeError()"}],"source_content_type":"text/x-python","patch_set":38,"id":"1b892b86_c8f5b099","line":200,"updated":"2025-01-16 16:48:22.000000000","message":"Looks to be unreachable, given how `all_expected_containers_by_day[num_days]` is constructed. Maybe better suited to a test?","commit_id":"883a65beff6b49caf5b085abb42f8339885f5a6f"}],"test/unit/obj/test_expirer.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f6890c38f8467f4d1a59e8558e7cc35df47ef76e","unresolved":false,"context_lines":[{"line_number":1319,"context_line":"            self.assertEqual(1, divisor)"},{"line_number":1320,"context_line":"            container_per_proc[p] \u003d containers"},{"line_number":1321,"context_line":"        # assertions make the most sense against the process set"},{"line_number":1322,"context_line":"        all_expected_containers \u003d set(itertools.chain.from_iterable("},{"line_number":1323,"context_line":"            container_per_proc.values()))"},{"line_number":1324,"context_line":"        # 3 days worth of containers * 100 containers per day"},{"line_number":1325,"context_line":"        self.assertEqual(300, len(all_expected_containers))"}],"source_content_type":"text/x-python","patch_set":9,"id":"18271200_a622cb0f","line":1322,"updated":"2024-07-24 15:21:36.000000000","message":"I found myself getting confused by the use of \u0027expected\u0027 here which typically in tests suggests this is an expected value, but here we\u0027re dealing with an actual value (which happens to be containers that the expirer expect) 😕","commit_id":"f5a63a1b4f12987d22cbbf613cd3d7673ae2eba8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f950c1e90422b1d2679334da66c43dd6d865e46e","unresolved":true,"context_lines":[{"line_number":1400,"context_line":"                                      swift\u003dself.fake_swift)"},{"line_number":1401,"context_line":"            index, divisor, containers \u003d x.select_task_containers_to_expire("},{"line_number":1402,"context_line":"                \u0027.expiring_objects\u0027)"},{"line_number":1403,"context_line":"            # there\u0027s more processes than containers, each process will share"},{"line_number":1404,"context_line":"            # the container with a \"few\" other nodes - still better than all"},{"line_number":1405,"context_line":"            # nodes listing all containers and sharing with all processes!"},{"line_number":1406,"context_line":"            minimum_share \u003d ("}],"source_content_type":"text/x-python","patch_set":9,"id":"b3fbcd00_f55cf161","line":1403,"range":{"start_line":1403,"start_character":14,"end_line":1403,"end_character":52},"updated":"2024-07-22 17:32:28.000000000","message":"stale comment? 500 nodes vs 1000 containers per day","commit_id":"f5a63a1b4f12987d22cbbf613cd3d7673ae2eba8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f6890c38f8467f4d1a59e8558e7cc35df47ef76e","unresolved":false,"context_lines":[{"line_number":1400,"context_line":"                                      swift\u003dself.fake_swift)"},{"line_number":1401,"context_line":"            index, divisor, containers \u003d x.select_task_containers_to_expire("},{"line_number":1402,"context_line":"                \u0027.expiring_objects\u0027)"},{"line_number":1403,"context_line":"            # there\u0027s more processes than containers, each process will share"},{"line_number":1404,"context_line":"            # the container with a \"few\" other nodes - still better than all"},{"line_number":1405,"context_line":"            # nodes listing all containers and sharing with all processes!"},{"line_number":1406,"context_line":"            minimum_share \u003d ("}],"source_content_type":"text/x-python","patch_set":9,"id":"b029e935_41b85141","line":1403,"range":{"start_line":1403,"start_character":14,"end_line":1403,"end_character":52},"in_reply_to":"b3fbcd00_f55cf161","updated":"2024-07-24 15:21:36.000000000","message":"Done","commit_id":"f5a63a1b4f12987d22cbbf613cd3d7673ae2eba8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f950c1e90422b1d2679334da66c43dd6d865e46e","unresolved":true,"context_lines":[{"line_number":1411,"context_line":"        # assertions make the most sense against the process set"},{"line_number":1412,"context_line":"        all_expected_containers \u003d set(itertools.chain.from_iterable("},{"line_number":1413,"context_line":"            container_per_proc.values()))"},{"line_number":1414,"context_line":"        # 3 days worth of containers * 100 containers per day"},{"line_number":1415,"context_line":"        self.assertEqual(3000, len(all_expected_containers))"},{"line_number":1416,"context_line":"        # distributed evenly per day"},{"line_number":1417,"context_line":"        for containers in container_per_proc.values():"}],"source_content_type":"text/x-python","patch_set":9,"id":"d3b37a3b_91393e40","line":1414,"updated":"2024-07-22 17:32:28.000000000","message":"stale comment? task_container_per_day \u003d 1000 so 3k containers total","commit_id":"f5a63a1b4f12987d22cbbf613cd3d7673ae2eba8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f6890c38f8467f4d1a59e8558e7cc35df47ef76e","unresolved":false,"context_lines":[{"line_number":1411,"context_line":"        # assertions make the most sense against the process set"},{"line_number":1412,"context_line":"        all_expected_containers \u003d set(itertools.chain.from_iterable("},{"line_number":1413,"context_line":"            container_per_proc.values()))"},{"line_number":1414,"context_line":"        # 3 days worth of containers * 100 containers per day"},{"line_number":1415,"context_line":"        self.assertEqual(3000, len(all_expected_containers))"},{"line_number":1416,"context_line":"        # distributed evenly per day"},{"line_number":1417,"context_line":"        for containers in container_per_proc.values():"}],"source_content_type":"text/x-python","patch_set":9,"id":"35e0dc39_c375fd30","line":1414,"in_reply_to":"d3b37a3b_91393e40","updated":"2024-07-24 15:21:36.000000000","message":"Done","commit_id":"f5a63a1b4f12987d22cbbf613cd3d7673ae2eba8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f950c1e90422b1d2679334da66c43dd6d865e46e","unresolved":true,"context_lines":[{"line_number":1466,"context_line":"        # 3 days worth of containers * 100 containers per day (+ unexpected!)"},{"line_number":1467,"context_line":"        self.assertEqual(300 + len(unexpected_containers),"},{"line_number":1468,"context_line":"                         len(all_expected_containers))"},{"line_number":1469,"context_line":"        # this one is crazy; I\u0027m not sure I understand it fully"},{"line_number":1470,"context_line":"        found_per_proc_container_len_count \u003d defaultdict(int)"},{"line_number":1471,"context_line":"        for containers in container_per_proc.values():"},{"line_number":1472,"context_line":"            found_per_proc_container_len_count[len(containers)] +\u003d 1"}],"source_content_type":"text/x-python","patch_set":9,"id":"c99ddd89_236acd62","line":1469,"updated":"2024-07-22 17:32:28.000000000","message":"I understood it as:\nthere\u0027s 3 normal days worth of containers, plus another day-equivalent of unexpected, plus another 67 unexpected.\n\nSo all procs get at least 4 containers, then the 67 are striped across 900 procs and 9 * 67 \u003d 603 procs get a 5th container","commit_id":"f5a63a1b4f12987d22cbbf613cd3d7673ae2eba8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f6890c38f8467f4d1a59e8558e7cc35df47ef76e","unresolved":false,"context_lines":[{"line_number":1466,"context_line":"        # 3 days worth of containers * 100 containers per day (+ unexpected!)"},{"line_number":1467,"context_line":"        self.assertEqual(300 + len(unexpected_containers),"},{"line_number":1468,"context_line":"                         len(all_expected_containers))"},{"line_number":1469,"context_line":"        # this one is crazy; I\u0027m not sure I understand it fully"},{"line_number":1470,"context_line":"        found_per_proc_container_len_count \u003d defaultdict(int)"},{"line_number":1471,"context_line":"        for containers in container_per_proc.values():"},{"line_number":1472,"context_line":"            found_per_proc_container_len_count[len(containers)] +\u003d 1"}],"source_content_type":"text/x-python","patch_set":9,"id":"7bcfd0e7_d1474a47","line":1469,"in_reply_to":"c99ddd89_236acd62","updated":"2024-07-24 15:21:36.000000000","message":"Done","commit_id":"f5a63a1b4f12987d22cbbf613cd3d7673ae2eba8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f6890c38f8467f4d1a59e8558e7cc35df47ef76e","unresolved":true,"context_lines":[{"line_number":2322,"context_line":"            all_delete_object_count +\u003d len(o_delete_object)"},{"line_number":2323,"context_line":"            # each process gets into today/ready after listing fewer containers"},{"line_number":2324,"context_line":"            self.assertLess(o_num_iter_before_delete,"},{"line_number":2325,"context_line":"                            expected_calls[\u0027listing_before_delete\u0027])"},{"line_number":2326,"context_line":""},{"line_number":2327,"context_line":"        # but in aggregate all containers are listed and ready tasks reaped"},{"line_number":2328,"context_line":"        self.assertEqual(all_iter_object_count, expected_calls[\u0027listing\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"446c0087_101a7489","line":2325,"updated":"2024-07-24 15:21:36.000000000","message":"So this is comparing the actual number of listings per proc using parallel (each proc will have divisor 1) vs the number of listings for a single proc using \u0027serial\u0027 with divisor 1. But I think with serial each of the three processes would  have divisor 3, so actually need 3x more listings to fill their round robin caches?? \n\nWith parallel, each proc gets to completely ignore 2/3 of the \u0027older\u0027 containers and gets to the reapable tasks sooner.\n\nWith serial, each proc (a) has to list all the \u0027older\u0027 containers, but also (b) only finds tasks at a rate of 1 in 3 of those that are reapable.\n\nWith the diff below I get \n\n```\ntest/unit/obj/test_expirer.py:2370 (TestObjectExpirer.test_parallel_task_container_iteration_hastens_deletes)\n167 !\u003d 68\n```\n\nbut if I pass divisor\u003d1 to ``self._count_expected_calls`` it\u0027s 134 !\u003d 68, which I _think_ makes sense - with serial mode, the bigger the divisor the more listings each proc has to make to gather sufficient tasks.\n\nEither way parallel is better, I\u0027m just trying to grok the assertions!\n\n```\ndiff --git a/test/unit/obj/test_expirer.py b/test/unit/obj/test_expirer.py\nindex b89a4fab3..7ef1fa46c 100644\n--- a/test/unit/obj/test_expirer.py\n+++ b/test/unit/obj/test_expirer.py\n@@ -2233,7 +2233,7 @@ class TestObjectExpirer(TestCase):\n             target_account, target_container)\n         return delete_timestamp \u003c\u003d Timestamp(now - delay_reaping)\n\n-    def _count_expected_calls(self, now, task_containers):\n+    def _count_expected_calls(self, now, task_containers, divisor\u003d1):\n         \"\"\"\n         This counts how many iterations through tasks in the provided\n         task_containers would be needed to fill the\n@@ -2247,7 +2247,10 @@ class TestObjectExpirer(TestCase):\n         for task_container in task_containers:\n             if Timestamp(task_container) \u003e now:\n                 continue\n-            for task_object in expirer_account[task_container]:\n+            for index, task_object in enumerate(\n+                    expirer_account[task_container]):\n+                if index % divisor:\n+                    continue\n                 if self._is_task_reapable(now, task_object):\n                     num_expected_delete_calls +\u003d 1\n             num_expected_listing_calls +\u003d 1\n@@ -2393,7 +2396,7 @@ class TestObjectExpirer(TestCase):\n         # calculate expectations based on serial process configuration\n         task_containers \u003d self.fake_swift.aco_dict[\u0027.expiring_objects\u0027].keys()\n         expected_calls \u003d self._count_expected_calls(\n-            now, sorted(task_containers))\n+            now, sorted(task_containers), divisor\u003d3)\n\n         # ... but take observation of distributed process configuration\n         self.conf[\u0027task_container_iteration_strategy\u0027] \u003d \u0027parallel\u0027\n@@ -2413,7 +2416,7 @@ class TestObjectExpirer(TestCase):\n             all_iter_object_count +\u003d len(o_iter_object)\n             all_delete_object_count +\u003d len(o_delete_object)\n             # each process gets into today/ready after listing fewer containers\n-            self.assertLess(o_num_iter_before_delete,\n+            self.assertEqual(o_num_iter_before_delete,\n                             expected_calls[\u0027listing_before_delete\u0027])\n\n         # but in aggregate all containers are listed and ready tasks reaped\n         ```","commit_id":"f5a63a1b4f12987d22cbbf613cd3d7673ae2eba8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c35cf6bde61c0905c5315a70990602a26b8fe9ea","unresolved":false,"context_lines":[{"line_number":2322,"context_line":"            all_delete_object_count +\u003d len(o_delete_object)"},{"line_number":2323,"context_line":"            # each process gets into today/ready after listing fewer containers"},{"line_number":2324,"context_line":"            self.assertLess(o_num_iter_before_delete,"},{"line_number":2325,"context_line":"                            expected_calls[\u0027listing_before_delete\u0027])"},{"line_number":2326,"context_line":""},{"line_number":2327,"context_line":"        # but in aggregate all containers are listed and ready tasks reaped"},{"line_number":2328,"context_line":"        self.assertEqual(all_iter_object_count, expected_calls[\u0027listing\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"c242cccb_7677a3fc","line":2325,"in_reply_to":"446c0087_101a7489","updated":"2024-10-24 18:54:34.000000000","message":"`_count_expected_calls` does seem to assume `divisor \u003d 1`; which is probably a \"best case\" IF you\u0027re iterating all task containers and performing all tasks.\n\nI think if we want to use `divsor` in `_count_expected_calls` \"correctly\" we\u0027d have to do the same \"hash_path\" calculation as the serial strategy; but it would just make the expecation \"worse\" (i.e. you still have to spin past delay_repaing tasks; but once you get into the \"meat\" you\u0027re not even getting a full bite!)\n\nThe idea is just that we don\u0027t have to list ALL the delay reaping containers before we start doing deletes; there\u0027s probably other ways we could say the same thing.","commit_id":"f5a63a1b4f12987d22cbbf613cd3d7673ae2eba8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f6890c38f8467f4d1a59e8558e7cc35df47ef76e","unresolved":true,"context_lines":[{"line_number":2325,"context_line":"                            expected_calls[\u0027listing_before_delete\u0027])"},{"line_number":2326,"context_line":""},{"line_number":2327,"context_line":"        # but in aggregate all containers are listed and ready tasks reaped"},{"line_number":2328,"context_line":"        self.assertEqual(all_iter_object_count, expected_calls[\u0027listing\u0027])"},{"line_number":2329,"context_line":"        self.assertEqual(all_delete_object_count, expected_calls[\u0027delete\u0027])"},{"line_number":2330,"context_line":""},{"line_number":2331,"context_line":"    def test_run_once_unicode_problem(self):"}],"source_content_type":"text/x-python","patch_set":9,"id":"371571cc_04c97475","line":2328,"updated":"2024-07-24 15:21:36.000000000","message":"so here are we comparing aggregate number of listings across _all_ parallel procs with number of listings for _each_ serial proc? The assertion usefully confirms that all containers are listed, but IIUC the aggregate number of listing in the serial case is actually 3x the aggregate number of listings in the parallel case.\n\nor maybe I am confused.","commit_id":"f5a63a1b4f12987d22cbbf613cd3d7673ae2eba8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c35cf6bde61c0905c5315a70990602a26b8fe9ea","unresolved":false,"context_lines":[{"line_number":2325,"context_line":"                            expected_calls[\u0027listing_before_delete\u0027])"},{"line_number":2326,"context_line":""},{"line_number":2327,"context_line":"        # but in aggregate all containers are listed and ready tasks reaped"},{"line_number":2328,"context_line":"        self.assertEqual(all_iter_object_count, expected_calls[\u0027listing\u0027])"},{"line_number":2329,"context_line":"        self.assertEqual(all_delete_object_count, expected_calls[\u0027delete\u0027])"},{"line_number":2330,"context_line":""},{"line_number":2331,"context_line":"    def test_run_once_unicode_problem(self):"}],"source_content_type":"text/x-python","patch_set":9,"id":"e7c6e764_1ae10e30","line":2328,"in_reply_to":"371571cc_04c97475","updated":"2024-10-24 18:54:34.000000000","message":"oic, that makes sense, when we get the \"expected_calls\" we comment:\n\n```\n        # calculate expectations based on serial process configuration\n```\n\nWhich sounds A LOT like \"serial processES configuration\" - I see how this could be confusing; but the \"expected_calls\" helper describes itself as:\n\n```\n        This counts how many iterations through tasks in the provided\n        task_containers would be needed to fill the\n        round_robin_task_cache_size.\n```\n\nso it\u0027s really just talking about \"iterating ALL containers/tasks serially and skip the delay_reaping tasks\" not \"iterating all containers/task and skipping delay reaping AND then ALSO some *other* random tasks like a serial multi-process worker configuration does currently\"\n\nI\u0027m skeptical of giving these assertions anymore slack than necessary.  But we could do *another* test that compares a 3 node serial run with a 3 node parallel run?  All of the *observstations* would be the same - but the base line values for what we\u0027re comparing the behaviors to would be even WORSE.   I\u0027m torn about the maintance value/overhead of having another mostly duplicated test with weaker assertions because it COULD be useful for understanding the motivatoin of this new feature if you didn\u0027t have access to the commit message for example.  I\u0027m going to skip that work for now, but we can revisit the issue or add additional tests in a follow-up.","commit_id":"f5a63a1b4f12987d22cbbf613cd3d7673ae2eba8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"165ac405346d68b7bc5ad898222370395988a2cb","unresolved":true,"context_lines":[{"line_number":1374,"context_line":"        # expected container sort in descending order across processes"},{"line_number":1375,"context_line":"        self.assertEqual({0: (0, 1, [86398, 86400, 172798, 172800]),"},{"line_number":1376,"context_line":"                          1: (0, 1, [86399, 172799])},"},{"line_number":1377,"context_line":"                         actual)"},{"line_number":1378,"context_line":""},{"line_number":1379,"context_line":"        actual \u003d self._do_test_parallel_strategy("},{"line_number":1380,"context_line":"            containers, processes\u003d3, task_container_per_day\u003d3)"}],"source_content_type":"text/x-python","patch_set":12,"id":"4e7b2b14_d70a9c52","line":1377,"updated":"2024-07-25 10:22:03.000000000","message":"it would be nice-to-have (but not must-have) the containers more uniformly distributed across processes in the long term.","commit_id":"1d6cc6936c13e3deea674d8e93caa32599097d15"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c35cf6bde61c0905c5315a70990602a26b8fe9ea","unresolved":true,"context_lines":[{"line_number":1374,"context_line":"        # expected container sort in descending order across processes"},{"line_number":1375,"context_line":"        self.assertEqual({0: (0, 1, [86398, 86400, 172798, 172800]),"},{"line_number":1376,"context_line":"                          1: (0, 1, [86399, 172799])},"},{"line_number":1377,"context_line":"                         actual)"},{"line_number":1378,"context_line":""},{"line_number":1379,"context_line":"        actual \u003d self._do_test_parallel_strategy("},{"line_number":1380,"context_line":"            containers, processes\u003d3, task_container_per_day\u003d3)"}],"source_content_type":"text/x-python","patch_set":12,"id":"a5f6b112_36d2983a","line":1377,"in_reply_to":"4e7b2b14_d70a9c52","updated":"2024-10-24 18:54:34.000000000","message":"yeah that\u0027s weird - 2 proc \u0026 6 task_container - why is it uneven?\n\ninteresting; so we iterate the containers in each day:\n\n```\n[86400, 86399, 86398]\n[172800, 172799, 172798]\n```\n\nand assign each process a slice:\n\n```\n0: slice(0, 3, 2)\n1: slice(1, 3, 2)\n```\n\nSo this results in process-0 getting 2 and process-1 getting 1 container out of the 3 for each \"day\"\n\nThat doesn\u0027t seem optimal!  Perhaps some virtue in brining back the `distribute_evently` algorithm?","commit_id":"1d6cc6936c13e3deea674d8e93caa32599097d15"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"165ac405346d68b7bc5ad898222370395988a2cb","unresolved":true,"context_lines":[{"line_number":1474,"context_line":"                          1: (0, 1, [86397, 86399]),"},{"line_number":1475,"context_line":"                          2: (0, 1, [86398]),"},{"line_number":1476,"context_line":"                          3: (1, 2, [86396, 86400])},"},{"line_number":1477,"context_line":"                         actual)"},{"line_number":1478,"context_line":""},{"line_number":1479,"context_line":"    def test_select_task_containers_to_expire_single_process(self):"},{"line_number":1480,"context_line":"        x \u003d expirer.ObjectExpirer(self.conf, logger\u003dself.logger,"}],"source_content_type":"text/x-python","patch_set":12,"id":"27a76cce_cc754b59","line":1477,"updated":"2024-07-25 10:22:03.000000000","message":"these tests added in patchset 10 also pass with the previous implementation of _parallel_strategy in patchset 9","commit_id":"1d6cc6936c13e3deea674d8e93caa32599097d15"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"17483026f37f04a4e6453f3263b36608c87a08d0","unresolved":false,"context_lines":[{"line_number":1474,"context_line":"                          1: (0, 1, [86397, 86399]),"},{"line_number":1475,"context_line":"                          2: (0, 1, [86398]),"},{"line_number":1476,"context_line":"                          3: (1, 2, [86396, 86400])},"},{"line_number":1477,"context_line":"                         actual)"},{"line_number":1478,"context_line":""},{"line_number":1479,"context_line":"    def test_select_task_containers_to_expire_single_process(self):"},{"line_number":1480,"context_line":"        x \u003d expirer.ObjectExpirer(self.conf, logger\u003dself.logger,"}],"source_content_type":"text/x-python","patch_set":12,"id":"388955c4_0cc0036e","line":1477,"in_reply_to":"27a76cce_cc754b59","updated":"2024-07-30 18:07:38.000000000","message":"Acknowledged","commit_id":"1d6cc6936c13e3deea674d8e93caa32599097d15"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"752ffb4f67567b868adb5a36dfacc2b312284148","unresolved":false,"context_lines":[{"line_number":2797,"context_line":""},{"line_number":2798,"context_line":"        self.assertEqual("},{"line_number":2799,"context_line":"            {\u0027tasks.assigned\u0027: 1000,"},{"line_number":2800,"context_line":"                \u0027tasks.delayed\u0027: 2000,"},{"line_number":2801,"context_line":"                \u0027objects\u0027: 1000},"},{"line_number":2802,"context_line":"            self.expirer.logger.statsd_client.get_increment_counts()"},{"line_number":2803,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":17,"id":"8bc92160_50e2bf3a","line":2800,"updated":"2024-08-02 18:18:32.000000000","message":"got this extra tab fixed.","commit_id":"3943942aa0384bf87703658312aa28b11e5f90f8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"84eb3687a8998770892eaea3a96dda97ddb8ab92","unresolved":true,"context_lines":[{"line_number":2668,"context_line":""},{"line_number":2669,"context_line":""},{"line_number":2670,"context_line":"if __name__ \u003d\u003d \u0027__main__\u0027:"},{"line_number":2671,"context_line":"    main()"}],"source_content_type":"text/x-python","patch_set":28,"id":"5361dd32_463bb37b","side":"PARENT","line":2671,"updated":"2024-11-01 21:06:35.000000000","message":"i\u0027m still getting used to a new editor","commit_id":"0febd2ab06bd56db511b2c87c0203a9975ab397e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"6b554579556cd5f491058e5f3e052ad491ccae3a","unresolved":false,"context_lines":[{"line_number":2668,"context_line":""},{"line_number":2669,"context_line":""},{"line_number":2670,"context_line":"if __name__ \u003d\u003d \u0027__main__\u0027:"},{"line_number":2671,"context_line":"    main()"}],"source_content_type":"text/x-python","patch_set":28,"id":"5b05a884_3e82001e","side":"PARENT","line":2671,"in_reply_to":"5361dd32_463bb37b","updated":"2024-11-05 00:25:56.000000000","message":"Done","commit_id":"0febd2ab06bd56db511b2c87c0203a9975ab397e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"84eb3687a8998770892eaea3a96dda97ddb8ab92","unresolved":true,"context_lines":[{"line_number":25,"context_line":"from copy import deepcopy"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"from test.debug_logger import debug_logger"},{"line_number":28,"context_line":"from test.unit import FakeRing, mocked_http_conn, make_timestamp_iter"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"import mock"},{"line_number":31,"context_line":"import six"}],"source_content_type":"text/x-python","patch_set":28,"id":"8ea7cd4f_e75d63c8","line":28,"updated":"2024-11-01 21:06:35.000000000","message":"seems like an unrelated diff","commit_id":"75fd0ddd1798398d20ee37b6e9c80596b1192e76"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"6b554579556cd5f491058e5f3e052ad491ccae3a","unresolved":false,"context_lines":[{"line_number":25,"context_line":"from copy import deepcopy"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"from test.debug_logger import debug_logger"},{"line_number":28,"context_line":"from test.unit import FakeRing, mocked_http_conn, make_timestamp_iter"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"import mock"},{"line_number":31,"context_line":"import six"}],"source_content_type":"text/x-python","patch_set":28,"id":"5c74bd31_04189acd","line":28,"in_reply_to":"8ea7cd4f_e75d63c8","updated":"2024-11-05 00:25:56.000000000","message":"Done","commit_id":"75fd0ddd1798398d20ee37b6e9c80596b1192e76"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"84eb3687a8998770892eaea3a96dda97ddb8ab92","unresolved":true,"context_lines":[{"line_number":737,"context_line":"        with self.assertRaises(ValueError) as ctx:"},{"line_number":738,"context_line":"            expirer.ObjectExpirer(conf, logger\u003dself.logger,"},{"line_number":739,"context_line":"                                  swift\u003dself.fake_swift)"},{"line_number":740,"context_line":"        self.assertIn(\u0027invalid literal for int\u0027, str(ctx.exception))"},{"line_number":741,"context_line":""},{"line_number":742,"context_line":"    def test_init_task_container_iteration_strategy_default(self):"},{"line_number":743,"context_line":"        conf \u003d {}"}],"source_content_type":"text/x-python","patch_set":28,"id":"73796721_5862ce9e","line":740,"updated":"2024-11-01 21:06:35.000000000","message":"seems like some of these belong in the patch we already merged... \n\n914713: object-expirer: add round_robin_cache_size option | https://review.opendev.org/c/openstack/swift/+/914713\n\nmaybe they\u0027re just duplicated?","commit_id":"75fd0ddd1798398d20ee37b6e9c80596b1192e76"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"6b554579556cd5f491058e5f3e052ad491ccae3a","unresolved":false,"context_lines":[{"line_number":737,"context_line":"        with self.assertRaises(ValueError) as ctx:"},{"line_number":738,"context_line":"            expirer.ObjectExpirer(conf, logger\u003dself.logger,"},{"line_number":739,"context_line":"                                  swift\u003dself.fake_swift)"},{"line_number":740,"context_line":"        self.assertIn(\u0027invalid literal for int\u0027, str(ctx.exception))"},{"line_number":741,"context_line":""},{"line_number":742,"context_line":"    def test_init_task_container_iteration_strategy_default(self):"},{"line_number":743,"context_line":"        conf \u003d {}"}],"source_content_type":"text/x-python","patch_set":28,"id":"b6c5d292_50187ca7","line":740,"in_reply_to":"73796721_5862ce9e","updated":"2024-11-05 00:25:56.000000000","message":"they did not seem to be duplicated with any existing test coverage I could find, these look like new useful tests for an unrelated config option.\n\nI moved them out of this chain:\n\n934093: tests: additional validation for round_robin_cache_size | https://review.opendev.org/c/openstack/swift/+/934093","commit_id":"75fd0ddd1798398d20ee37b6e9c80596b1192e76"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"84eb3687a8998770892eaea3a96dda97ddb8ab92","unresolved":true,"context_lines":[{"line_number":2759,"context_line":""},{"line_number":2760,"context_line":"    def _expirer_run_once_with_mocks("},{"line_number":2761,"context_line":"            self, now\u003dNone,"},{"line_number":2762,"context_line":"            stub_pop_queue\u003dNone):"},{"line_number":2763,"context_line":"        \"\"\""},{"line_number":2764,"context_line":"        call self.expirer.run_once() with some things (optionally) stubbed out"},{"line_number":2765,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":28,"id":"bfdf24ea_7b8d603a","line":2762,"updated":"2024-11-01 21:06:35.000000000","message":"unrelated diff churn","commit_id":"75fd0ddd1798398d20ee37b6e9c80596b1192e76"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"6b554579556cd5f491058e5f3e052ad491ccae3a","unresolved":false,"context_lines":[{"line_number":2759,"context_line":""},{"line_number":2760,"context_line":"    def _expirer_run_once_with_mocks("},{"line_number":2761,"context_line":"            self, now\u003dNone,"},{"line_number":2762,"context_line":"            stub_pop_queue\u003dNone):"},{"line_number":2763,"context_line":"        \"\"\""},{"line_number":2764,"context_line":"        call self.expirer.run_once() with some things (optionally) stubbed out"},{"line_number":2765,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":28,"id":"54c4a8b7_6ebd082d","line":2762,"in_reply_to":"bfdf24ea_7b8d603a","updated":"2024-11-05 00:25:56.000000000","message":"Done","commit_id":"75fd0ddd1798398d20ee37b6e9c80596b1192e76"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"84eb3687a8998770892eaea3a96dda97ddb8ab92","unresolved":true,"context_lines":[{"line_number":3109,"context_line":"            for target_path in self.expired_target_paths[self.just_past_time]"},{"line_number":3110,"context_line":"        ])"},{"line_number":3111,"context_line":"        self.assertEqual(self.expirer.logger.get_lines_for_level(\u0027info\u0027), ["},{"line_number":3112,"context_line":"            \u0027Pass beginning (serial strategy) for task account \u0027"},{"line_number":3113,"context_line":"            \u0027.expiring_objects; 4 possible containers; 12 possible objects\u0027,"},{"line_number":3114,"context_line":"            \u0027Pass completed in 0s; 5 objects expired\u0027,"},{"line_number":3115,"context_line":"        ])"}],"source_content_type":"text/x-python","patch_set":28,"id":"93c41168_90896a61","line":3112,"updated":"2024-11-01 21:06:35.000000000","message":"hrmm... I don\u0027t see any assertions on this log line with `(parallel strategy)`","commit_id":"75fd0ddd1798398d20ee37b6e9c80596b1192e76"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"6b554579556cd5f491058e5f3e052ad491ccae3a","unresolved":false,"context_lines":[{"line_number":3109,"context_line":"            for target_path in self.expired_target_paths[self.just_past_time]"},{"line_number":3110,"context_line":"        ])"},{"line_number":3111,"context_line":"        self.assertEqual(self.expirer.logger.get_lines_for_level(\u0027info\u0027), ["},{"line_number":3112,"context_line":"            \u0027Pass beginning (serial strategy) for task account \u0027"},{"line_number":3113,"context_line":"            \u0027.expiring_objects; 4 possible containers; 12 possible objects\u0027,"},{"line_number":3114,"context_line":"            \u0027Pass completed in 0s; 5 objects expired\u0027,"},{"line_number":3115,"context_line":"        ])"}],"source_content_type":"text/x-python","patch_set":28,"id":"c1a84472_70543b35","line":3112,"in_reply_to":"93c41168_90896a61","updated":"2024-11-05 00:25:56.000000000","message":"Done","commit_id":"75fd0ddd1798398d20ee37b6e9c80596b1192e76"}]}