21:00:06 <timburke> #startmeeting swift
21:00:06 <openstack> Meeting started Wed Jul 22 21:00:06 2020 UTC and is due to finish in 60 minutes.  The chair is timburke. Information about MeetBot at http://wiki.debian.org/MeetBot.
21:00:07 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
21:00:10 <openstack> The meeting name has been set to 'swift'
21:00:17 <timburke> who's here for the swift meeting?
21:00:31 <alecuyer> o/
21:00:33 <rledisez> hi o/
21:00:34 <seongsoocho> o/
21:01:44 <clayg> are we having a 🎉!!!?
21:01:51 <timburke> agenda's at https://wiki.openstack.org/wiki/Meetings/Swift
21:02:00 <timburke> #topic releases
21:02:42 <timburke> so i'm realizing that i've been pretty bad about doing releases lately. i feel like i'd mentioned wanting to do a release a month or so ago, but just never got around to it
21:02:58 <timburke> so now it's been 3 months since 2.25.0
21:03:16 * tdasilva is lurking
21:03:17 <timburke> does anyone know of patches we really ought to have before a 2.26.0?
21:04:35 <clayg> not really, but we should cut 2.26.0 so we can start landing stuff for 2.27.0!!
21:04:47 <timburke> the only two that come to mind for me are p 742033 and p 739164
21:04:47 <patchbot> https://review.opendev.org/#/c/742033/ - swift - py3: Work with proper native string paths in crypt... - 3 patch sets
21:04:49 <patchbot> https://review.opendev.org/#/c/739164/ - swift - ec: Add an option to write fragments with legacy crc - 1 patch set
21:05:05 <clayg> yes, both of those are terrible 😥
21:05:10 <timburke> but as clayg points out, we can always have another release :-)
21:05:20 <clayg> i mean the problems the address - the patches are probably 🤩
21:05:37 <zaitcev> we're going to have 2.917.0 eventually if this continues
21:05:43 <timburke> and that last one doesn't entirely make sense without https://review.opendev.org/#/c/738959/
21:05:44 <patchbot> patch 738959 - liberasurecode - Be willing to write fragments with legacy crc - 1 patch set
21:06:24 <timburke> zaitcev, with luck we'll feel like we can drop py2 in the not *so* distant future -- that seems to warrant a 3.0 ;-)
21:06:28 <clayg> yeah the ec one seems worth some effort to get all the versions to line up
21:06:50 <clayg> the py3 fix is probably going to be backported stable releases anyway (timburke is doing great with the backports 👏)
21:07:33 <zaitcev> so, you went with an environment variable after all.
21:07:47 <clayg> zaitcev: did you have a better idea?!
21:07:51 <zaitcev> In that case, PyLibEC does not need a change, right?
21:08:12 <timburke> zaitcev, or rather, i never got around to trying to do it as a new, supported API
21:08:25 <timburke> correct, pyeclib won't need to change as things stand
21:09:01 <timburke> (which seems like a point in the env var's favor to me)
21:10:09 <clayg> ✅
21:10:42 <timburke> so i'm not really hearing much in the way of known issues -- is anyone going to be able to review either of those two threads in the near future? or should we just release without them?
21:11:53 <timburke> clayg's right about backporting the py3 fix -- i'm definitely planning to take it back to train, and if we get it reviewed soon-ish, i'll include it in the next stable release (which i hope to do concurrent with 2.26.0)
21:13:16 <clayg> the py3 fix is newer; but i'm less clear on how to effectively review the libec patches
21:15:38 <timburke> clayg, fwiw, i built a few different versions of libec (i want to say 1.0.9, 1.5.0, 1.6.0, and a new 1.6.1+ that supports the env var) and used LD_PRELOAD to split-brain my proxy and object layers
21:15:55 <clayg> 🤯
21:17:09 <timburke> the tricky part with *really* old libec is making sure you've got a pyeclib that supports it. it took a bit to get it right as i recall
21:17:25 <clayg> well that sounds miserable
21:17:30 <mattoliverau> o/ (sorry I'm late)
21:17:49 <timburke> but i think 1.5.0, 1.6.0, and 1.6.1+ likely all work well enough with master or latest-tag pyeclib
21:18:17 <clayg> I think i'm more likely to review the encryption/py3 one - can someone offer to help with the libec?
21:18:40 <timburke> the only reason i went back so far was to confirm that the tool i attached to the bug did the right thing on frags with no CRC
21:21:52 <timburke> clayg, thanks for picking up the py3 patch. i guess we'll continue kicking the can down the road for the ec work, but i think it's important that we get to a resolution by the end of the cycle
21:22:02 <timburke> on to updates!
21:22:14 <timburke> #topic replicaiton networks, saio, and probe tests
21:23:20 <timburke> just before the meeting i pushed up fresh versions of some patches that let me run separate replication servers in my saio
21:24:34 <timburke> i mentioned some concerns about the upgrade path in -swift
21:26:08 <clayg> it's gunna be FINE
21:26:17 <clayg> ... probably
21:26:20 <timburke> in particular, since replication servers today can't handle GET/PUT/etc. methods, when p 735751 comes in, there's the potential for a lot of 405s during a rolling upgrade
21:26:21 <patchbot> https://review.opendev.org/#/c/735751/ - swift - Allow direct and internal clients to use the repli... - 5 patch sets
21:26:59 <clayg> i mean, anyone running EC has already turned off replication_server = True; so it really only effects account/container servers
21:27:00 <timburke> in talking it through with clayg, i think it's fairly tractable -- you'd just need to make sure that any configs with replication_server=true have that line cleared -- then you can the server-can-respond-to-all-methods behavior
21:27:11 <clayg> 🤞
21:27:33 <clayg> I mean; we can probably test/verify that once we have vsaio ready to replication server!  💪
21:29:05 <timburke> so i bring this up for two reasons: first to raise awareness of the potential upgrade issue, and second to find out how other people are deploying their swifts -- will it even be much of a concern?
21:30:22 <rledisez> so, we don't use replication_server=True, so it's not a major concern to me. I assume every operator are reading changelog before upgrading
21:30:45 <timburke> i *did* discover that nvidia (formerly swiftstack) sets replication_server=true on a/c... so, that's a thing we'll need to fix...
21:31:07 <rledisez> but at least an operator upgrade one node to check everything is fine, so have a clear error message if the config line still exist is also mandatory I think (I mean, process does not start and log an error)
21:32:15 <clayg> rledisez: well the "error" would be with old code and old configs; once everyone is upgraded it's fine 🤔
21:32:16 <timburke> the trouble isn't the upgraded node, though -- it's the *other* ones that only respond to REPLICATE :-(
21:32:33 <rledisez> ha… I missed that
21:32:54 <clayg> timburke: I'm pretty sure we can find precedent where the answer was "push out a config that makes sense before you upgrade" - we just need to verify that works; but I think it's fine
21:33:44 <timburke> cool, i'll keep pushing on that then ;-)
21:33:46 <rledisez> I'm totally OK with that, if there is a proper upgrade procedure it seems standard in the software industry
21:33:59 <mattoliverau> can a new upgrade fall back to the client-data-path network on a 405 from a server on a replicated network?
21:34:01 <clayg> we'll fix our configs; it doesn't effect rledisez; other people probably don't even use replication networks, or if they do they already learned to turn it off because ssync ... or they get some errors while upgrading!
21:35:03 <clayg> mattoliverau: 🤮 I think timburke was sort of going that path, but with a config option - automatic fallback might have to produce warnings... cruft 🤔
21:35:43 <clayg> mattoliverau: but it's a good point!  if we're worried enough about the errors fallback to client network would work and shouldn't be much more painful than what we do know... we could probably clean it up eventually!
21:35:59 <timburke> yeah, it'd be doable -- but i'd worry about the extra complexity and knowing when we could feel safe removing it
21:36:00 <clayg> timburke: I'm not THAT ^ worried about it - YOU?
21:37:39 <clayg> mattoliverau: do you know anyone that deploys with replication_server = true?  (for a/c server; N.B. replication_server = true is currently *broken* for ec/ssync - which is more or less what we're trying to fix 🤷)
21:38:09 <timburke> yeah, i think i'm not so worried -- might be worth confirming that we're not going to flood logs with tracebacks or anything, but the upgrade window should be fairly short and it's only disrupting consistency engine work which is used to delays
21:38:12 <mattoliverau> I wonder if a fallback to the other network is ever really bad.. on link failures and congestion, swift will keep going. I guess we don't want to effect user traffic, and detecting when to fallback could get problematic.
21:38:15 <clayg> timburke: I think we also have "upgradeimpact: everything is going to replication network" on our side?  like you kind of have to really *want* to get in on this?
21:39:31 <mattoliverau> clayg: I can't think of anyone, but can look into our cloud product to see what we configure (we still have customers running openstack).
21:39:33 <timburke> i guess another option would be to get the respond-to-all-methods fix in now, then do the move-all-traffic-to-replication-network fix later
21:39:40 <clayg> mattoliverau: if I opt'd into replication networks it's probably for segregation, and I don't think I'd want errors to push over to client network
21:39:51 <clayg> mattoliverau: thanks!
21:40:43 <timburke> (though i'm also kind of happy that these piled up together to force us to acknowledge that the upgrade is a little tricky)
21:40:46 <clayg> timburke: i think they go together and we regularly introduce changes that will cause upgraded servers to not talk to old versions until they get upgraded 🤷
21:40:59 <mattoliverau> clayg: I guess, but self healing and things just working with warnings in the log could be cool :) But sure fair enough
21:41:57 <timburke> only other question i had was whether we should work toward having saio and probe tests *expecting* an isolated replication network
21:42:27 <clayg> i mean, it's not even the container-replicator... it's like the *sharder* and ... updater?  I think it's fine.
21:43:27 <timburke> reconciler, reaper, container-sync...
21:44:57 <timburke> currently, https://review.opendev.org/#/c/741723/ is a hack and a half -- there's literally a comment like `#  :vomit:` in there at the moment
21:44:58 <patchbot> patch 741723 - swift - wip: Allow probe tests to run with separate replic... - 2 patch sets
21:45:43 <zaitcev> container-sync cannot use replication network by definition, I think. It talks to _other_ cluster.
21:46:19 <timburke> zaitcev, true, it does -- but it uses internal-client to get the data to send
21:46:37 <clayg> 💡
21:46:51 <timburke> making probe tests tolerant of either config seems tricky; if we could assume it to be there, i think it could clean up better
21:47:58 <timburke> something to think about, anyway. i want to make sure clayg gets some time to talk about...
21:48:05 <timburke> #topic waterfall ec
21:48:18 <timburke> clayg, i saw new patches go up! how's it going?
21:48:28 <zaitcev> Coincidentally I wrote a little function that does not make the assumption  node_id == (my_port - 6010) / 10
21:48:33 <clayg> it's kinda stalled out waiting for feedback; the latest patch was just some minor "rename things"
21:49:06 <clayg> timburke: you had said something like "the per-replica timeout would make more sense if it started at 0 = first parity" or something like that - and that makes sense to me
21:50:04 <clayg> the fact that first (in replicated) or n-data (in ec) replicated timeouts are "ignored" is kinda dumb
21:50:35 <clayg> and I can't really imagine a path in the future where they'd be used... so... i just need to figure out how to express something like that in code
21:51:05 <timburke> i think it could also make it so the config won't strictly *need* to be per-policy, though it seems like you'd likely want to tune it differently for different policies
21:51:16 <clayg> under the hood the `get_timeout(n_replica)` function will probably still grab out of a list... but it'll either be sparce, or have to `+ offset`
21:51:56 <clayg> oh yeah; i hadn't thought about having global default... hrm
21:52:03 <clayg> so maybe that's where the next bit of work will be
21:53:09 <clayg> timburke: can you confirm you're leaning towards *further* decoupling (remove vistigial-ec from GetOrHeadHandler & remove vistigial-replicated from ECFragGetter) as much as possible - or do you see some path where they go back together?
21:53:26 <clayg> because that's like a 4th patch I haven't even started on
21:54:32 <clayg> I did go back and look at some of the EC tests that I "made more robust" and they seem to work (or fail in a reasonable way) with the old code; so I'm pretty confident in maintaining the status quo
21:54:33 <timburke> i see all of this (and the further cleanup that could be done now that they're separated) as a pretty convincing case that they should stay apart
21:55:13 <clayg> that is I think everything will work and continue to be just as robust post upgrade if you don't turn on the new timeouts (but I think people will want to turn on the timeouts; just like concurrent_gets)
21:55:42 <timburke> unless you've gotten a new picture of how they could come back together, anyway
21:55:57 <timburke> all right, last few minutes
21:56:02 <timburke> #topic open discussion
21:56:12 <timburke> anything else we should bring up today?
21:56:18 <clayg> ok, so I'll probably leave p 709326 and it pre-req alone; respin 737096 and work onto something ontop of that with more refactoring
21:56:19 <patchbot> https://review.opendev.org/#/c/709326/ - swift - Enable more concurrency for EC GET - 2 patch sets
21:56:36 <clayg> shrinking doesn't really work!  https://review.opendev.org/#/c/741721/
21:56:36 <patchbot> patch 741721 - swift - add swift-manage-shard-ranges shink command - 2 patch sets
21:56:59 <clayg> at least it's not working well for me when I try to do it manually without auto_shard = true (which is how my clusters are)
21:57:19 <mattoliverau> oh interesting
21:57:30 <clayg> so I really want to merge https://review.opendev.org/#/c/741721/ and get that deployed ASAP (it's mostly stealing all the fixes from tim and then putting a cli ontop of it)
21:57:31 <patchbot> patch 741721 - swift - add swift-manage-shard-ranges shink command - 2 patch sets
21:58:17 <clayg> also I really want to merge https://review.opendev.org/#/c/742535/ because until I have way to do manual shrinking I have a bunch of overlapping shard ranges and sometimes a bunch of listings get lost
21:58:17 <patchbot> patch 742535 - swift - container-sharding: Stable overlap order - 1 patch set
21:58:45 <clayg> i tried to keep both of those neat-and-tight cause I really want these fixes!
21:59:14 <mattoliverau> I'll fire up a sharding env and have a play with them.
21:59:55 <clayg> while you have a sharded db up check out https://review.opendev.org/#/c/737056/ too!
21:59:55 <patchbot> patch 737056 - swift - swift-container-info: Show shard ranges summary - 3 patch sets
22:00:09 <clayg> I think timburke and I agreed on that one now 🤞
22:00:16 <mattoliverau> kk :)
22:00:45 <timburke> yeah, that was definitely one of the things i noticed while messing with https://review.opendev.org/#/c/738149/ -- shrinking currently needs to be "online" and actively push new info to the shard that should be shrunk
22:00:45 <patchbot> patch 738149 - swift - Have shrinking and sharded shards save all ranges ... - 5 patch sets
22:01:02 <timburke> all right, looks like we're out of time
22:01:16 <timburke> thank you all for coming, and thank you for working on swift!
22:01:19 <timburke> #endmeeting