| opendevreview | ASHWIN A NAIR proposed openstack/swift master: s3api errors for unsupported headers x-delete-at, x-delete-after https://review.opendev.org/c/openstack/swift/+/864788 | 01:25 |
|---|---|---|
| opendevreview | Matthew Oliver proposed openstack/swift master: WIP sharder: update own_sr stats explicitly https://review.opendev.org/c/openstack/swift/+/852283 | 01:42 |
| opendevreview | Matthew Oliver proposed openstack/swift master: Sharding: No stat updates before CLEAVED state https://review.opendev.org/c/openstack/swift/+/837811 | 01:42 |
| opendevreview | Matthew Oliver proposed openstack/swift master: WIP: sharding: Block moving to CLEAVED if cleaved rows < expected https://review.opendev.org/c/openstack/swift/+/843973 | 01:42 |
| opendevreview | ASHWIN A NAIR proposed openstack/swift master: proxy-server exception logging shows replication_ip/port https://review.opendev.org/c/openstack/swift/+/860866 | 02:33 |
| opendevreview | Alistair Coles proposed openstack/swift master: swift_proxy: add memcache skip success/error stats for shard range. https://review.opendev.org/c/openstack/swift/+/858942 | 10:49 |
| opendevreview | Alistair Coles proposed openstack/swift master: trivial: fix flakey account/test_auditor.py assertion https://review.opendev.org/c/openstack/swift/+/866138 | 11:02 |
| zigo | timburke: Thanks a lot for the investigations ! | 11:24 |
| zigo | timburke: Should I still apply https://review.opendev.org/c/openstack/swift/+/866051 to my debian/zed branch? | 11:25 |
| opendevreview | Merged openstack/swift master: trivial: fix flakey account/test_auditor.py assertion https://review.opendev.org/c/openstack/swift/+/866138 | 14:48 |
| cylopez | hello, openstack-swift did "illegal transfer encoding" error in swiftclient ring your bell about a glance image upload from horizone ? | 16:00 |
| cylopez | I got it, glance doesn't provide the size during to swiftclient, so swift backend is replying 501 not implemented | 16:32 |
| opendevreview | Alistair Coles proposed openstack/swift master: sharder: update own_sr stats explicitly https://review.opendev.org/c/openstack/swift/+/852283 | 17:05 |
| timburke | zigo, yeah, i think that patch should square you on 3.11.0 | 17:33 |
| opendevreview | Tim Burke proposed openstack/swift master: Tolerate EOFError on input() https://review.opendev.org/c/openstack/swift/+/866209 | 19:59 |
| timburke | #startmeeting swift | 21:00 |
| opendevmeet | Meeting started Wed Nov 30 21:00:15 2022 UTC and is due to finish in 60 minutes. The chair is timburke. Information about MeetBot at http://wiki.debian.org/MeetBot. | 21:00 |
| opendevmeet | Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. | 21:00 |
| opendevmeet | The meeting name has been set to 'swift' | 21:00 |
| timburke | who's here for the swift meeting? | 21:00 |
| mattoliver | o/ | 21:00 |
| kota | o/ | 21:01 |
| acoles | o/ | 21:01 |
| timburke | as usual, the agenda's on the wiki | 21:02 |
| timburke | #link https://wiki.openstack.org/wiki/Meetings/Swift | 21:02 |
| timburke | first up | 21:02 |
| timburke | #topic func test failures | 21:02 |
| timburke | so we've still got those failing func tests on centos9 | 21:02 |
| timburke | and gmann was good enough to pin our dsvm job to focal so it wouldn't start failing, too | 21:03 |
| timburke | we've got a fix in hand, and it's even got a +2 (thanks, acoles!) | 21:03 |
| timburke | #link https://review.opendev.org/c/openstack/swift/+/863441 | 21:04 |
| timburke | so: are there any objections to merging it? | 21:04 |
| mattoliver | Ok cool. I said I'd look at that, sorry, got distracted. Promise to take a look first thing today. | 21:04 |
| timburke | no worries; thanks mattoliver | 21:05 |
| mattoliver | From when I looked at it, it made sense. | 21:05 |
| timburke | then separately: do we think it's worth removing our dependence on stdlib's http parsing? i have a gut feeling that it would be a fairly large endeavor | 21:06 |
| timburke | and it's not clear whether we'd want to have that in-tree or push it into eventlet. i got the impression from https://github.com/eventlet/eventlet/pull/574 that temoto might be receptive to that... | 21:08 |
| timburke | it doesn't necessarily mean that we'd have to own the parser, either -- something like https://github.com/python-hyper/h11/ looks somewhat appealing | 21:09 |
| mattoliver | That does sound like a large undertaking. And keeping it maintained. Although we do pull more and more. I dunno, but if eventlet want to maintain it. Maybe it's worth it. | 21:09 |
| mattoliver | Although would that mean I tighter coupling with eventlet | 21:10 |
| timburke | and we'd almost certainly need to revisit https://review.opendev.org/c/openstack/swift/+/427911 | Replace MIME with PUT+POST for EC and Encryption | 21:11 |
| timburke | anyway, i think we can debate that more later. my biggest worry is that we already have a bunch of large undertakings in progress | 21:13 |
| mattoliver | yeah | 21:13 |
| mattoliver | not a bad idea, maybe add it to ideas or something so we can discuss it more, or have a place to write things | 21:13 |
| timburke | as long as we can continue to paper over the current trouble, i think we can defer that decision | 21:13 |
| timburke | next up | 21:14 |
| mattoliver | +1 | 21:14 |
| timburke | #topic testing under py310 | 21:14 |
| timburke | just a quick update that we're still blocked on requirements | 21:14 |
| timburke | #link https://review.opendev.org/c/openstack/requirements/+/863581/ | 21:14 |
| timburke | i gave a nudge just before the meeting, we'll see if we can get this moving | 21:15 |
| timburke | #topic testing under py311 | 21:15 |
| timburke | regardless of what *we're* testing with, our users will continue marching forward! ;-) | 21:16 |
| timburke | and by doing that, we helped discover a cpythong bug | 21:16 |
| timburke | zigo noticed it first | 21:16 |
| timburke | #link https://meetings.opendev.org/irclogs/%23openstack-swift/%23openstack-swift.2022-11-24.log.html | 21:16 |
| mattoliver | yup, and you found a rather large issue in 3.11 too right? | 21:17 |
| timburke | i eventually ran it down and wrote it up | 21:17 |
| timburke | #link https://github.com/python/cpython/issues/99886 | 21:17 |
| timburke | and it looks like they're working on a fix | 21:17 |
| timburke | #link https://github.com/python/cpython/pull/99902 | 21:17 |
| zigo | FYI, I have warned Doko aka Matthias Klose (maintainer of the interpreter in Debian and Ubuntu) about it on #debian-python. | 21:18 |
| zigo | Hopefully, we'll have a fix soon for it. | 21:18 |
| timburke | one question i've still got on my mind: i found a workaround to avoid the segfault -- is it worth us applying that? or should we just wait it out for a 3.11.1? | 21:18 |
| zigo | However, it's not clear to me: do I still need a patch on the Swift side for it? | 21:19 |
| timburke | #link https://review.opendev.org/c/openstack/swift/+/866051 | 21:19 |
| zigo | Oh, thanks. | 21:19 |
| zigo | IMO, not worth merging upstream, but I can take it in downstream Debian until the interpreter is fixed. | 21:19 |
| timburke | zigo, if cpython fixes their issue, i don't think the swift patch is needed (though i also haven't tested with the WIP fix yet) | 21:19 |
| zigo | I'll build and let you know if that's an ok-ish temp fix for me. | 21:20 |
| timburke | 👍 | 21:20 |
| zigo | Thanks again tim ! | 21:20 |
| acoles | great detective work timburke | 21:20 |
| timburke | there was something surprising in my work-around patch that seemed like we might still want... | 21:21 |
| timburke | #link https://review.opendev.org/c/openstack/swift/+/866051/2/swift/common/db.py#a585 | 21:22 |
| timburke | the blanket exception handling in that context manager on master doesn't sit right with me | 21:22 |
| zigo | I just wrote about the above at the Debian bug entry: https://bugs.debian.org/1024784 | 21:22 |
| timburke | thanks | 21:23 |
| timburke | all right, we'll plan on holding off on that patch, though i might pull that bit i highlighted out as its own patch | 21:24 |
| timburke | #topic ring v2 | 21:24 |
| timburke | so based on some feedback from clayg i started playing around with using zipfiles as rings | 21:25 |
| timburke | #link https://review.opendev.org/c/openstack/swift/+/865066 | 21:25 |
| timburke | it's definitely still rough (not near as polished as the patch we talked about at the PTG), but the core ideas are all there | 21:26 |
| timburke | but i think my biggest concern is over the name change | 21:26 |
| timburke | 1. there are a bunch of places throughout the code base where we assume rings will be named *.ring.gz | 21:27 |
| timburke | 2. dealing with automatic ring reloading seems like it'd get a little hairy | 21:28 |
| mattoliver | I plan to as a follow up to the zip patch as it stands, so see what the fallout of a name change would look like. | 21:28 |
| mattoliver | might play with that today if there isn't a fire or distraction at work | 21:28 |
| timburke | that said, there were definitely some nice things about being able to use zip/unzip on the thing -- in particular, it seems like a much better way to shove in whatever other extension data you might want to put in a ring | 21:29 |
| mattoliver | I assume ring reloaded, we'd just be looking at the latest mtime. | 21:29 |
| mattoliver | maybe it wont be so bad | 21:29 |
| mattoliver | but time and code will tell :P | 21:30 |
| timburke | mattoliver, the thing that really got in my way was that we hang on to the filename we loaded and keep checking that -- being able to flop between two names got messy | 21:31 |
| mattoliver | hmm, yeah | 21:31 |
| timburke | anyway, based on that, i pushed up a new swift-ring-packer utility | 21:31 |
| timburke | #link https://review.opendev.org/c/openstack/swift/+/866082 | 21:31 |
| timburke | lets you pack/unpack/list v2 rings made with the patch that we talked about in the PTG | 21:32 |
| timburke | that's all i've got | 21:33 |
| timburke | #topic open discussion | 21:33 |
| timburke | what else should we talk about this week? | 21:33 |
| mattoliver | I've made some progress on cleaning up the tracing stuff | 21:33 |
| mattoliver | even moved it into a better namespace inside swift | 21:34 |
| mattoliver | At the end of the chain I have a docs patch with an overview_trace | 21:34 |
| zigo | timburke: I can confirm your workaround fixes the Swift Debian package unit test runs (just did a build run...). | 21:35 |
| timburke | \o/ | 21:35 |
| timburke | on both fronts ;-) | 21:35 |
| mattoliver | The doc is very rough, but plan to put design decisions there, because I did have to make some interesting decisions thanks to eventlet | 21:35 |
| mattoliver | And it's kinda important to know when reviewing tracing in swift | 21:35 |
| mattoliver | #link https://review.opendev.org/c/openstack/swift/+/865612 | 21:36 |
| mattoliver | ^ thats the end of the chain with the doc | 21:36 |
| mattoliver | I'm also trying to revive the shrinking chain | 21:37 |
| mattoliver | #link https://review.opendev.org/c/openstack/swift/+/843973 | 21:37 |
| mattoliver | ^currenlty ending there | 21:38 |
| mattoliver | I see acoles has worked worked on the front end of the chain again. So will check it out today | 21:38 |
| mattoliver | yup, he double worked :P | 21:38 |
| acoles | mattoliver: I think it is in good shape, thanks for your help. | 21:39 |
| mattoliver | nps | 21:39 |
| acoles | I checked test coverage and added some | 21:39 |
| acoles | I'll try to keep working up the chain (which I just realised I did not rebase), maybe we can get the first merged | 21:40 |
| mattoliver | yeah! | 21:40 |
| mattoliver | good plan | 21:40 |
| timburke | i'll take another look, too | 21:41 |
| acoles | I've been working on reducing the memcache blob size for cached shard ranges https://review.opendev.org/c/openstack/swift/+/863562 | 21:42 |
| mattoliver | oh cool | 21:42 |
| acoles | there's two ideas in there: 1) inspired by timburke's patch, compact the serialised shard range data by only storing the lower & name | 21:43 |
| acoles | 2) use a shallow tree to break up 1000's of shard ranges into leaves of 100's | 21:43 |
| timburke | nice! | 21:44 |
| timburke | i bet i can help with that first TODO ;-) | 21:44 |
| acoles | timburke: oh, please do 🙏 | 21:44 |
| mattoliver | it looks awesome, and thanks for responding to my questions inline. I'll loop back to it soon | 21:45 |
| acoles | one wrinkle I uncovered (well, probe tests uncovered!): the backend may return overlaps in updating shard range lists (ok overlaps, when a parent is sharding and children are created) | 21:45 |
| acoles | and the overlap does not play well with compacting the shard ranges to only (lower, name) :O | 21:46 |
| acoles | so I ended up needing to filter out the overlaps before serialising to cache | 21:46 |
| acoles | everything is fine on master because of the way the ranges sort by (upper, state, lower) but once we throw away upper and state the overlaps mess things up | 21:47 |
| mattoliver | ahh yeah | 21:47 |
| acoles | so...should we change the backend to filter what it returns? | 21:47 |
| timburke | i remember seeing that on my patch, too -- pushed up https://review.opendev.org/c/openstack/swift/+/852221 fwiw | 21:47 |
| acoles | that means we have an upgrade dance - backends must be upgraded before proxies | 21:48 |
| mattoliver | I guess having a cached valid path is good anyway. So long as any mistakes can eventually be fixed. | 21:48 |
| timburke | i suppose the better approach might be to have proxies do the filtering themselves... | 21:48 |
| acoles | mattoliver: yes, all we want in cache is a single contiguous path of namespaces | 21:49 |
| acoles | I need to ponder more what the proxy should do in the face of gaps ... we could have the proxy stretch a neighbor to cover a gap for updates 🤷 | 21:49 |
| mattoliver | or put the root in there | 21:50 |
| timburke | yeah, that was my thought, too | 21:50 |
| mattoliver | or not cache and force it to go back to the container for truth, but that would cause hotspotting (but alert us to the problem quickly) :P | 21:50 |
| acoles | yes. again, I think we (maybe timburke ) have dabbled with the backend filling ALL gaps with the root, but again that gets us into an upgrade wrangle | 21:51 |
| timburke | though if we're only tracking (lower, name), we necessarily start stretching shards | 21:51 |
| acoles | timburke: we can put ANY name (including root) in a tuple with a lower to fill a gap | 21:51 |
| timburke | true | 21:51 |
| acoles | I mean, we could even pick a random other range to spread the pain :) | 21:52 |
| mattoliver | oh thats true | 21:52 |
| mattoliver | and take load off the root | 21:52 |
| acoles | ...and muddle the debugging.. :( | 21:52 |
| mattoliver | well maybe just leave the gap, as tim says it'll get caught by the preceeding range | 21:53 |
| mattoliver | then the question remains what happens if the gap is at the start | 21:53 |
| acoles | ok, I'll work up something to handle gaps | 21:54 |
| acoles | at this point I'd appreciate any feedback on whether the whole endeavour seems worthwhile | 21:54 |
| timburke | definitely seems worthwhile to me -- we've been struggling with those large cache values, for sure | 21:54 |
| mattoliver | +1 | 21:55 |
| mattoliver | and there only going to go up, unless we do sometihng | 21:55 |
| acoles | and whether the class hierarchy makes sense to others - for a while I've wanted a superclass of ShardRange that just knows about namespace bounds | 21:55 |
| acoles | I recently checked and we have one shard range list at > 4MB! | 21:56 |
| mattoliver | that is a little nuts. | 21:56 |
| timburke | one question: now that there are two lookups, what happens if the first succeeds, but the second fails? i'm guessing we just treat it as a miss and go back to the root for a fresh set of everything? | 21:56 |
| mattoliver | how big is the list once you run it through your code now? | 21:56 |
| acoles | mattoliver: less ;-) | 21:57 |
| mattoliver | lol, great, ship it then :P | 21:57 |
| acoles | timburke: that would be the intent | 21:57 |
| timburke | mattoliver, on my other patch (that just reduced what we track without breaking it into a tree), i saw a 3.1MB cache value shrink to 0.9MB | 21:58 |
| acoles | mattoliver: seriously, each blob is reduced to some configurable max-items, but for our 4MB maybe a 10x reduction? | 21:58 |
| acoles | And then we can estimate at least a 2x reduction for simply dropping the uppers | 21:59 |
| mattoliver | nice! | 21:59 |
| acoles | ok, so what timburke measured is for the compaction alone, then the shallow tree gets us even smaller if needed | 22:00 |
| timburke | acoles, we're always limited to just the two layers, right? seems like we'd want to target len(shard_ranges)**0.5 shards per blob | 22:00 |
| timburke | all right, we're at time | 22:01 |
| timburke | thank you all for coming, and thank you for working on swift! | 22:01 |
| timburke | #endmeeting | 22:01 |
| opendevmeet | Meeting ended Wed Nov 30 22:01:23 2022 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4) | 22:01 |
| opendevmeet | Minutes: https://meetings.opendev.org/meetings/swift/2022/swift.2022-11-30-21.00.html | 22:01 |
| opendevmeet | Minutes (text): https://meetings.opendev.org/meetings/swift/2022/swift.2022-11-30-21.00.txt | 22:01 |
| opendevmeet | Log: https://meetings.opendev.org/meetings/swift/2022/swift.2022-11-30-21.00.log.html | 22:01 |
| acoles | timburke: yeah, so mattoliver asked about deeper trees and i commented on patch that I stopped myself making the tree deeper (by recursion) because i think one level gives us lots of headroom, but I also tried to make it extensible in future | 22:01 |
| timburke | ah, got it | 22:01 |
| acoles | timburke: TBH the compaction alone might be enough (?) in many case | 22:02 |
| timburke | i definitely like the idea of keeping growth sub-linear | 22:03 |
| acoles | I actually started with a recursive arbitrary depth tree, then chanted YAGNI 100 times and deleted that code :P | 22:03 |
| timburke | lol | 22:03 |
| acoles | it hurt | 22:03 |
| acoles | ok, good night o/ | 22:04 |
| timburke | o/ | 22:04 |
| opendevreview | Tim Burke proposed openstack/swift master: WIP: restructure cached updating shard ranges https://review.opendev.org/c/openstack/swift/+/863562 | 22:34 |
| opendevreview | Tim Burke proposed openstack/swift master: DB locks shouldn't squelch errors https://review.opendev.org/c/openstack/swift/+/866239 | 23:48 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!