)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"3ad883132cf8aa46d4eba61cfce6232659914234","unresolved":true,"context_lines":[{"line_number":11,"context_line":"timestamp, expirer queue updates can. Therefore the tests do make"},{"line_number":12,"context_line":"sense in that context."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Rename the vars to exactly what they are; the author of this patch"},{"line_number":15,"context_line":"found the references to \u0027put\u0027 and \u0027post\u0027 confusing in the context of a"},{"line_number":16,"context_line":"ContainerBroker test."},{"line_number":17,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"bc1d91bc_09bd478b","line":14,"updated":"2024-12-09 20:04:39.000000000","message":"I think they were named after what they were representing - and this change is renaming them to how they\u0027re used.  I don\u0027t know if that\u0027s only clarifying.","commit_id":"53139e1b24f3d72666a8ad04d141bd72b4eebc1c"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"3ad883132cf8aa46d4eba61cfce6232659914234","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"33c53894_f5f9484f","updated":"2024-12-09 20:04:39.000000000","message":"I think it\u0027s probably a much larger scope to rework the expirer-bytes handling to use the \"x-[data-]-timestamp\" to represent the expired object datafile timestamp (and x-meta-timestamp to represent the req.x-timestamp) - particularly I\u0027m worried using x-size directly may \"double account\" for bytes unless we already exclude `.expiring_objects` from aggregated reporting.\n\nOn the scope of ONLY \"improve these tests\" I think it\u0027s useful existing context to demonstrate (or comment) that the *value* of x-ctype-timestamp IS the datafile/put-timestamp of the original-request/existing-size-metadata from which we\u0027re pulling the bytes value that\u0027s encoded into content-type.\n\nI think we should include \"older because PUT/datafile\" in the new comment.\n\nOR perhaps if the vars continued to be named putN_ts we need comment that their *usage* as `ctype_timestamp\u003dputN_ts` is \"intentional\":\n\n```\nobj.server uses putN_ts as ctype_timestamp because that\u0027s where it (and SLO) writes the \"extra size info\" for the row; leaving x-size to be the *actual* size represented by the row, i.e. `0` (or \"manifest-size\" in the case of SLO)\n```\n\nThen we try NOT to open the \"ctype updates on inconsistent SLOs\" can of worms OR the hope that native-MPU will hopefully use x-size directly and only store \"manifest-bytes\" somewhere near the \"manifest-etag\" (if that\u0027s even useful) - because I\u0027m not sure either of those issues really tells us anything about how/where the expirer should be scribbling down the size of the object to be expired along with the delete-at row.","commit_id":"53139e1b24f3d72666a8ad04d141bd72b4eebc1c"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"10a382a833211d8e28ff1215c356e609d009620f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"5eeec5d0_16aac6b0","updated":"2024-12-10 11:02:21.000000000","message":"I\u0027ve reduced the change down to a comment.\n\nI prefer good separation between components; it should be possible to maintain one component without loading the context of another component. In that respect the var names were a distraction, to me at least. On the other hand, it is useful to have comments as to why a component\u0027s features are necessary, so some bleeding of concerns is inevitable.","commit_id":"3a5bbcd7a6501962ee4fd545e99de2e16d2c2853"}],"test/unit/container/test_backend.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"6dc57cfbc72c755075d9fc74cdd9d46ada961609","unresolved":true,"context_lines":[{"line_number":7199,"context_line":"    def test_in_order_expirer_bytes_ctype(self):"},{"line_number":7200,"context_line":"        broker \u003d self._get_broker()"},{"line_number":7201,"context_line":"        # in the context of expirer queue updates the ctype timestamp can be"},{"line_number":7202,"context_line":"        # older than the row\u0027s data timestamp"},{"line_number":7203,"context_line":"        ts_ctype1 \u003d next(self.ts)"},{"line_number":7204,"context_line":"        ts_ctype2 \u003d next(self.ts)"},{"line_number":7205,"context_line":"        ts_data \u003d next(self.ts)"}],"source_content_type":"text/x-python","patch_set":1,"id":"84e952e2_0dfb3a76","line":7202,"updated":"2024-12-09 19:40:09.000000000","message":"Might be worth some explanation of *why* this happens:\n\n\u003e We use ctype to record bytes_used, which needs to reflect the object\u0027s .data, while the row\u0027s data timestamp corresponds to the .meta (or, on initial upload, .data) that applied the X-Delete-At.","commit_id":"53139e1b24f3d72666a8ad04d141bd72b4eebc1c"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"3ad883132cf8aa46d4eba61cfce6232659914234","unresolved":true,"context_lines":[{"line_number":7199,"context_line":"    def test_in_order_expirer_bytes_ctype(self):"},{"line_number":7200,"context_line":"        broker \u003d self._get_broker()"},{"line_number":7201,"context_line":"        # in the context of expirer queue updates the ctype timestamp can be"},{"line_number":7202,"context_line":"        # older than the row\u0027s data timestamp"},{"line_number":7203,"context_line":"        ts_ctype1 \u003d next(self.ts)"},{"line_number":7204,"context_line":"        ts_ctype2 \u003d next(self.ts)"},{"line_number":7205,"context_line":"        ts_data \u003d next(self.ts)"}],"source_content_type":"text/x-python","patch_set":1,"id":"175d58fb_3171bcaf","line":7202,"updated":"2024-12-09 20:04:39.000000000","message":"```\n... because the ctype timestamp *is the timestamp of the put/datafile*\n```\n\ni.e. I think the name of the variable name was trying to say something important that\u0027s getting lost here.","commit_id":"53139e1b24f3d72666a8ad04d141bd72b4eebc1c"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"3ad883132cf8aa46d4eba61cfce6232659914234","unresolved":true,"context_lines":[{"line_number":7201,"context_line":"        # in the context of expirer queue updates the ctype timestamp can be"},{"line_number":7202,"context_line":"        # older than the row\u0027s data timestamp"},{"line_number":7203,"context_line":"        ts_ctype1 \u003d next(self.ts)"},{"line_number":7204,"context_line":"        ts_ctype2 \u003d next(self.ts)"},{"line_number":7205,"context_line":"        ts_data \u003d next(self.ts)"},{"line_number":7206,"context_line":""},{"line_number":7207,"context_line":"        broker.put_object("}],"source_content_type":"text/x-python","patch_set":1,"id":"8589de60_e5033c11","line":7204,"updated":"2024-12-09 20:04:39.000000000","message":"yeah what I don\u0027t love about this is hides where the timestamp *comes from* - in the context of the expirer this timestamp is the timestamp of \"the put at t1 that had size\u003d1\" and \"the put at t2 that had size\u003d2\"\n\nThis variable name is more about \"how it\u0027s used\" than \"what it means\"","commit_id":"53139e1b24f3d72666a8ad04d141bd72b4eebc1c"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"3ad883132cf8aa46d4eba61cfce6232659914234","unresolved":true,"context_lines":[{"line_number":7219,"context_line":""},{"line_number":7220,"context_line":"        self.assertEqual([{"},{"line_number":7221,"context_line":"            \u0027content_type\u0027: \u0027text/plain;swift_expirer_bytes\u003d2\u0027,"},{"line_number":7222,"context_line":"            \u0027created_at\u0027: encode_timestamps(ts_data, ts_ctype2, ts_ctype2),"},{"line_number":7223,"context_line":"            \u0027deleted\u0027: 0,"},{"line_number":7224,"context_line":"            \u0027etag\u0027: \u0027d41d8cd98f00b204e9800998ecf8427e\u0027,"},{"line_number":7225,"context_line":"            \u0027name\u0027: \u00271234-a/c/o\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"26846c50_0e28aee3","line":7222,"updated":"2024-12-09 20:04:39.000000000","message":"\"the row\u0027s data timestamp\" - I discovered I was confused about the idea that the `x-timestamp` in the put_object API is NOT last_modified; it\u0027s the `x-[data-]-timestamp`\n\nLike if I do a post to an object the \"last_modified\" changes:\n\n```\nvagrant@saio:~$ swift upload test test\ntest\nvagrant@saio:~$ swift list test --json --prefix\u003dtest\n[\n  {\n    \"bytes\": 5,\n    \"content_type\": \"application/octet-stream\",\n    \"hash\": \"d8e8fca2dc0f896fd7cb4cb0031ba249\",\n    \"last_modified\": \"2024-12-09T18:55:14.515390\",\n    \"name\": \"test\"\n  }\n]\nvagrant@saio:~$ swift post test test -H \u0027x-object-meta-color: blue\u0027\nvagrant@saio:~$ swift list test --json --prefix\u003dtest\n[\n  {\n    \"bytes\": 5,\n    \"content_type\": \"application/octet-stream\",\n    \"hash\": \"d8e8fca2dc0f896fd7cb4cb0031ba249\",\n    \"last_modified\": \"2024-12-09T18:55:32.299840\",\n    \"name\": \"test\"\n  }\n]\n```\n\n^ apparently \"last_modified\" always uses the `X-Meta-Timestamp`/`metadata[\u0027X-Timestamp\u0027]`\n\nhttps://github.com/NVIDIA/swift/blob/master/swift/container/backend.py#L1308-L1318\n\nThe main idea being in order to merge:\n\n```\nupdate.t3 \u003d data.t2 + ctype.t3\nupdate.t4 \u003d data.t1 + ctype.t4\n```\n\n... we have to synthesize a row `data.t2 + ctype.t4` and encode it such that \"last_modified \u003d\u003d t4\" - so that\u0027s where the put_object `meta_timestamp` kwarg comes in.\n\nThe expirer doesn\u0027t really care about the last_modified of the row; although it may soon:\n\n932447: Object-server: return 409 if x-delete-at is newer than queue entry | https://review.opendev.org/c/openstack/swift/+/932447\n\nIf we wanted to try and migrate, we could consider hiding the bytes in the etag (which is also made up for expirer rows) or x-size (which is always 0 for expirer row) instead of the content-type (borrowing from where SLOs put them) and maybe start using x-timestamp\u003d\u003ddata-timestamp and x-meta-timestamp\u003dreq.timestamp?\n\nPerhaps that would work \"more intuitively\" with how \"normal\" object-listings manage the trio of \"size-metadata-timestamp; ctype-metadata-timestamp; last-modified-metadata-timestamp\" even tho the expirer only cares about two \"size-metadata-timestamp; last-modified-metadata-timestamp\" and (confusingly?) uses x-ctype-timestamp cause that\u0027s where it writes the size-metadata!?\n\nhttps://github.com/NVIDIA/swift/blob/master/swift/obj/server.py#L739-L759","commit_id":"53139e1b24f3d72666a8ad04d141bd72b4eebc1c"}]}