| tdasilva | mattoliverau: o/ | 00:08 |
|---|---|---|
| mattoliverau | tdasilva: o/ | 00:08 |
| *** hoonetorg has quit IRC | 01:16 | |
| *** hoonetorg has joined #openstack-swift | 01:20 | |
| seongsoocho | morning ~ | 01:21 |
| tdasilva | seongsoocho: o/ | 01:25 |
| mattoliverau | seongsoocho: o/ | 01:30 |
| seongsoocho | I have a trouble uploading object with formpost in train version (python3). If the filename in form-data contains unicode string, the upload failed. | 01:32 |
| seongsoocho | https://github.com/openstack/swift/blob/master/swift/common/swob.py#L308 | 01:32 |
| seongsoocho | I think wsgi_quote function can't handle unicode string. | 01:32 |
| seongsoocho | I'm testing it and once the problem is cleared I'll register it as a bug. | 01:33 |
| *** threestrands has joined #openstack-swift | 02:30 | |
| *** diablo_rojo has joined #openstack-swift | 03:49 | |
| *** diablo_rojo has quit IRC | 04:24 | |
| *** threestrands has quit IRC | 04:28 | |
| *** psachin has joined #openstack-swift | 04:49 | |
| *** psachin has quit IRC | 04:54 | |
| *** threestrands has joined #openstack-swift | 05:40 | |
| timburke | well *that* looks like some crazy-town... https://github.com/openstack/swift/blob/2.23.1/swift/common/utils.py#L4447-L4451 | 05:50 |
| timburke | (which is getting called at https://github.com/openstack/swift/blob/2.23.1/swift/common/middleware/formpost.py#L273 ) | 05:52 |
| timburke | makes me worry that non-ascii content-types are broken, too :-/ | 05:56 |
| timburke | (at least, in some circumstances) | 05:56 |
| *** baojg has joined #openstack-swift | 05:56 | |
| *** baojg has quit IRC | 05:58 | |
| *** psachin has joined #openstack-swift | 06:17 | |
| seongsoocho | timburke: ok.. and I have one question. Is there any reason that why wsgi_quote encode a wsgi_path to latin1? | 06:23 |
| timburke | because wsgi uses latin1 for headers -- even if swift requires that container metadata be valid utf-8 strings, when a wsgi server like eventlet parses the bytes off the wire, it reads them as latin1 and passes them to us as latin1 | 06:26 |
| timburke | so then we later need to re-encode to bytes and check that they decode as utf-8 | 06:26 |
| timburke | https://www.python.org/dev/peps/pep-3333/#unicode-issues has been the bane of my existence the last year or two :-( | 06:27 |
| seongsoocho | oh.. ok. I see. | 06:28 |
| *** psachin has quit IRC | 06:29 | |
| timburke | wsgi_quote and wsgi_unquote work with what i've teken to calling "wsgi strings" -- normal strs on py2, and utf-8-pretending-to-be-latin-1 on py3. both of those functions expect the input to be a wsgi string, and will return a wsgi string | 06:30 |
| seongsoocho | :( .... The unicode issue is always difficult to me.. | 06:32 |
| seongsoocho | I'm relieved to find this issue before release to production. | 06:32 |
| *** hoonetorg has quit IRC | 07:20 | |
| *** rpittau|afk is now known as rpittu | 07:44 | |
| *** rpittu is now known as rpittau | 07:44 | |
| *** threestrands has quit IRC | 08:08 | |
| *** psachin has joined #openstack-swift | 08:15 | |
| *** tkajinam has quit IRC | 08:16 | |
| openstackgerrit | Thiago da Silva proposed openstack/swift master: New Object Versioning mode https://review.opendev.org/682382 | 08:18 |
| openstackgerrit | Thiago da Silva proposed openstack/swift master: Account listing for object versioning https://review.opendev.org/700411 | 10:24 |
| *** mvkr has quit IRC | 10:43 | |
| clayg | tdasilva: yay for patch 700411!!! | 15:26 |
| patchbot | https://review.opendev.org/#/c/700411/ - swift - Account listing for object versioning - 1 patch set | 15:26 |
| clayg | tdasilva: yay for p 700114 too!! | 15:53 |
| patchbot | https://review.opendev.org/#/c/700114/ - swift - s3api DELETE request return version-id and marker - 1 patch set | 15:53 |
| *** rpittau is now known as rpittau|afk | 15:58 | |
| openstackgerrit | Merged openstack/swift master: sharding: Let swift-manage-shard-ranges accept a relative path https://review.opendev.org/700248 | 16:51 |
| timburke | good morning | 17:02 |
| timburke | clayg, thanks for the reviews! i think i'm'a rebase p 682382 and p 699892 in some order -- got any preference on which comes first? | 17:03 |
| patchbot | https://review.opendev.org/#/c/682382/ - swift - New Object Versioning mode - 54 patch sets | 17:03 |
| patchbot | https://review.opendev.org/#/c/699892/ - swift - sharding: Better-handle newlines in names - 2 patch sets | 17:03 |
| clayg | no preference, either is fine, no conflicts from me | 17:03 |
| clayg | tdasilva: had some hot fixups to s3api versioning that should be squashed in my opinon | 17:03 |
| timburke | should i try to set the old sysmeta in more cases? | 17:03 |
| clayg | no that doesn't really "help" | 17:04 |
| timburke | yeah, squashing seems entirely reasonable to me -- pretty much anytime someone's rebasing anyway | 17:04 |
| clayg | I'm pretty much come around to we have to write it tho - but there's a future lesson for us about having code take action on implicit assumptions instead of explicit | 17:04 |
| clayg | if it said something like 'if is root container do root update' instead of 'if can't find shard metadata assume root' I think that would have been better | 17:05 |
| clayg | lacking a time machine I guess ... well, like I said I guess i'm coming around 🤷♂️ | 17:06 |
| timburke | ...should i try to set it in *fewer*? and include an upgrade note like "shut down your sharders until upgrade is complete, kthxbye" | 17:06 |
| *** d34dh0r53 has quit IRC | 17:09 | |
| *** d34dh0r53 has joined #openstack-swift | 17:12 | |
| timburke | what do we think about having a cluster-wide setting for https://review.opendev.org/#/c/700056/ ? that was my approach in https://review.opendev.org/#/c/695131/ but i think that was mainly out of laziness | 17:13 |
| patchbot | patch 700056 - swift - Middleware that allow for a user to quote Etag - 2 patch sets | 17:13 |
| patchbot | patch 695131 - swift - Add proxy-server option to quote-wrap all ETags - 5 patch sets | 17:13 |
| timburke | clayg, idea! yes, set X-Container-Sysmeta-Shard-Root, but set it to something (anything!) without a slash! then _load_root_info() (and by extension, the root_account and root_container properties) will blow up on old code | 17:18 |
| clayg | take that old code! | 17:19 |
| timburke | i think i'd be ok with dropping X-Backend-Container-Path if we want. either way | 17:19 |
| clayg | that's the unquoted version - the one that the lack thereof will cause old code to send shard updates ... to themselves? until the upgrade? | 17:20 |
| clayg | maybe I should try to understand the failure mode better - when I originally did the review I didn't quite understand the context of the code path... | 17:20 |
| clayg | ... maybe after aother look I'd be ok dropping the dangerous unquoted ... oh what if we *always* included it and it was always unquoted? it's safe to *store* right? it's just incorrect to send it in headers without quoting? | 17:21 |
| timburke | no -- that's the one that gets the object update to the right shard -- without it, we'll try the root, which will either (a) eat the update and get it to the right shard as part of misplaced-objects handling or (b) tell the updater where it *should* be sending that update | 17:21 |
| clayg | IIRC the only thing I didn't like about the change was the metadata that was only there "sometimes" | 17:22 |
| clayg | I write one-off scripts alot and if I look at the metadata on a container I'd like all of the metadata there to be obvious in what it is and what it means and also representative of the metadata I'd find on other containers 🤷♂️ | 17:23 |
| timburke | there were two (kinda three) spots where i do something like that -- in the proxy: https://review.opendev.org/#/c/699892/2/swift/proxy/controllers/obj.py | 17:24 |
| patchbot | patch 699892 - swift - sharding: Better-handle newlines in names - 2 patch sets | 17:24 |
| timburke | and in the sharder: https://review.opendev.org/#/c/699892/2/swift/container/sharder.py | 17:24 |
| patchbot | patch 699892 - swift - sharding: Better-handle newlines in names - 2 patch sets | 17:24 |
| clayg | I'm absolutely find with sending any transient information on the wire that helps with rolling upgrade - if it's not necessary I agree it's optional | 17:25 |
| timburke | (honestly, i don't entirely remember why the sharder is setting that meta in two places -- i'd have to look at it some more) | 17:25 |
| clayg | but it's really only the "writing down different things" that smelled to me - everything else seemed quite reasonable | 17:25 |
| timburke | 👍 | 17:25 |
| clayg | timburke: I would have swore it was because later when a sharder loads metadata the missing sysmeta made it be stupid | 17:25 |
| clayg | but if there was a way to use the stupid to our advantage where old code starts blowing up and skipping all sharding until it upgrade that would be great! | 17:26 |
| clayg | i'd hate to have to say "shut down all sharders until you upgrade" just to avoid writing down some extra metadata... | 17:27 |
| clayg | i guess it doesn't make sense to store sysmeta that's not safe to send in headers - there's to much code that wants to always send sysmeta... maybe we could setup a blacklist translation sort of filter i.e. `x-sysmeta-<unquoted-key>: <unquoted-value> <=> x-sysmeta-quoted-<key>: <quoted-value>` | 17:28 |
| timburke | oh, right -- first we go issue a bunch of PUTs for all the shard ranges we found, then we *also* create local handoffs that need that info aas we're cleaving... i'm starting to remember now... | 17:28 |
| timburke | the good news is, i'm *pretty sure* no one's actually been bitten by this bug yet. i feel like we would've gotten reports if they had | 17:30 |
| timburke | so how about setting the old value to something like `root_path if root_path == quote(root_path) else 'see-bug-1856894'` -- and you only get the tracebacks if you've got "dangerous" container names | 17:33 |
| DHE | Found 15 ranges in 15.4685s (total object count 58509615) # nice | 17:42 |
| timburke | \o/ | 17:44 |
| clayg | timburke: how about always set it to see-bug-1856894 😬 | 17:45 |
| clayg | like... even if we encounter old containers that had it set and it would have just happened to be safe 🤔 | 17:45 |
| timburke | my main worry would be that it'll be a little disruptive if you've got some (working! happy!) in-progress sharding going on when you upgrade | 17:46 |
| *** baffle has quit IRC | 17:46 | |
| *** csmart has quit IRC | 17:46 | |
| *** psachin has quit IRC | 17:51 | |
| openstackgerrit | Merged openstack/swift master: sharding: Tolerate blank limits when listing https://review.opendev.org/700115 | 17:52 |
| clayg | yeah i mean i guess it depends on how long your upgrades run | 18:02 |
| clayg | but settig the path to something invalid some of the time seems about as bad as it sometimes missing - maybe worse? | 18:02 |
| timburke | i can't help but wonder if we maybe should've been having a discussion like this for p 555245 and p 570436... | 18:12 |
| patchbot | https://review.opendev.org/#/c/555245/ - swift - Fix versioned writes error with url-encoded object... (MERGED) - 3 patch sets | 18:12 |
| patchbot | https://review.opendev.org/#/c/570436/ - swift - Clarify that archive location headers should be UR... (MERGED) - 1 patch set | 18:12 |
| clayg | i mean maybe we missed something there - do either of them write down pieces of sysmeta that's only there *sometimes* ? | 18:16 |
| clayg | otherwise it's probably not related to whatever unneasyness was feeling about p 699892 (which I guess I'll note I had +2'd once already) - so, it may not be such a big deal? | 18:18 |
| patchbot | https://review.opendev.org/#/c/699892/ - swift - sharding: Better-handle newlines in names - 2 patch sets | 18:18 |
| clayg | so I am getting null character names into the reconciler just fine - that's good, I guess the rows since row_id api doesn't ever filter reserved | 18:19 |
| timburke | no, my thinking was more about adding a new header that unequivocally represents an unquoted name | 18:20 |
| clayg | yeah the only issue seems to be the direct client queue pop to the container server: container-reconciler: STDERR: DirectClientException: Container server 127.0.0.1:6031 direct DELETE '/sdb3/17/.misplaced_objects/1577124000/0%3A/AUTH_test/%00container%005fc6fd02-bee0-4c49-8b00-8e1bbb731ce5/%00object%0071550385-8ef4-417e-bcbb-e8d4a9f97a10' gave status 400: 127.0.0.1:6031 400 Bad Request: device sdb3 | 18:22 |
| timburke | makes sense | 18:23 |
| clayg | all the container updates from the object requests themselves seem to propogate up just fine 🤔 | 18:23 |
| clayg | timburke: does it? I was sort of surprised a backend direct request got rejected - bad request device sdb3? | 18:24 |
| timburke | i don't think that's the actual error message or anything | 18:24 |
| timburke | oh... and i bet it bombs out the whole reconcile() loop... ugh | 18:27 |
| clayg | yeah you're right DirectClientException interacts oddly with the __str__ method for it's parent ClientException | 18:28 |
| clayg | acctually, it all seems to work except the queue entry doesn't go away | 18:28 |
| timburke | is this with just one object, or multiple objects? my read on it makes me think that the first queue entry will get moved over fine, then we bomb out trying to pop_queue(), so we never get to the next entry | 18:30 |
| timburke | i'd be happy to be proven wrong, though! | 18:30 |
| clayg | well, I think pop_queue supresses errors | 18:31 |
| clayg | container-reconciler: Reconciler Stats: {'cleanup_attempt': 1, 'cleanup_success': 1, 'pop_queue': 1, 'found_object': 1, 'success': 1} (txn: tx575c112f317544dd92fd4-005e0107d5) | 18:31 |
| clayg | ^ but that's after a bunch of tracebacks - and it's going to keep doing that forever | 18:31 |
| clayg | so, definately not making progress | 18:31 |
| timburke | i guess the GreenPool in direct_delete_container_entry quiets the exceptions? interesting | 18:33 |
| timburke | "quiets" -- it logs tracebacks (which is good); it just doesn't propogate errors | 18:34 |
| clayg | anyway, the 400 is `Invalid reserved-namespace object` which *does* make sense - i was thinking the complaint was "includes null namespace" - but now I remember what was *actually* wrong with the reconciler queue 👍 | 18:35 |
| timburke | :-( why does https://f20bec36e3f198a9ccf9-fa78c758d05065ae4af77013f9d746d5.ssl.cf2.rackcdn.com/682382/54/check/swift-tox-func-encryption-py27/059bfa9/job-output.txt have b"strings" in the failure message?? | 18:48 |
| timburke | so weird... it's got paths like "/home/zuul/src/opendev.org/openstack/swift/.tox/func-encryption/local/lib/python2.7/site-packages/unittest2/case.py" -- so *that's* good | 18:51 |
| *** csmart has joined #openstack-swift | 18:58 | |
| *** baffle has joined #openstack-swift | 18:58 | |
| timburke | shoot -- just remembered https://github.com/openstack/swift/blob/2.23.0/swift/container/server.py#L296-L297 ... hmm... | 19:21 |
| clayg | for quoting? | 19:22 |
| timburke | yup | 19:22 |
| timburke | ...might be safe to just start quoting/unquoting? and rely on version skew causing 404s until upgrade completes... | 19:26 |
| clayg | Dec 23 20:41:51 saio proxy-server: While processing manifest /v1/AUTH_test/%00container%0095622d98-ecfd-4211-b4e1-e7fc867c5944/%00object%009540df1d-c3fb-49e5-bbcd-7f1233ae1d06-direct-test, got 412 while retrieving /v1/AUTH_test/%00container%0095622d98-ecfd-4211-b4e1-e7fc867c5944/%00manifest_part_00 (txn: tx62c44b77b24c4a93b0c18-005e01268f) (client_ip: 127.0.0.1) | 20:46 |
| clayg | so dunno, maybe I shouldn't bother with the reconciler test that does manifests 🤔 | 20:46 |
| clayg | yeah I think the only other option is to make DLO reserved name aware - like if the *manifest* is in the reserved namespace *and* the request has a allow header 🤮 | 20:53 |
| timburke | SLO's happy tho? | 20:57 |
| timburke | hmm... i suppose it probably wouldn't be... | 20:58 |
| timburke | i kinda *want* it to be, though -- like, it'd be great (IMO) if creating an SLO whose segments are in a versioned container would translate the segment names *for me*... so even if they got deleted/replaced, the SLO still works until someone cleans up the underlying old versions | 21:00 |
| timburke | though maybe it'd be better to jam version_id keys into the manifest instead -- so we don't expose the null namespace... | 21:00 |
| timburke | dkjasfkasd | 21:11 |
| timburke | https://github.com/openstack/swift/blob/2.23.0/test/probe/test_sharder.py#L567-L569 | 21:11 |
| timburke | w | 21:11 |
| timburke | t | 21:11 |
| timburke | f | 21:11 |
| clayg | oh, yeah not DLO 🤦♂️ SLO | 21:23 |
| clayg | it's the head requests | 21:23 |
| clayg | I think we can save "MPUs that referenced versioned segments correctly" for ALO | 21:25 |
| openstackgerrit | Clay Gerrard proposed openstack/swift master: Allow reconciler to handle reserved names https://review.opendev.org/700449 | 21:30 |
| *** mvkr has joined #openstack-swift | 21:39 | |
| timburke | lol -- so trying to shard a container with unicode in the name gets me 400s -- "Metadata must be valid UTF-8" | 21:41 |
| timburke | i *think* that might be because i'm doing all this on py3, and the unicode that the testis using fits in latin-1... | 21:42 |
| timburke | so then it gets mangled into non-utf8 crap | 21:42 |
| openstackgerrit | Tim Burke proposed openstack/swift master: sharding: Fix conainer_name typo in probe test https://review.opendev.org/700454 | 21:51 |
| clayg | HAHAHHA conainer_name | 21:56 |
| clayg | poor py3 | 21:56 |
| timburke | i mena -- at least we were testing non-ascii *object names*, right? | 22:03 |
| clayg | yes, i really like the idea of pulling auto_create_prefix into constraints! | 22:06 |
| clayg | i'll try and think about a reasonable upgrade path for that maybe 🤮 | 22:07 |
| timburke | :-( getting a quoted location and *not* unquoting doesn't mean you'll get a 404 when you do the update -- it means you'll pop some *other* container into existence... | 22:24 |
| timburke | looks like i've gotta split *that*, too | 22:24 |
| timburke | whooo! py2 fails, too! now, to sort out whether it's just because of https://bugs.python.org/issue30458 being fixed in new-enough python... | 22:31 |
| mattoliverau | morning | 22:37 |
| timburke | mattoliverau, o/ | 22:59 |
| *** tkajinam has joined #openstack-swift | 23:07 | |
| *** hoonetorg has joined #openstack-swift | 23:08 | |
| seongsoocho | morning | 23:56 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!