14:00:03 <rosmaita> #startmeeting cinder
14:00:03 <rosmaita> #link https://etherpad.openstack.org/p/cinder-victoria-meetings
14:00:03 <rosmaita> #topic roll call
14:00:04 <openstack> Meeting started Wed Jul  1 14:00:03 2020 UTC and is due to finish in 60 minutes.  The chair is rosmaita. Information about MeetBot at http://wiki.debian.org/MeetBot.
14:00:05 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
14:00:07 <openstack> The meeting name has been set to 'cinder'
14:00:19 <tosky> o/
14:00:28 <walshh_> hi
14:00:35 <jungleboyj> o/
14:00:55 <whoami-rajat> Hi
14:00:59 <e0ne> hi
14:01:31 <rosmaita> hello everyone
14:01:32 <rosmaita> #link https://etherpad.openstack.org/p/cinder-victoria-meetings
14:01:35 <geguileo> hi! o/
14:02:04 <jungleboyj> Happy Wednesday!
14:02:12 <rosmaita> specs freeze in a few hours, so that's what we'll be discussing mostly today
14:02:18 <rosmaita> but first some announcements
14:02:25 <rajinir> o/
14:02:33 <rosmaita> #topic updates - mid-cycle
14:02:44 <rosmaita> thanks to everyone who participated last week
14:02:53 <rosmaita> if you missed it, you can catch up on youtube
14:03:14 <rosmaita> though abishop said he was going to wait until after part 2 and then binge watch
14:03:20 <rosmaita> so, whatever works for you
14:03:21 <LiangFang> o/
14:03:30 <rosmaita> #link https://wiki.openstack.org/wiki/CinderVictoriaMidCycleSummary
14:03:39 <lseki> o/
14:03:40 <rosmaita> you can find a link to the recording ^^
14:03:40 <jungleboyj> :-)
14:03:58 <rosmaita> #topic updates - monthly video meeting poll
14:03:59 <smcginnis> o/
14:04:09 <rosmaita> #link https://rosmaita.wufoo.com/forms/monthly-video-meeting-proposal/
14:04:57 <rosmaita> i noticed that there were some communication difficulties at the mid-cycle, so the poll includes an option about whether we should really do one meeting a month in video or not
14:05:20 <rosmaita> but if we commit to taking good notes in irc, i think it's worth a try
14:05:34 <rosmaita> anyway, the proposed first video meeting would be 29 July
14:05:41 <rosmaita> so we have a bit of time to think about this
14:05:56 <rosmaita> anyway, the poll closes 1200 UTC on Thursday 9 July
14:06:19 <rosmaita> #updates - brocade FCZM driver
14:06:25 <rosmaita> oops
14:06:36 <rosmaita> #topic updates - brocade FCZM driver
14:07:00 <rosmaita> i sent out an email summarizing what we discussed at the mid-cycle
14:07:05 <rosmaita> #link http://lists.openstack.org/pipermail/openstack-discuss/2020-June/015692.html
14:07:12 <rosmaita> so, we'll see if anyone cares
14:07:30 <rosmaita> #topic updates - need to keep an eye on the cinder-plugin-ceph-tempest job
14:07:41 <rosmaita> this was really bugging me yesterday, but maybe it's a fluke
14:07:46 <tosky> rosmaita: master or previous branches?
14:07:51 <rosmaita> we made the job voting on 11 march this year
14:07:55 <rosmaita> tosky: both
14:07:56 <tosky> there is an open review that should fix the issues
14:08:09 <rosmaita> #link https://zuul.opendev.org/t/openstack/builds?job_name=cinder-plugin-ceph-tempest
14:08:16 <rosmaita> tosky: that's good news
14:09:07 <rosmaita> #topic updates - code coverage job
14:09:37 <rosmaita> i finally followed up on my PTG action to put up a job so you can have the code coverage report in groovy html on each patch
14:09:47 <rosmaita> #link https://review.opendev.org/#/c/738687/
14:10:00 <rosmaita> (it still needs reviews)
14:10:13 <rosmaita> right now, all it does is post the coverage output for you to look at
14:10:56 <rosmaita> we could add something that would pass/fail based on coverage falling past a threshhold
14:11:03 <rosmaita> e0ne is in favor of that
14:11:11 <whoami-rajat> #link https://6d4b1c17bd5712b1da1e-24da3718548989a700aa54b9db0ff79c.ssl.cf2.rackcdn.com/738687/8/check/cinder-code-coverage/7bb5c35/cover/
14:11:13 <whoami-rajat> looks great
14:11:37 <e0ne> I know that not everybody agrees with me, so I put +1 on that patch
14:12:15 <smcginnis> Putting a pass/fail on coverage is a hard thing to do right.
14:12:28 <smcginnis> I've been in endless debates about what the right amount of coverage should be.
14:12:29 <jungleboyj> This is cool.  Good info at least.
14:12:38 <rosmaita> yeah, it's a start
14:12:56 <rosmaita> i'll think about how we could track the coverage to address e0ne's concern
14:13:17 <rosmaita> my worry is that i don't want it to be non-voting and turn into the pylint job
14:13:29 <rosmaita> that's always red and so you don't notice anymore
14:13:44 <e0ne> rosmaita: +1
14:14:13 <smcginnis> Alternatively, we could just add coverage as part of the base commands that are run so it's always available for the normal UT runs.
14:14:56 <e0ne> rosmaita, smcginnis: we can use --fault-unnder (https://coverage.readthedocs.io/en/coverage-5.1/cmd.html) option
14:15:37 <rosmaita> let's discuss on the patch, or in a few meetings
14:15:49 <rosmaita> i think for now just having the info available is good
14:16:06 <e0ne> Agree. it's good for the beginning
14:16:11 <rosmaita> that's it for updates, unless anyone else has an announcement
14:16:13 <jungleboyj> ++ Starting by having it out there is a good step.
14:16:42 <jungleboyj> Don't want the voting question to stop that.
14:16:59 <rosmaita> yeah, let's make it a separate discussion
14:17:14 <rosmaita> #topic minor driver refactoring
14:17:28 <e0ne> rosmaita: thanks for raising this up!
14:17:30 <rosmaita> #link https://review.opendev.org/#/c/656888/
14:17:37 <rosmaita> e0ne: thanks for doing it!
14:17:46 <tosky> e0ne: --fault-under is not good IMHO, it should be a delta
14:17:48 <e0ne> a minor change with a huge diff
14:17:56 <rosmaita> ok, what's going on is that e0ne did some refactoring
14:18:08 <rosmaita> and, it turns out that he didn't have to modify a lot of unit tests
14:18:12 <rosmaita> because they weren't there
14:18:21 <rosmaita> so my point in bringing this up is ...
14:18:39 <rosmaita> if you are a driver maintainer, look at that patch ^^ and check your driver
14:18:56 <rosmaita> and you might want to think about a follow up patch adding a test or two
14:19:05 <rosmaita> there are some sample tests on that patch
14:19:34 <e0ne> just for the note: I'm not going to add unit-tests for drivers for this patch or as a follow up patch
14:19:40 <rosmaita> the main thing is, we will review the patch by hand, but it's easy to miss something
14:19:45 <rosmaita> e0ne: good note
14:19:54 <rosmaita> yeah, this is a driver maintainer's responsibility
14:20:08 <rosmaita> so please take a look
14:20:26 <smcginnis> ++
14:20:31 <rosmaita> but do it soon, like within the next few hours
14:20:57 <rosmaita> because it looks good, and we will merge it soon so e0ne isn't in merge conflict hell
14:21:14 <e0ne> :)
14:21:40 <rosmaita> #topic victoria spec review
14:22:03 <rosmaita> ok, i got sidetracked on some stuff, so my knowledge of the state of specs is from last night
14:22:10 <rosmaita> but here we go
14:22:34 <rosmaita> #topic spec - Remove quota usage cache
14:22:42 <rosmaita> #link https://review.opendev.org/#/c/730701/
14:22:53 <rosmaita> Sam left some performance info on the spec, i think
14:23:24 <rosmaita> looks like Lucio's objection is formatting
14:23:38 <rosmaita> (which is fine, we want to be able to read the thing)
14:24:40 <rosmaita> so, at a glance, it looks like getting the usage dynamically isn't too big a hit
14:24:52 <jungleboyj> Looks like the performance impact is sufficiently minimal though.
14:25:26 <rosmaita> i suspect there will be some weird things that we won't know about until someone actually tries to implement this
14:25:41 <rosmaita> so probably no point trying to work them out in advance
14:25:42 <eharney> IMO we should look at our db schema and see if we have indexes etc. there that would make this perform as well as possible
14:26:01 <rosmaita> eharney: that is a good point, please add it as a comment to the spec
14:26:30 <rosmaita> yeah, good indices are key
14:27:19 <rosmaita> so, the author is in australia, and i think today is almost over there
14:27:55 <rosmaita> i think it would be ok to stretch the deadline for him to attend to some changes
14:28:14 <rosmaita> i think the mood here is mostly positive toward this spec?
14:28:22 <e0ne> rosmaita: +1
14:28:49 <jungleboyj> +1
14:29:33 <rosmaita> i guess this spec-freeze is a bit early relative to the ptg, but we can at least weed out the no-chance specs
14:29:56 <rosmaita> ok, i'll put a note that he can revise and it will still be acceptable until the next cinder meeting
14:30:13 <rosmaita> #topic specs - Support modern compression algorithms in cinder backup
14:30:23 <rosmaita> #link https://review.opendev.org/#/c/726307/
14:30:45 <rosmaita> this one is kind of in the same boat, revision wise
14:31:04 <rosmaita> my feeling is that we should update our compression algo options
14:31:28 <rosmaita> but to what ... i think i agree with eric that we should just pick 1 of the 2 options
14:31:36 <rosmaita> as to which one ...
14:31:40 <eharney> yeah
14:31:50 <rosmaita> we're going to have to get this past the requirements team
14:31:58 <eharney> i haven't done much looking into which would be a better choice
14:32:16 <rosmaita> so i think in preparing the info for the patch for global-requirements, we can decide which is best
14:32:22 <eharney> i assume any would pass the requirements concerns
14:32:26 <e0ne> what about make these things plugable?
14:32:59 <eharney> they are
14:33:11 <eharney> but we don't want to give people a bunch of extra options just because we can, and have to maintain that forever
14:33:39 <rosmaita> i like zstandard better, but i'm not sure it's packaged as widely as the other option
14:34:06 <rosmaita> i think packaging is an issue for inclusion in global-requirements
14:34:34 <rosmaita> anyway, the author already has a patch up adding the algos to cinder
14:34:42 <rosmaita> so either one will be an easy code change
14:34:53 <rosmaita> it's really a matter of working out which is best
14:35:23 <rosmaita> so i think give him another week to get an argument together, and onto the spec?
14:35:57 <rosmaita> any objection?
14:36:08 <e0ne> eharney: actually, all algorithms and compressors are hard-coded
14:36:35 <e0ne> we can make it extendable like we did for db provider
14:36:42 <eharney> why?
14:37:03 <e0ne> so end-users will be able to add out-of-tree compressing algos to use it with cinder
14:37:21 <eharney> and then end up with backups that can only be consumed by certain versions of cinder on certain clouds on certain platforms..
14:37:50 <smcginnis> ^
14:37:53 <eharney> i don't see a benefit to justify that mess
14:37:55 <e0ne> goor point
14:38:08 <rosmaita> yeah, let's keep these as 2 separate issues
14:38:14 <jungleboyj> ++
14:38:23 <rosmaita> so for now, we can say let's add a modern compression algo
14:38:38 <rosmaita> and we can discuss later whether the plugin approach makes sense
14:38:44 <geguileo> would it make sense to update the default compressor?
14:39:03 <geguileo> if we are going to add it to the requirements already and it won't be optional
14:39:33 <rosmaita> no, i think gzip is a venerable compression algo and we can honor it by keeping it the default
14:39:43 <geguileo> lol
14:40:11 <rosmaita> also, it's a standard library algo
14:40:12 <e0ne> geguileo: we should not break current deployments with rolling upgrades
14:40:49 <jungleboyj> It seems like this is a new feature that people can use if they wish.
14:40:56 <rosmaita> #topic specs - Reset state robustification
14:41:00 <geguileo> e0ne: that's a very good reason
14:41:08 <rosmaita> #link https://review.opendev.org/#/c/682456/
14:41:14 <jungleboyj> Mmmm robustification
14:41:25 <e0ne> geguileo: that's why we still have tgt as a default option :(
14:41:37 <rosmaita> i think this is good, the open question is how to handle the --force option
14:41:40 <geguileo> e0ne: ouch
14:41:57 <eharney> i prefer just making the API safe and leaving the --force cases to cinder-manage
14:42:31 <rosmaita> i am ok with that
14:42:59 <rosmaita> eharney: why don't you revise the spec and people can comment, you can have a 1 week extension too
14:43:16 <eharney> ok
14:43:18 <rosmaita> #topic specs - Default volume type overrides
14:43:27 <rosmaita> #link https://review.opendev.org/#/c/733555/
14:43:48 <rosmaita> this one had 2 +2s, i just left it open so people could still comment
14:43:58 <whoami-rajat> it looks perfect now
14:44:25 <whoami-rajat> thanks geguileo for the quick update
14:44:41 <geguileo> np
14:44:54 <rosmaita> ok, i will re-read and we can get this one approved before the deadline
14:44:55 <rosmaita> yay!
14:45:11 <rosmaita> topic specs - Backup Backends Configuration
14:45:20 <rosmaita> #link https://review.opendev.org/#/c/712301/
14:45:26 <rosmaita> this one looks good too
14:45:49 <e0ne> thanks everybody for your feedback on this!
14:46:14 <rosmaita> looks like e0ne has revised, i think we can get this approved in the next hour or so
14:46:29 <rosmaita> #topic specs - Support revert any snapshot to the volume
14:46:30 <e0ne> :)
14:46:40 <rosmaita> #link https://review.opendev.org/#/c/736111/
14:47:05 <rosmaita> smcginnis: thanks for your comment on there, i think you explained clearly why we want this to have a generic implementation
14:47:25 <rosmaita> that can be overriden by backends that can do better
14:47:31 <eharney> i think this spec needs more thought on the implications of doing this... it seems like a minefield of usability and perf problems
14:47:48 <smcginnis> I hate to be negative about it, because I do think it would be very useful. But there are many issues with trying to do this for all backends.
14:48:01 <eharney> it also doesn't explain how it actually works
14:48:09 <smcginnis> Very light on details.
14:48:27 <smcginnis> The work item list of "change restriction" is a little vague. :)
14:48:29 <rosmaita> ok, i think this one we ask them to continue working on for W
14:48:38 <rosmaita> smcginnis: i think W has a name?
14:48:48 <smcginnis> Wombat IIRC.
14:49:00 <rosmaita> glad i asked, i was thinking Wallaby
14:49:17 <rosmaita> ok, i will leave an encouraging comment on the spec
14:49:26 <smcginnis> Oh, maybe you're right.
14:49:31 <jungleboyj> I thought it was Wallaby.
14:49:34 <smcginnis> I really should know since I ran it, but now I forget.
14:49:36 <jungleboyj> I am pretty sure it is.
14:49:44 <smcginnis> Yeah, you're all right. Don't listen to me. :)
14:49:45 <eharney> this needs some details on what actually happens on a typical backend that can do this in the "ideal" way
14:49:46 <jungleboyj> Because that was the one I liked.
14:49:57 <smcginnis> eharney: ++
14:49:58 <rosmaita> i was so devastated by my V name defeat, that i can't remember anything anymore
14:50:24 <eharney> because it's very unclear to the newer snapshots if you revert to an old snap
14:50:29 <eharney> unclear what happens to*
14:50:32 <jungleboyj> You weren't Victoriaous ?
14:50:44 <rosmaita> jungleboyj: :P
14:51:07 <smcginnis> eharney: I know at least for some it's an internal update of a pointer, but that's definitely not how all storage works.
14:51:13 <jungleboyj> This feels like one of those proposals that is a good idea but ends up being a dangerous minefield upon implementation.
14:51:19 <rosmaita> eharney: keep your thoughts coming, i will pull them out of the meeting log and put them on the spec
14:51:27 <rosmaita> ok, one final spec
14:51:40 <rosmaita> #topic specs - volume list query optimization
14:51:50 <rosmaita> #link https://review.opendev.org/#/c/726070/
14:52:01 <rosmaita> i think you may have seen my note to the ML
14:52:10 <rosmaita> i think this is really a bug
14:52:18 <rosmaita> or actually a set of 3 or so bugs
14:52:51 <rosmaita> but, it may still be worth a spec
14:52:53 <eharney> i think rosmaita teased out some good details on this that i wasn't able to quite grasp when we last discussed this
14:53:01 <rosmaita> becuase the fix will have to be in 2 places
14:53:04 <eharney> but i haven't followed up enough on picking through this yetr
14:53:05 <eharney> yet*
14:53:10 <rosmaita> rest api layer and also the db layer
14:53:38 <rosmaita> but i don't think it's going to require a mv change
14:54:55 <eharney> i would suggest not calling this an "optimization"
14:55:08 <rosmaita> yes, the title needs to change
14:55:31 <rosmaita> it's basically "correct the volume-list filtering"
14:55:42 <rosmaita> and maybe "correct the volume list"
14:56:13 <rosmaita> and maybe "never, ever do this kind of change again where we hijack the volume status to have internal states that aren't displayable in the rest api"
14:56:23 <eharney> lol
14:57:22 <rosmaita> i think maybe i will write this up as a bug, and tell the author to feel free to pick up the bug?
14:58:22 <jungleboyj> Two Minute warning.
14:58:26 <eharney> sensible enough
14:58:39 <rosmaita> ok, works for me
14:58:53 <jungleboyj> ++
14:58:57 <rosmaita> ok, cool, we got through all the non-WIP specs
14:59:05 <rosmaita> #topic open discussion for 60 seconds
14:59:10 <jungleboyj> *celebration*
14:59:25 <tosky> regarding the cinder-plugin-ceph-tempest test, this patch may help on train and older branches, but it needs to go through master and ussuri first: https://review.opendev.org/#/c/738273/
14:59:29 * tosky was ready
14:59:37 <rosmaita> ty tosky
15:00:11 <rosmaita> already has a +2!
15:00:16 <rosmaita> ok, gotta go
15:00:19 <rosmaita> #endmeeting