)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"125523b4b6ca918bd709806ec6f2e3779af3c07b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"c03c7200_0c4e17fd","updated":"2024-12-18 17:12:31.000000000","message":"@Clay thanks for taking a look\n\nI\u0027m not really trying to make a strong argument about how many potential versions of the same-timestamped-data-file is better *in terms of a user\u0027s perspective*. But having only one version of the same-timestamped-data-file IS better in terms of internal services (e.g. an mpu auditor) being able to deterministically locate that version. \n\nGiven that, I\u0027d reframe the question to \"is it better to have potentially multiple versions of the same-timestamped-data-file?\" - if it is then we\u0027ll need to cook up a different plan on feature/mpu. \n\nFWIW, IF the multiple versions of same data file had a sensible semantic (i.e. if the different metadata represented the ordering client PUTs and POSTs in a sensible way) then I could see an argument that says \"it\u0027s better for users to see updated versions of the same data file\". But as the probe test illustrates, the metadata applied to multiple versions of the same data file does NOT sensibly reflect the order of client POSTs. Furthermore, there is only ever ONE version of a data file that is PUT while versioning is enabled.\n\nSo I can\u0027t convince myself that having multiple versions of the same data file IS helpful to users (but my opinion IS biased by my desire for a solution on feature/mpu!)","commit_id":"e2e3360f7dc748cbbcf084a8dd84f555652e4f91"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b99e6483cad9abb6486bbf219f21967c18eaa372","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"e6ddc5f3_860207ef","updated":"2024-12-18 23:56:06.000000000","message":"I\u0027d like to spend a little more time with the probe test, but this seems perfectly reasonable.","commit_id":"e2e3360f7dc748cbbcf084a8dd84f555652e4f91"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"6ea49138370ca9ba7f1a858232e53daf23daa179","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"183ee7b7_25e4e07c","updated":"2024-12-18 14:08:01.000000000","message":"This change is a requirement to fix an issue on feature/mpu i.e. given a data timestamp it should be possible to infer the name of any versions that was created of that data file.","commit_id":"e2e3360f7dc748cbbcf084a8dd84f555652e4f91"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"edb5b166b9f8d4f1a75fd996848b6f4cdc731c38","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"94c4d81f_3a0f813a","updated":"2024-12-18 15:41:26.000000000","message":"it\u0027s not *immeidately* obvious to me why having one less object in the versions container is strictly always better.  It\u0027s unfortunate that we\u0027re loosing `(V2, meta2a)` but I don\u0027t see how that\u0027s avoidable given the availability of `V2` when `meta2a` is added.  Loosing `(V1, meta1)` isn\u0027t obviously a significant win?\n\nMaybe I\u0027m misunderstading... I didn\u0027t runt his probe test on master yet cause I have something else checked out ATM.","commit_id":"e2e3360f7dc748cbbcf084a8dd84f555652e4f91"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"8e911a6089cc43179da191b4b61a6653da98c0ae","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"33998c46_3df23487","in_reply_to":"e6ddc5f3_860207ef","updated":"2025-01-02 23:28:21.000000000","message":"\u003e seems perfectly reasonable\n\nI think I could get behind \"the version-id of an object is known and fixed at the time when the .data is created EVEN IF UNVERSIONED\" - but I think we should consider how to make that change to versioning on-purpose and if it should apply to all current unversioned objects retroactively.\n\ne.g. what if the version we\u0027re copying from had a .meta update?  We\u0027re creating a version-id and setting it to represent the data/metadata \"as we found it\" at the time we perform the null-version-COPY.  Maybe \"which timestamp should we use as the version-id\" is all \"how many angles can dance on the head of a pin\" since AWS version-ids are opaque anyway, and we just \"happen\" to use inverted timestamps in versioned object names because they sort the way we wanted to make listings efficient.\n\nIf we can accept \"well unversioned POST requests to versioned containers write the metadata to the wrong version sometimes\" maybe we can accept \"well null-version-COPY was always best effort anyway; if you trigger it wrong and loose your unversioned metadata: so be it\"\n\nHonestly the whole null-version-COPY strategy is always a tragedy of availability regardless, there\u0027s plenty of other scenarios where you could loose different data/metadata.  Maybe I\u0027m talking myself into \"this is fine\"","commit_id":"e2e3360f7dc748cbbcf084a8dd84f555652e4f91"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"8e911a6089cc43179da191b4b61a6653da98c0ae","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"015f492d_1cd96d38","updated":"2025-01-02 23:28:21.000000000","message":"I think the most prudent thing to do is merge this change to versioning into feature-mpu and sort it out once we have more of native-mpus worked out and we\u0027re preparing the native-mpu patch chain to merge.  It\u0027s good that we\u0027re reloading versioning\u0027s wonkyness into our heads sooner than later.\n\nI think it\u0027s totally possible as we start settling into feature-mpu some more we realize their interaction with object versioning will require some updates in versioning - perhaps even JUST THIS ONE MINOR change specifically - but I\u0027m cautious of trying to get it merged to master before we\u0027ve settled on all the native-mpu requirements for object versioning.\n\nBut I could be wrong; maybe this is only/always/strictly better for master and the fact feature-mpu would benefit from getting it (or other versioning related drive-by improvements) out of the way and on master ASAP is nothing but win.","commit_id":"85d6f8a984dd0d7f075318a9d42f6d78e6bcc337"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"1077cd339c6362a501e9b386724f3ba89278501c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"b067ee78_b706437b","updated":"2025-01-03 16:14:15.000000000","message":"I\u0027m still happy with waiting to get this w/ feature-mpu just because I wonder if there might be yet other improvements/tests that will be needed to versioning by the time native-mpu is done which will make even more sense when bundled together.\n\nBut I\u0027m coming around to the idea this is a useful improvement on it\u0027s own.\n\nSorry to waffle; I guess since we don\u0027t really have that many users of versioning it feels like a distraction that\u0027s reasonable to keep grouped with feature-mpu for now?","commit_id":"85d6f8a984dd0d7f075318a9d42f6d78e6bcc337"}],"swift/common/middleware/versioned_writes/object_versioning.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"709bffc31b1cdac79639fc4c3139e5aa2a394b51","unresolved":true,"context_lines":[{"line_number":455,"context_line":"                    get_resp.headers[\u0027last-modified\u0027],"},{"line_number":456,"context_line":"                    \u0027%a, %d %b %Y %H:%M:%S GMT\u0027))))"},{"line_number":457,"context_line":"        # drop any offset from ts_source to keep ~Timestamp happy"},{"line_number":458,"context_line":"        # TODO: or, we could fix ~Timestamp to support offsets"},{"line_number":459,"context_line":"        ts_source \u003d Timestamp(ts_source).normal"},{"line_number":460,"context_line":"        vers_obj_name \u003d self._build_versions_object_name("},{"line_number":461,"context_line":"            object_name, ts_source)"}],"source_content_type":"text/x-python","patch_set":1,"id":"f0570dbb_3df3332b","line":458,"updated":"2024-11-26 18:39:52.000000000","message":"see https://review.opendev.org/c/openstack/swift/+/936196","commit_id":"f8aca839e04eb64ba4985275ff75c217da011f57"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"125523b4b6ca918bd709806ec6f2e3779af3c07b","unresolved":false,"context_lines":[{"line_number":455,"context_line":"                    get_resp.headers[\u0027last-modified\u0027],"},{"line_number":456,"context_line":"                    \u0027%a, %d %b %Y %H:%M:%S GMT\u0027))))"},{"line_number":457,"context_line":"        # drop any offset from ts_source to keep ~Timestamp happy"},{"line_number":458,"context_line":"        # TODO: or, we could fix ~Timestamp to support offsets"},{"line_number":459,"context_line":"        ts_source \u003d Timestamp(ts_source).normal"},{"line_number":460,"context_line":"        vers_obj_name \u003d self._build_versions_object_name("},{"line_number":461,"context_line":"            object_name, ts_source)"}],"source_content_type":"text/x-python","patch_set":1,"id":"bbe93454_53ca69e7","line":458,"in_reply_to":"f0570dbb_3df3332b","updated":"2024-12-18 17:12:31.000000000","message":"Acknowledged","commit_id":"f8aca839e04eb64ba4985275ff75c217da011f57"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"edb5b166b9f8d4f1a75fd996848b6f4cdc731c38","unresolved":true,"context_lines":[{"line_number":236,"context_line":"        return get_reserved_name(\u0027versions\u0027, container_name)"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"    def _build_versions_object_name(self, object_name, ts):"},{"line_number":239,"context_line":"        inv \u003d ~Timestamp(Timestamp(ts).normal)"},{"line_number":240,"context_line":"        return get_reserved_name(object_name, inv.internal)"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"    def _split_version_from_name(self, versioned_name):"}],"source_content_type":"text/x-python","patch_set":4,"id":"353d8bef_b4384fa0","line":239,"updated":"2024-12-18 15:41:26.000000000","message":"should we keep the comment here to remind us to fix this when ~Timestamp works with offsets?","commit_id":"e2e3360f7dc748cbbcf084a8dd84f555652e4f91"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"22091139c84dbbbb6bf4446e2117aacac4f2ec34","unresolved":true,"context_lines":[{"line_number":236,"context_line":"        return get_reserved_name(\u0027versions\u0027, container_name)"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"    def _build_versions_object_name(self, object_name, ts):"},{"line_number":239,"context_line":"        inv \u003d ~Timestamp(Timestamp(ts).normal)"},{"line_number":240,"context_line":"        return get_reserved_name(object_name, inv.internal)"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"    def _split_version_from_name(self, versioned_name):"}],"source_content_type":"text/x-python","patch_set":4,"id":"74315bbb_e612382b","line":239,"in_reply_to":"297a6c40_c601d20e","updated":"2024-12-19 10:19:17.000000000","message":"I don\u0027t think the offset should ever contribute to the naming of the version. Offsets are used internally to (re)write the same data, so there\u0027s no need to differentiate versions based on offsets. \n\nIf we do include offsets in version names, clients would start to see anomalous version-ids, not that clients should care about the content of a version-id. But worse, including offsets in version names would defeat the purpose of this change i.e. exactly one version for the same data file. I\u0027m thinking of a version of an object in the wrong policy being created before the reconciler bumps its timestamp offset, and then it gets versioned again in a manner similar to the probe test.\n\nI\u0027m not sure it is necessary for us to decide in this patch that ``~Timestamp`` should quietly ignore offsets forever. There may be some future application that wants offsets supported?\n\nFWIW I did take a stab at making ``~Timestamp`` support offsets https://review.opendev.org/c/openstack/swift/+/936196 before deciding it wasn\u0027t necessary or desirable.","commit_id":"e2e3360f7dc748cbbcf084a8dd84f555652e4f91"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"125523b4b6ca918bd709806ec6f2e3779af3c07b","unresolved":true,"context_lines":[{"line_number":236,"context_line":"        return get_reserved_name(\u0027versions\u0027, container_name)"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"    def _build_versions_object_name(self, object_name, ts):"},{"line_number":239,"context_line":"        inv \u003d ~Timestamp(Timestamp(ts).normal)"},{"line_number":240,"context_line":"        return get_reserved_name(object_name, inv.internal)"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"    def _split_version_from_name(self, versioned_name):"}],"source_content_type":"text/x-python","patch_set":4,"id":"bc70316d_5ee3dd49","line":239,"in_reply_to":"353d8bef_b4384fa0","updated":"2024-12-18 17:12:31.000000000","message":"absolutely, the comment has mistakenly moved to the follow on refactor. will fix.","commit_id":"e2e3360f7dc748cbbcf084a8dd84f555652e4f91"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"8e911a6089cc43179da191b4b61a6653da98c0ae","unresolved":false,"context_lines":[{"line_number":236,"context_line":"        return get_reserved_name(\u0027versions\u0027, container_name)"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"    def _build_versions_object_name(self, object_name, ts):"},{"line_number":239,"context_line":"        inv \u003d ~Timestamp(Timestamp(ts).normal)"},{"line_number":240,"context_line":"        return get_reserved_name(object_name, inv.internal)"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"    def _split_version_from_name(self, versioned_name):"}],"source_content_type":"text/x-python","patch_set":4,"id":"14fc59da_d7dc0ad0","line":239,"in_reply_to":"74315bbb_e612382b","updated":"2025-01-02 23:28:21.000000000","message":"sorry, i think i read the diff wrong; this comment in the current version looks great.","commit_id":"e2e3360f7dc748cbbcf084a8dd84f555652e4f91"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b99e6483cad9abb6486bbf219f21967c18eaa372","unresolved":true,"context_lines":[{"line_number":236,"context_line":"        return get_reserved_name(\u0027versions\u0027, container_name)"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"    def _build_versions_object_name(self, object_name, ts):"},{"line_number":239,"context_line":"        inv \u003d ~Timestamp(Timestamp(ts).normal)"},{"line_number":240,"context_line":"        return get_reserved_name(object_name, inv.internal)"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"    def _split_version_from_name(self, versioned_name):"}],"source_content_type":"text/x-python","patch_set":4,"id":"297a6c40_c601d20e","line":239,"in_reply_to":"bc70316d_5ee3dd49","updated":"2024-12-18 23:56:06.000000000","message":"I\u0027m not actually sure we *want* `~Timestamp` to work with offsets -- or, if we *do*, I\u0027m not convinced that `~Timestamp.now(offset\u003d1)` should give you anything other than `~Timestamp.now()`. Maybe we should just have `__inv__` ignore offsets, do that as part of this change (or if you\u0027re worried about conflating concerns, as a pre-req), and drop this part of the diff?","commit_id":"e2e3360f7dc748cbbcf084a8dd84f555652e4f91"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"8e911a6089cc43179da191b4b61a6653da98c0ae","unresolved":true,"context_lines":[{"line_number":448,"context_line":"        # if there\u0027s an existing object, then copy it to"},{"line_number":449,"context_line":"        # X-Versions-Location"},{"line_number":450,"context_line":"        ts_source \u003d get_resp.headers.get("},{"line_number":451,"context_line":"            \u0027x-timestamp\u0027,"},{"line_number":452,"context_line":"            calendar.timegm(time.strptime("},{"line_number":453,"context_line":"                get_resp.headers[\u0027last-modified\u0027],"},{"line_number":454,"context_line":"                \u0027%a, %d %b %Y %H:%M:%S GMT\u0027)))"}],"source_content_type":"text/x-python","patch_set":5,"id":"58d65acf_f41fbcd2","side":"PARENT","line":451,"updated":"2025-01-02 23:28:21.000000000","message":"I take it `x-timestamp` (unlike some `x-backend-[data-]timestamp`) just never has an offset and so was safe to use as `~Timestamp(ts)`.","commit_id":"0b534d58465f9fdad9e6bedb96c9ecc77107b074"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"1077cd339c6362a501e9b386724f3ba89278501c","unresolved":false,"context_lines":[{"line_number":448,"context_line":"        # if there\u0027s an existing object, then copy it to"},{"line_number":449,"context_line":"        # X-Versions-Location"},{"line_number":450,"context_line":"        ts_source \u003d get_resp.headers.get("},{"line_number":451,"context_line":"            \u0027x-timestamp\u0027,"},{"line_number":452,"context_line":"            calendar.timegm(time.strptime("},{"line_number":453,"context_line":"                get_resp.headers[\u0027last-modified\u0027],"},{"line_number":454,"context_line":"                \u0027%a, %d %b %Y %H:%M:%S GMT\u0027)))"}],"source_content_type":"text/x-python","patch_set":5,"id":"20816e77_657aefc5","side":"PARENT","line":451,"in_reply_to":"2841fdaa_1aea4557","updated":"2025-01-03 16:14:15.000000000","message":"Acknowledged","commit_id":"0b534d58465f9fdad9e6bedb96c9ecc77107b074"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"939457822e89fcf3eee1273cb7de229042fb533a","unresolved":true,"context_lines":[{"line_number":448,"context_line":"        # if there\u0027s an existing object, then copy it to"},{"line_number":449,"context_line":"        # X-Versions-Location"},{"line_number":450,"context_line":"        ts_source \u003d get_resp.headers.get("},{"line_number":451,"context_line":"            \u0027x-timestamp\u0027,"},{"line_number":452,"context_line":"            calendar.timegm(time.strptime("},{"line_number":453,"context_line":"                get_resp.headers[\u0027last-modified\u0027],"},{"line_number":454,"context_line":"                \u0027%a, %d %b %Y %H:%M:%S GMT\u0027)))"}],"source_content_type":"text/x-python","patch_set":5,"id":"2841fdaa_1aea4557","side":"PARENT","line":451,"in_reply_to":"58d65acf_f41fbcd2","updated":"2025-01-03 10:29:24.000000000","message":"x-timestamp returned from object servers has the \u0027normal\u0027 format i.e. no offset","commit_id":"0b534d58465f9fdad9e6bedb96c9ecc77107b074"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"8e911a6089cc43179da191b4b61a6653da98c0ae","unresolved":true,"context_lines":[{"line_number":454,"context_line":"        ts_source \u003d get_resp.headers.get("},{"line_number":455,"context_line":"            \u0027x-backend-data-timestamp\u0027,"},{"line_number":456,"context_line":"            get_resp.headers.get("},{"line_number":457,"context_line":"                \u0027x-timestamp\u0027,"},{"line_number":458,"context_line":"                calendar.timegm(time.strptime("},{"line_number":459,"context_line":"                    get_resp.headers[\u0027last-modified\u0027],"},{"line_number":460,"context_line":"                    \u0027%a, %d %b %Y %H:%M:%S GMT\u0027))))"}],"source_content_type":"text/x-python","patch_set":5,"id":"c9aa1d30_ab40b8a1","line":457,"updated":"2025-01-02 23:28:21.000000000","message":"in what scenarios is the `x-backend-data-timestamp` header missing from the response and is it always consistent to fallback to `x-timestamp` instead?","commit_id":"85d6f8a984dd0d7f075318a9d42f6d78e6bcc337"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"939457822e89fcf3eee1273cb7de229042fb533a","unresolved":true,"context_lines":[{"line_number":454,"context_line":"        ts_source \u003d get_resp.headers.get("},{"line_number":455,"context_line":"            \u0027x-backend-data-timestamp\u0027,"},{"line_number":456,"context_line":"            get_resp.headers.get("},{"line_number":457,"context_line":"                \u0027x-timestamp\u0027,"},{"line_number":458,"context_line":"                calendar.timegm(time.strptime("},{"line_number":459,"context_line":"                    get_resp.headers[\u0027last-modified\u0027],"},{"line_number":460,"context_line":"                    \u0027%a, %d %b %Y %H:%M:%S GMT\u0027))))"}],"source_content_type":"text/x-python","patch_set":5,"id":"82b9efb9_41f7f801","line":457,"in_reply_to":"c9aa1d30_ab40b8a1","updated":"2025-01-03 10:29:24.000000000","message":"heh, I wondered similarly about when we\u0027d ever fall back to last-modified.\n\nOld object servers didn\u0027t return x-backend-data-timestamp. x-backend-data-timestamp was added after EC policies https://review.opendev.org/c/openstack/swift/+/215276\n\nIn other places where we have a similar fallback [1] we\u0027d use x-backend-timestamp which has the internal format but I guess here the intent was to only ever consider the normal part of the timestamp.\n\n[1] e.g. https://github.com/openstack/swift/blob/f88efdb4df37577c687f68b2cc3712465955c20e/swift/obj/reconstructor.py#L443","commit_id":"85d6f8a984dd0d7f075318a9d42f6d78e6bcc337"}],"test/probe/brain.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b99e6483cad9abb6486bbf219f21967c18eaa372","unresolved":true,"context_lines":[{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    def post_object(self, container_name, object_name, headers):"},{"line_number":220,"context_line":"        return client.post_object("},{"line_number":221,"context_line":"            self.url, self.token, container_name, object_name, headers\u003dheaders)"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"    def delete_object(self, container_name, object_name):"},{"line_number":224,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"697d826c_e7d8efa9","line":221,"updated":"2024-12-18 23:56:06.000000000","message":"Should we update other brain-clients, too? https://review.opendev.org/c/openstack/swift/+/930779","commit_id":"e2e3360f7dc748cbbcf084a8dd84f555652e4f91"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"22091139c84dbbbb6bf4446e2117aacac4f2ec34","unresolved":false,"context_lines":[{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    def post_object(self, container_name, object_name, headers):"},{"line_number":220,"context_line":"        return client.post_object("},{"line_number":221,"context_line":"            self.url, self.token, container_name, object_name, headers\u003dheaders)"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"    def delete_object(self, container_name, object_name):"},{"line_number":224,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"902a4253_5a6e75c8","line":221,"in_reply_to":"697d826c_e7d8efa9","updated":"2024-12-19 10:19:17.000000000","message":"Done","commit_id":"e2e3360f7dc748cbbcf084a8dd84f555652e4f91"}],"test/probe/test_object_versioning.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"edb5b166b9f8d4f1a75fd996848b6f4cdc731c38","unresolved":true,"context_lines":[{"line_number":278,"context_line":"             # is PUT, but it didn\u0027t land on a symlink node so it isn\u0027t"},{"line_number":279,"context_line":"             # forwarded to the V2 version in the versions container. Oops."},{"line_number":280,"context_line":"             # After replication, the symlinks will have a .meta file, but the"},{"line_number":281,"context_line":"             # symlink resp headers are ignored when object_versioning resolves"},{"line_number":282,"context_line":"             # the version in the versions container, so we get the stale meta2"},{"line_number":283,"context_line":"             # metadata :( ..."},{"line_number":284,"context_line":"             (b\u0027V2\u0027, \u0027meta2\u0027),"}],"source_content_type":"text/x-python","patch_set":4,"id":"fecf4ac9_1f46a0fb","line":281,"updated":"2024-12-18 15:41:26.000000000","message":"\u003e are ignored\n\nI think we considered doing a \"merge\" of symlink and target metadata at some point and decided it\u0027s most correct to write to both (in-case a newer .data shows up?) but ignore it in resp (because we don\u0027t have any way to merge deleted metadata?)","commit_id":"e2e3360f7dc748cbbcf084a8dd84f555652e4f91"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"22091139c84dbbbb6bf4446e2117aacac4f2ec34","unresolved":false,"context_lines":[{"line_number":278,"context_line":"             # is PUT, but it didn\u0027t land on a symlink node so it isn\u0027t"},{"line_number":279,"context_line":"             # forwarded to the V2 version in the versions container. Oops."},{"line_number":280,"context_line":"             # After replication, the symlinks will have a .meta file, but the"},{"line_number":281,"context_line":"             # symlink resp headers are ignored when object_versioning resolves"},{"line_number":282,"context_line":"             # the version in the versions container, so we get the stale meta2"},{"line_number":283,"context_line":"             # metadata :( ..."},{"line_number":284,"context_line":"             (b\u0027V2\u0027, \u0027meta2\u0027),"}],"source_content_type":"text/x-python","patch_set":4,"id":"0205d323_e798b63a","line":281,"in_reply_to":"fecf4ac9_1f46a0fb","updated":"2024-12-19 10:19:17.000000000","message":"Acknowledged","commit_id":"e2e3360f7dc748cbbcf084a8dd84f555652e4f91"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"edb5b166b9f8d4f1a75fd996848b6f4cdc731c38","unresolved":true,"context_lines":[{"line_number":281,"context_line":"             # symlink resp headers are ignored when object_versioning resolves"},{"line_number":282,"context_line":"             # the version in the versions container, so we get the stale meta2"},{"line_number":283,"context_line":"             # metadata :( ..."},{"line_number":284,"context_line":"             (b\u0027V2\u0027, \u0027meta2\u0027),"},{"line_number":285,"context_line":"             # meta2a was POSTed *after* V2 was PUT but end up being copied"},{"line_number":286,"context_line":"             # into the .data version file for V1 when the stale replica was"},{"line_number":287,"context_line":"             # copied over the original V1 version!!"}],"source_content_type":"text/x-python","patch_set":4,"id":"75208c47_dc701285","line":284,"updated":"2024-12-18 15:41:26.000000000","message":"it\u0027s not possible to have a `(V2, meta2a)` object unless the user explicitly did the POST to the `version_id\u003dV2` - because the symlink pointing to V2 being the current object wasn\u0027t available.","commit_id":"e2e3360f7dc748cbbcf084a8dd84f555652e4f91"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"8e911a6089cc43179da191b4b61a6653da98c0ae","unresolved":false,"context_lines":[{"line_number":281,"context_line":"             # symlink resp headers are ignored when object_versioning resolves"},{"line_number":282,"context_line":"             # the version in the versions container, so we get the stale meta2"},{"line_number":283,"context_line":"             # metadata :( ..."},{"line_number":284,"context_line":"             (b\u0027V2\u0027, \u0027meta2\u0027),"},{"line_number":285,"context_line":"             # meta2a was POSTed *after* V2 was PUT but end up being copied"},{"line_number":286,"context_line":"             # into the .data version file for V1 when the stale replica was"},{"line_number":287,"context_line":"             # copied over the original V1 version!!"}],"source_content_type":"text/x-python","patch_set":4,"id":"1cfb40fd_bf14190d","line":284,"in_reply_to":"66884fbe_d2752545","updated":"2025-01-02 23:28:21.000000000","message":"yes, we agree.","commit_id":"e2e3360f7dc748cbbcf084a8dd84f555652e4f91"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"125523b4b6ca918bd709806ec6f2e3779af3c07b","unresolved":true,"context_lines":[{"line_number":281,"context_line":"             # symlink resp headers are ignored when object_versioning resolves"},{"line_number":282,"context_line":"             # the version in the versions container, so we get the stale meta2"},{"line_number":283,"context_line":"             # metadata :( ..."},{"line_number":284,"context_line":"             (b\u0027V2\u0027, \u0027meta2\u0027),"},{"line_number":285,"context_line":"             # meta2a was POSTed *after* V2 was PUT but end up being copied"},{"line_number":286,"context_line":"             # into the .data version file for V1 when the stale replica was"},{"line_number":287,"context_line":"             # copied over the original V1 version!!"}],"source_content_type":"text/x-python","patch_set":4,"id":"66884fbe_d2752545","line":284,"in_reply_to":"75208c47_dc701285","updated":"2024-12-18 17:12:31.000000000","message":"on a healthy system we could get a version ``(V2, meta2a)`` without the user needing to specify the ``version_id``, because the meta2a POST would be redirected to the version via the symlink. But the scenario here is that the replicas with a symlink to V2 are down.\n\nMaybe we\u0027re agreeing? 😊","commit_id":"e2e3360f7dc748cbbcf084a8dd84f555652e4f91"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"edb5b166b9f8d4f1a75fd996848b6f4cdc731c38","unresolved":true,"context_lines":[{"line_number":285,"context_line":"             # meta2a was POSTed *after* V2 was PUT but end up being copied"},{"line_number":286,"context_line":"             # into the .data version file for V1 when the stale replica was"},{"line_number":287,"context_line":"             # copied over the original V1 version!!"},{"line_number":288,"context_line":"             (b\u0027V1\u0027, \u0027meta2a\u0027)],"},{"line_number":289,"context_line":"            [(body, hdrs[\u0027x-object-meta-test\u0027])"},{"line_number":290,"context_line":"             for (hdrs, body) in versions])"},{"line_number":291,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3d00b180_ca4c921b","line":288,"updated":"2024-12-18 15:41:26.000000000","message":"so the only difference with master is we get a `(V1, meta1)` object *as well*?","commit_id":"e2e3360f7dc748cbbcf084a8dd84f555652e4f91"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"125523b4b6ca918bd709806ec6f2e3779af3c07b","unresolved":true,"context_lines":[{"line_number":285,"context_line":"             # meta2a was POSTed *after* V2 was PUT but end up being copied"},{"line_number":286,"context_line":"             # into the .data version file for V1 when the stale replica was"},{"line_number":287,"context_line":"             # copied over the original V1 version!!"},{"line_number":288,"context_line":"             (b\u0027V1\u0027, \u0027meta2a\u0027)],"},{"line_number":289,"context_line":"            [(body, hdrs[\u0027x-object-meta-test\u0027])"},{"line_number":290,"context_line":"             for (hdrs, body) in versions])"},{"line_number":291,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"b06c44a9_9c8f74cd","line":288,"in_reply_to":"3d00b180_ca4c921b","updated":"2024-12-18 17:12:31.000000000","message":"yes on master you\u0027d see another version and also the ordering is different so that the second version of V1 appears to be newer than V2, because on master it is named by the meta2a timestamp:\n```\nE       AssertionError: Lists differ: [(b\u0027V3\u0027, \u0027meta3\u0027), (b\u0027V2\u0027, \u0027meta2\u0027), (b\u0027V1\u0027, \u0027meta2a\u0027)] !\u003d [(b\u0027V3\u0027, \u0027meta3\u0027), (b\u0027V1\u0027, \u0027meta2a\u0027), (b\u0027V2\u0027, \u0027meta2\u0027), (b\u0027V1\u0027, \u0027meta1\u0027)]\nE\nE       First differing element 1:\nE       (b\u0027V2\u0027, \u0027meta2\u0027)\nE       (b\u0027V1\u0027, \u0027meta2a\u0027)\nE\nE       Second list contains 1 additional elements.\nE       First extra element 3:\nE       (b\u0027V1\u0027, \u0027meta1\u0027)\nE\nE       - [(b\u0027V3\u0027, \u0027meta3\u0027), (b\u0027V2\u0027, \u0027meta2\u0027), (b\u0027V1\u0027, \u0027meta2a\u0027)]\nE       ?                                                   ^^\nE\nE       + [(b\u0027V3\u0027, \u0027meta3\u0027), (b\u0027V1\u0027, \u0027meta2a\u0027), (b\u0027V2\u0027, \u0027meta2\u0027), (b\u0027V1\u0027, \u0027meta1\u0027)]\nE       ?                    +++++++++++++++++++                               ^\n```","commit_id":"e2e3360f7dc748cbbcf084a8dd84f555652e4f91"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"1077cd339c6362a501e9b386724f3ba89278501c","unresolved":true,"context_lines":[{"line_number":285,"context_line":"             # meta2a was POSTed *after* V2 was PUT but end up being copied"},{"line_number":286,"context_line":"             # into the .data version file for V1 when the stale replica was"},{"line_number":287,"context_line":"             # copied over the original V1 version!!"},{"line_number":288,"context_line":"             (b\u0027V1\u0027, \u0027meta2a\u0027)],"},{"line_number":289,"context_line":"            [(body, hdrs[\u0027x-object-meta-test\u0027])"},{"line_number":290,"context_line":"             for (hdrs, body) in versions])"},{"line_number":291,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"cfc96b2d_987c4b15","line":288,"in_reply_to":"5c755c12_4828760b","updated":"2025-01-03 16:14:15.000000000","message":"1) right, different datafiles have different timestamps (unless they both received the same POST!?)\n2) having only one copy of the same user-namespace data does seem like an improvement of the \"consistent brokeness\" which has got to be better than the \"*inconsistent* brokenness\" of \"a POST *might* result in the metadata of the existing version being replaced with metadata intended for a later version\"\n\nI agree the loss of `(V1, meta)` might be just \"how unversioned POST works\" - so maybe making it more consistent regardless of null-version-COPY is mostly just win.","commit_id":"e2e3360f7dc748cbbcf084a8dd84f555652e4f91"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"939457822e89fcf3eee1273cb7de229042fb533a","unresolved":true,"context_lines":[{"line_number":285,"context_line":"             # meta2a was POSTed *after* V2 was PUT but end up being copied"},{"line_number":286,"context_line":"             # into the .data version file for V1 when the stale replica was"},{"line_number":287,"context_line":"             # copied over the original V1 version!!"},{"line_number":288,"context_line":"             (b\u0027V1\u0027, \u0027meta2a\u0027)],"},{"line_number":289,"context_line":"            [(body, hdrs[\u0027x-object-meta-test\u0027])"},{"line_number":290,"context_line":"             for (hdrs, body) in versions])"},{"line_number":291,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5c755c12_4828760b","line":288,"in_reply_to":"77f7fc9e_6b363fb1","updated":"2025-01-03 10:29:24.000000000","message":"\u003e Maybe this change only ever overwrites metadata - which was never really supposed to be versioned anyway?\n\nCorrect, this change only causes metadata to be overwritten on existing versions. I don\u0027t have the historical context to know about \"which was never really supposed to be versioned anyway\", but metadata IS persisted with versions, albeit inconsistently [2].\n\n\u003e Is there any scenario where \"using the data-timestamp is an obvious win for more accurate consistency under failure\"?\n\nI don\u0027t think this patch offers any win for *data/metadata consistency* because I think POST consistency is broken with versioning regardless of how we name the versions, and regardless of whether it\u0027s a copied-version or a put-version.\n\nThe \"win\" is that the name of the copied-version becomes predictable given the created-at time.\n\nThere is arguably an improvement in terms of \"behavioural consistency\" (or \"consistent brokeness\" perhaps !!), as the following statements illustrate:\n\nFor versions created by copying or directly PUT: \n\n* With or without this change we\u0027d never overwrite a version with different *data*. [1] \n\nFor versions created by copying the previously unversioned object, WITHOUT this change:\n\n* a POST might result in a new version of the same data with new metadata intended for a later version [2].\n\n* a POST might result in the metadata of the existing version being replaced with metadata intended for a later version [3]\n\nFor versions created by copying the previously unversioned object, WITH this change:\n\n* a POST never results in a new version of the same data.\n\n* a POST might result in the metadata of the existing version being replaced with metadata intended for a later version [3].\n\nFor versions created by a PUT while versioning is enabled, with or without this change:\n\n* a POST never results in a new version of the same data.\n\n* a POST might result in the metadata of an existing version being replaced with metadata intended for a later version [3].\n\nSo with this change, the *behaviour* of POSTs w.r.t. copied-versions and put-versions is more consistent. But the data/metadata consistency problem remains:\n\nWithout this patch: if you POST to a versioned object the POST should be applied to the current version, but in failure conditions may be applied to an older version, or may result in another version with same data as an older version.\n\nWith this patch: if you POST to a versioned object the POST should be applied to the current version, but in failure conditions may be applied to an older version.\n\n---\n\n[1] If there were replicas of the same data file having different content in the user namespace then it\u0027s possible that one replica might be copied to the versions container, and a different replica later copied to the same version name. But there shouldn\u0027t ever be different replicas of the same data file having different content, and if there are then it\u0027s not a problem that\u0027s due to versioning middleware.\n\n[2] This is the scenario of the probe test in this patch: a stale replica of the previously unversioned object is left in the user namespace, it has a POST applied to it, and is later copied to the versions container again with a new name.\n\n[3] POSTs are redirected to the version according to the symlink POST response, and the symlink might be a stale replica pointing to an older version. https://github.com/openstack/swift/blob/c51e81f640274db31bf45b5253d8e99d19a7e3d6/swift/common/middleware/versioned_writes/object_versioning.py#L566-L600","commit_id":"e2e3360f7dc748cbbcf084a8dd84f555652e4f91"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"8e911a6089cc43179da191b4b61a6653da98c0ae","unresolved":true,"context_lines":[{"line_number":285,"context_line":"             # meta2a was POSTed *after* V2 was PUT but end up being copied"},{"line_number":286,"context_line":"             # into the .data version file for V1 when the stale replica was"},{"line_number":287,"context_line":"             # copied over the original V1 version!!"},{"line_number":288,"context_line":"             (b\u0027V1\u0027, \u0027meta2a\u0027)],"},{"line_number":289,"context_line":"            [(body, hdrs[\u0027x-object-meta-test\u0027])"},{"line_number":290,"context_line":"             for (hdrs, body) in versions])"},{"line_number":291,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"77f7fc9e_6b363fb1","line":288,"in_reply_to":"b06c44a9_9c8f74cd","updated":"2025-01-02 23:28:21.000000000","message":"so we agree that in principle that `(V1, meta2a)` object should never have existed if somehow the V2 symlink had been available at the time of the POST of meta2a.  But, it\u0027s un-avoidable in the current implementation of versioning.  And neither timestamp choice during the inconsistnet read during the null-version-COPY eventually \"resolves\" to `(V2, meta2a)` existing anywhere (assuming that was what was intended).\n\nBut this change also throws out `(V1, meta1)` which is the only consistent data/meta that existed before versioning was turned on; however perhaps it\u0027s reasonable that it overwrites it with `(V1, meta2a)` in its place since unversioned POST requests to version-enabled containers always have the potential to write the metadata to an \"old\" version if the symlink is stale, even when a null-version-COPY isn\u0027t involved.\n\nWhen the null-version-COPY needs to happen, AND the null-version is an MPU manifest - it makes sense to me that the version-id of the COPY\u0027d manifest should be deterministic in such a way that the segments still know the version-id of the object they belong too.  I understand the motivation for this change.\n\nWhat I\u0027m less sure about is if we should change master to agree \"the version we make-up at null-version-COPY should be the data timestamp\" because we\u0027re experimenting with doing something like that on feature-mpu.\n\nIs there any scenario where \"using the data-timestamp is an obvious win for more accurate consistency under failure\"?  It seems like it only ever has the potential to \"overwrite\" a previously copied version with a newer version *after* the user has turned on \"don\u0027t throw away data; only create new versions\"?  Maybe this change only ever overwrites *metadata* - which was never really supposed to be versioned anyway?","commit_id":"e2e3360f7dc748cbbcf084a8dd84f555652e4f91"}]}
