)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9c860da0dbb5cda8c53ba338263839d1521f3a95","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"b562b22e_eaf45fb9","updated":"2026-04-24 13:48:36.000000000","message":"I think this reveals test deficiency on master https://review.opendev.org/c/openstack/swift/+/986089 , but I think it is ok to merge and then pull the fix from master later?","commit_id":"d599203e9218ec95f34846736042a41f45300ce9"}],"swift/common/middleware/s3api/controllers/multi_upload.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b8e5ba8f22dc8f7177e6ec987d0aa24245d60296","unresolved":true,"context_lines":[{"line_number":553,"context_line":"                \u0027Conditional uploads are not supported.\u0027)"},{"line_number":554,"context_line":""},{"line_number":555,"context_line":"        resp, is_marker \u003d _get_upload_info(req, self.app, upload_id)"},{"line_number":556,"context_line":"        marker_delta \u003d ("},{"line_number":557,"context_line":"            float(Timestamp(resp.sw_headers.get(\u0027X-Backend-Timestamp\u0027, \u00270\u0027)))"},{"line_number":558,"context_line":"            - float(NormalTimestamp.now()))"},{"line_number":559,"context_line":"        if is_marker and marker_delta \u003e\u003d 0:"},{"line_number":560,"context_line":"            # Somehow the marker was created in the future w.r.t. this thread\u0027s"},{"line_number":561,"context_line":"            # clock. The manifest PUT may succeed but the subsequent marker"},{"line_number":562,"context_line":"            # DELETE will fail, so don\u0027t attempt either."},{"line_number":563,"context_line":"            self.logger.error(\u0027Unable to Complete Multipart Upload,\u0027"},{"line_number":564,"context_line":"                              \u0027 marker is %0.5fs newer\u0027, marker_delta)"},{"line_number":565,"context_line":"            raise ServiceUnavailable(reason\u003d\u0027mpu_clock_skew\u0027)"},{"line_number":566,"context_line":""},{"line_number":567,"context_line":"        headers \u003d {\u0027Accept\u0027: \u0027application/json\u0027,"},{"line_number":568,"context_line":"                   sysmeta_header(\u0027object\u0027, \u0027upload-id\u0027): upload_id}"}],"source_content_type":"text/x-python","patch_set":1,"id":"0203e491_8fb1d0cd","side":"PARENT","line":565,"range":{"start_line":556,"start_character":8,"end_line":565,"end_character":61},"updated":"2026-04-21 15:14:08.000000000","message":"This hunk (from https://review.opendev.org/c/openstack/swift/+/984467) just kinda went away -- does that seem right to you, @alistairncoles@gmail.com?\n\nThis reminds me: what\u0027s the upgrade plan for in-progress uploads?","commit_id":"99bb7e44d8f3e93222412176240b4ff9af32d7f0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"ba3ba60da267e99ba3a4cca05feb8f5af38cfb62","unresolved":true,"context_lines":[{"line_number":553,"context_line":"                \u0027Conditional uploads are not supported.\u0027)"},{"line_number":554,"context_line":""},{"line_number":555,"context_line":"        resp, is_marker \u003d _get_upload_info(req, self.app, upload_id)"},{"line_number":556,"context_line":"        marker_delta \u003d ("},{"line_number":557,"context_line":"            float(Timestamp(resp.sw_headers.get(\u0027X-Backend-Timestamp\u0027, \u00270\u0027)))"},{"line_number":558,"context_line":"            - float(NormalTimestamp.now()))"},{"line_number":559,"context_line":"        if is_marker and marker_delta \u003e\u003d 0:"},{"line_number":560,"context_line":"            # Somehow the marker was created in the future w.r.t. this thread\u0027s"},{"line_number":561,"context_line":"            # clock. The manifest PUT may succeed but the subsequent marker"},{"line_number":562,"context_line":"            # DELETE will fail, so don\u0027t attempt either."},{"line_number":563,"context_line":"            self.logger.error(\u0027Unable to Complete Multipart Upload,\u0027"},{"line_number":564,"context_line":"                              \u0027 marker is %0.5fs newer\u0027, marker_delta)"},{"line_number":565,"context_line":"            raise ServiceUnavailable(reason\u003d\u0027mpu_clock_skew\u0027)"},{"line_number":566,"context_line":""},{"line_number":567,"context_line":"        headers \u003d {\u0027Accept\u0027: \u0027application/json\u0027,"},{"line_number":568,"context_line":"                   sysmeta_header(\u0027object\u0027, \u0027upload-id\u0027): upload_id}"}],"source_content_type":"text/x-python","patch_set":1,"id":"98620497_3f5839ca","side":"PARENT","line":565,"range":{"start_line":556,"start_character":8,"end_line":565,"end_character":61},"in_reply_to":"0203e491_8fb1d0cd","updated":"2026-04-28 11:13:50.000000000","message":"\u003eThis hunk (from https://review.opendev.org/c/openstack/swift/+/984467) just kinda went away -- does that seem right to you, \n\nIt\u0027s right inasmuch as on this branch the legacy multi_upload controller is replaced by a prototype mpu controller that is quite different in places and isn\u0027t yet complete. So I haven\u0027t been trying to keep it in sync with the master middleware, and the unit tests all skip.\n\nAt some point the new impl will need to grow unit tests and be audited against the old.\n\nThat might be the wrong strategy. An alternative would be to implement a different controller module (multi_upload2.py ??) on this branch and keep multi_upload.py in sync with master. That would at least eliminate merge conflict distractions.\n\nI\u0027d be happy to switch to a new controller module and revert multi_upload.py to master??\n\n\u003e This reminds me: what\u0027s the upgrade plan for in-progress uploads?\n\nI haven\u0027t worked that out yet. We may need two controllers and fallback to the old if the marker is detected to be legacy. Could be another reason to ultimately have two controller modules.","commit_id":"99bb7e44d8f3e93222412176240b4ff9af32d7f0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0d2fc1aeccab33d5b6ace8e284cfcbf327e09fef","unresolved":true,"context_lines":[{"line_number":553,"context_line":"                \u0027Conditional uploads are not supported.\u0027)"},{"line_number":554,"context_line":""},{"line_number":555,"context_line":"        resp, is_marker \u003d _get_upload_info(req, self.app, upload_id)"},{"line_number":556,"context_line":"        marker_delta \u003d ("},{"line_number":557,"context_line":"            float(Timestamp(resp.sw_headers.get(\u0027X-Backend-Timestamp\u0027, \u00270\u0027)))"},{"line_number":558,"context_line":"            - float(NormalTimestamp.now()))"},{"line_number":559,"context_line":"        if is_marker and marker_delta \u003e\u003d 0:"},{"line_number":560,"context_line":"            # Somehow the marker was created in the future w.r.t. this thread\u0027s"},{"line_number":561,"context_line":"            # clock. The manifest PUT may succeed but the subsequent marker"},{"line_number":562,"context_line":"            # DELETE will fail, so don\u0027t attempt either."},{"line_number":563,"context_line":"            self.logger.error(\u0027Unable to Complete Multipart Upload,\u0027"},{"line_number":564,"context_line":"                              \u0027 marker is %0.5fs newer\u0027, marker_delta)"},{"line_number":565,"context_line":"            raise ServiceUnavailable(reason\u003d\u0027mpu_clock_skew\u0027)"},{"line_number":566,"context_line":""},{"line_number":567,"context_line":"        headers \u003d {\u0027Accept\u0027: \u0027application/json\u0027,"},{"line_number":568,"context_line":"                   sysmeta_header(\u0027object\u0027, \u0027upload-id\u0027): upload_id}"}],"source_content_type":"text/x-python","patch_set":1,"id":"e584db25_cfeb2b4d","side":"PARENT","line":565,"range":{"start_line":556,"start_character":8,"end_line":565,"end_character":61},"in_reply_to":"961971d8_b074862f","updated":"2026-04-29 09:18:49.000000000","message":"\u003e Do we think there\u0027ll be an appetite to do a feature/mpu-review branch for the final merge?\n\nAlmost certainly.","commit_id":"99bb7e44d8f3e93222412176240b4ff9af32d7f0"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d9a70597d4614916ee46635fb29202ac98b76de0","unresolved":true,"context_lines":[{"line_number":553,"context_line":"                \u0027Conditional uploads are not supported.\u0027)"},{"line_number":554,"context_line":""},{"line_number":555,"context_line":"        resp, is_marker \u003d _get_upload_info(req, self.app, upload_id)"},{"line_number":556,"context_line":"        marker_delta \u003d ("},{"line_number":557,"context_line":"            float(Timestamp(resp.sw_headers.get(\u0027X-Backend-Timestamp\u0027, \u00270\u0027)))"},{"line_number":558,"context_line":"            - float(NormalTimestamp.now()))"},{"line_number":559,"context_line":"        if is_marker and marker_delta \u003e\u003d 0:"},{"line_number":560,"context_line":"            # Somehow the marker was created in the future w.r.t. this thread\u0027s"},{"line_number":561,"context_line":"            # clock. The manifest PUT may succeed but the subsequent marker"},{"line_number":562,"context_line":"            # DELETE will fail, so don\u0027t attempt either."},{"line_number":563,"context_line":"            self.logger.error(\u0027Unable to Complete Multipart Upload,\u0027"},{"line_number":564,"context_line":"                              \u0027 marker is %0.5fs newer\u0027, marker_delta)"},{"line_number":565,"context_line":"            raise ServiceUnavailable(reason\u003d\u0027mpu_clock_skew\u0027)"},{"line_number":566,"context_line":""},{"line_number":567,"context_line":"        headers \u003d {\u0027Accept\u0027: \u0027application/json\u0027,"},{"line_number":568,"context_line":"                   sysmeta_header(\u0027object\u0027, \u0027upload-id\u0027): upload_id}"}],"source_content_type":"text/x-python","patch_set":1,"id":"961971d8_b074862f","side":"PARENT","line":565,"range":{"start_line":556,"start_character":8,"end_line":565,"end_character":61},"in_reply_to":"98620497_3f5839ca","updated":"2026-04-28 21:23:09.000000000","message":"I\u0027m fine with merging this -- I just wanted to flag it up as a notable thing that changed on `master` but is (currently) irrelevant on `feature/mpu`. We\u0027ll just want to keep an eye out for it as we *do* figure out our upgrade story.\n\nAs far as tactics, I\u0027m flexible. It would be fairly easy to revert `multi_upload.py` to master, but I definitely don\u0027t want to lose sight of the plan for how \"new\" MPUs work. A pair of controllers feels likely to meet our upgrade needs -- I might suggest `multi_upload.py` and `multi_upload_slo.py` (or maybe `multi_upload_legacy.py`?) as names though.\n\nDo we think there\u0027ll be an appetite to do a `feature/mpu-review` branch for the final merge? If so, I think it may make it easier to track the history through a rename.","commit_id":"99bb7e44d8f3e93222412176240b4ff9af32d7f0"}],"swift/common/middleware/versioned_writes/object_versioning.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c9b31458f44f03a771647233b44f158a7ee5d2f3","unresolved":true,"context_lines":[{"line_number":1277,"context_line":"                params[\u0027marker\u0027] \u003d build_versions_object_max_name("},{"line_number":1278,"context_line":"                    params[\u0027marker\u0027])"},{"line_number":1279,"context_line":"            elif params[\u0027version_marker\u0027] \u003d\u003d \u0027null\u0027:"},{"line_number":1280,"context_line":"                params[\u0027marker\u0027] \u003d get_reserved_name(params[\u0027marker\u0027], \u0027\u0027)"},{"line_number":1281,"context_line":"            else:"},{"line_number":1282,"context_line":"                try:"},{"line_number":1283,"context_line":"                    version \u003d params.pop(\u0027version_marker\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5e5a81e9_f29b9184","line":1280,"range":{"start_line":1280,"start_character":35,"end_line":1280,"end_character":74},"updated":"2026-04-22 14:25:26.000000000","message":"Maybe should get abstracted to a `build_versions_object_prefix` like in https://review.opendev.org/c/openstack/swift/+/985691/1/swift/common/middleware/versioned_writes/object_versioning.py#234","commit_id":"9ea555881df01fd0e6d8e22cb758d314318693c1"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"600fdc5a2561a6e5d380499be7eab779233572e4","unresolved":false,"context_lines":[{"line_number":1277,"context_line":"                params[\u0027marker\u0027] \u003d build_versions_object_max_name("},{"line_number":1278,"context_line":"                    params[\u0027marker\u0027])"},{"line_number":1279,"context_line":"            elif params[\u0027version_marker\u0027] \u003d\u003d \u0027null\u0027:"},{"line_number":1280,"context_line":"                params[\u0027marker\u0027] \u003d get_reserved_name(params[\u0027marker\u0027], \u0027\u0027)"},{"line_number":1281,"context_line":"            else:"},{"line_number":1282,"context_line":"                try:"},{"line_number":1283,"context_line":"                    version \u003d params.pop(\u0027version_marker\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"6a496ae9_ee971645","line":1280,"range":{"start_line":1280,"start_character":35,"end_line":1280,"end_character":74},"in_reply_to":"5e5a81e9_f29b9184","updated":"2026-04-24 13:47:25.000000000","message":"Done.\n\nAlso, build_versions_object_max_name seems a bit unnecessary given build_versions_object_prefix","commit_id":"9ea555881df01fd0e6d8e22cb758d314318693c1"}],"test/unit/container/test_backend.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"600fdc5a2561a6e5d380499be7eab779233572e4","unresolved":true,"context_lines":[{"line_number":969,"context_line":"                                 account\u003d\u0027test_account\u0027,"},{"line_number":970,"context_line":"                                 container\u003d\u0027test_container\u0027)"},{"line_number":971,"context_line":"        # create it"},{"line_number":972,"context_line":"        with mock_normal_timestamp_now(self.ts()):"},{"line_number":973,"context_line":"            broker.initialize(start.internal, POLICIES.default.idx)"},{"line_number":974,"context_line":"        info, is_deleted \u003d broker.get_info_is_deleted()"},{"line_number":975,"context_line":"        self.assertEqual(is_deleted, broker.is_deleted())"}],"source_content_type":"text/x-python","patch_set":2,"id":"f3902a3d_5d0ee4d2","line":972,"updated":"2026-04-24 13:47:25.000000000","message":"I think this and the change at line 6409 are revealing a bug in the test setup on master: The container info table is now created using a NormalTimestamp, so that is what should be mocked, which requires that the historic versions of _create_container_info_table *all* ought to change like line 6409.\n\nIt was benign on master because the self.ts() and self.normal_ts() iters would start at the same int time, so the first call to either yielded the same time (just different types). But in the TestContainerBrokerBeforeSystags class added on feature/mpu, self.ts() is used in the setUp which puts it out of step with self.normal_ts().\n\nFix on master https://review.opendev.org/c/openstack/swift/+/986089","commit_id":"9ea555881df01fd0e6d8e22cb758d314318693c1"}]}
