)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"bcd4134da56a761c78790ded8e2fe9ec6cbbd9f0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"edc3edaf_2d179832","updated":"2023-10-23 14:55:13.000000000","message":"1) how\u0027d you come across this?  does it need an lp bug #?\n2) I don\u0027t think I was aware of this feature - do you have any docs or a related-chagne you could link to?\n3) Is there an upgrade impact - i guess if the account name had a \u0027%%20\u0027 it was already broken?\n4) pete, quoting seems more robust than a rsplit - wouldn\u0027t account names with any special url quotable character be just as relevant?\n5) needs tests","commit_id":"a9d40a3e4ac80f4d7b658a77340a44edc0556cdd"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"80901b72e3a22d9729aebe9f52bc75df22712042","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"ac3e27be_3486f32f","in_reply_to":"a3992249_720223d6","updated":"2024-01-13 05:57:54.000000000","message":"Oh, man, this suppression stuff is *old* -- see [`217198b8`](https://github.com/openstack/swift/commit/217198b83b69095724c8cb8aae6746fbbfbe8556)","commit_id":"a9d40a3e4ac80f4d7b658a77340a44edc0556cdd"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"3bbeb2870b15e7d9cb2134740f396c83e8ac359b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"a3992249_720223d6","in_reply_to":"edc3edaf_2d179832","updated":"2023-10-23 19:13:49.000000000","message":"\u003e how\u0027d you come across this?\n\nIt\u0027s a bit of a long story -- this was just one actionable/patchable thing I could actually push up from it.\n\nI\u0027ve been playing with my own external monitoring at home -- one of the things I wanted to do was to track %CPU usage *per process*, or at least *per process type*. It started out just looking at snapshots of `ps` `%CPU` output, but I quickly noticed that it wouldn\u0027t give me great results -- you can see some of the issues by running `top | head` repeatedly in an AIO -- very spikey, or correlated more with how long it\u0027d been since the process was restarted than with current actual workload. I moved toward tracking the times/etimes so I could get %CPU for the 10s or 30s window between prom scrapes, which gave me much better/smoother results, for the most part.\n\nA few processes, though, would still have kinda weird/spiky/unexpected behavior. `container-updater` was one of them -- and I think it was that my fallback (if I didn\u0027t already know about a process) was to go back to using %CPU. So, between\n* the fact that we\u0027re constantly spinning off a new process for each part to send updates for and\n* the fact that my home cluster has few enough containers that part ≅ db\n\nI\u0027m constantly spinning up new, short-lived processes, and the overhead of that really comes through in the %CPU fallback. (Or at any rate, it *seemed to* -- the whole side project has made me really appreciate how difficult it is to maintain alignment between\n\n* what you\u0027re actually measuring,\n* what you think you\u0027re measuring, and\n* what\u0027s actually useful to measure.)\n\nThat led me to looking more at not just how we\u0027re managing processes, but how we do IPC, too -- and here we are. At least I finally got to the point of realizing that we really **can** get more than one account in these temp files! It took me a bit to grok it in my constrained environments.\n\n\u003e does it need an lp bug #?\n\nProbably.\n\n\u003e Is there an upgrade impact\n\nNope -- they\u0027re temp files, only used for the one run.\n\n\u003e i guess if the account name had a \u0027%%20\u0027 it was already broken?\n\nYep -- but only if/when you need to suppress the account server. The split will likely die with a `ValueError` when we try to unpack three (or more) items to two vars.\n\n\u003e wouldn\u0027t account names with any special url quotable character be just as relevant?\n\nNot really -- you\u0027d want to be clear about what the delimiter is, though, to make sure we aren\u0027t splitting on more than just `\u0027 \u0027`. Especially with py3, there\u0027s more characters that count as whitespace...\n\n\u003e needs tests\n\nThis is true for *all* of `container-updater` -- but that\u0027s no reason to *not* add them now! I\u0027ll see what I can do -- might just make some DBs on disk and really spin out subprocesses to really demonstrate the bad, though. In which case the test run might get noisy -- or we\u0027ll need to swap the `sys.exit()` for a `os._exit()`...\n\nOr maybe it\u0027s enough to just poke at things with `process_container`/`_load_suppressions` ... I\u0027ll see what I come up with.","commit_id":"a9d40a3e4ac80f4d7b658a77340a44edc0556cdd"}],"swift/container/updater.py":[{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"8f650916bb456d102bab0251bf691fee33822cbc","unresolved":true,"context_lines":[{"line_number":120,"context_line":"        try:"},{"line_number":121,"context_line":"            with open(filename, \u0027r\u0027) as tmpfile:"},{"line_number":122,"context_line":"                for line in tmpfile:"},{"line_number":123,"context_line":"                    account, until \u003d line.split()"},{"line_number":124,"context_line":"                    until \u003d float(until)"},{"line_number":125,"context_line":"                    self.account_suppressions[unquote(account)] \u003d until"},{"line_number":126,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":1,"id":"37f69c7c_d5b78d6a","line":123,"updated":"2023-10-21 00:21:04.000000000","message":"Why not just rsplit(maxsplit\u003d1)? Solves the issue, doesn\u0027t it?","commit_id":"a9d40a3e4ac80f4d7b658a77340a44edc0556cdd"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"3bbeb2870b15e7d9cb2134740f396c83e8ac359b","unresolved":true,"context_lines":[{"line_number":120,"context_line":"        try:"},{"line_number":121,"context_line":"            with open(filename, \u0027r\u0027) as tmpfile:"},{"line_number":122,"context_line":"                for line in tmpfile:"},{"line_number":123,"context_line":"                    account, until \u003d line.split()"},{"line_number":124,"context_line":"                    until \u003d float(until)"},{"line_number":125,"context_line":"                    self.account_suppressions[unquote(account)] \u003d until"},{"line_number":126,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":1,"id":"0a051058_d0e03358","line":123,"in_reply_to":"37f69c7c_d5b78d6a","updated":"2023-10-23 19:13:49.000000000","message":"Not if there\u0027s a newline in the account name -- you\u0027d have to be a little crazy to allow your users to create such an account, but... we allow it...","commit_id":"a9d40a3e4ac80f4d7b658a77340a44edc0556cdd"}]}
