16:00:10 <smcginnis> #startmeeting Cinder
16:00:11 <openstack> Meeting started Wed Mar  8 16:00:10 2017 UTC and is due to finish in 60 minutes.  The chair is smcginnis. Information about MeetBot at http://wiki.debian.org/MeetBot.
16:00:12 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
16:00:14 <openstack> The meeting name has been set to 'cinder'
16:00:18 <Swanson> hello
16:00:23 <smcginnis> ping dulek duncant eharney geguileo winston-d e0ne jungleboyj jgriffith thingee smcginnis hemna xyang1 tbarron scottda erlon rhedlind jbernard _alastor_ bluex karthikp_ patrickeast dongwenjuan JaniceLee cFouts Thelo vivekd adrianofr mtanino yuriy_n17 karlamrhein diablo_rojo jay.xu jgregor baumann rajinir wilson-l reduxio wanghao thrawn01 chris_morrell stevemar watanabe.isao,tommylikehu
16:00:24 <geguileo> Hi! o/
16:00:26 <eharney> hi
16:00:28 <jungleboyj> Morning.
16:00:29 <smcginnis> mdovgal ildikov wxy viks ketonne abishop sivn
16:00:32 <xyang1> Hi
16:00:34 <cuhler> Hi
16:00:35 <hemna> o/
16:00:41 <viks> \o
16:00:45 <patrickeast> Hi
16:00:48 <tbarron> hi
16:00:53 <breitz> hi
16:00:54 <tommylikehu_> hi
16:01:23 <smcginnis> #topic Announcements
16:01:34 <smcginnis> #link https://etherpad.openstack.org/p/cinder-spec-review-tracking Review focus
16:01:34 <wxy> o/
16:01:46 <smcginnis> The review etherpad has been updated.
16:02:05 <smcginnis> Would be great to get those new driver contributors feedback earlier rather than later.
16:02:19 <smcginnis> Still waiting on CI for most of them though I think. Well, a couple.
16:02:36 <erlon> hey
16:02:39 <mlakat> hi
16:02:53 <gouthamr> hi
16:02:59 <Yogi1> Hello
16:03:08 <smcginnis> #topic Pike Goals
16:03:21 <smcginnis> There are a couple of community wide goals set for Pike.
16:03:30 <rarora> hi
16:03:42 <smcginnis> We're in pretty good shape to meet those goals from the Cinder side, just a few things to get in place.
16:04:02 <smcginnis> #info Goal #1: WSGI via Apache
16:04:17 <smcginnis> #link https://governance.openstack.org/tc/goals/pike/deploy-api-in-wsgi.html WSGI Goal
16:04:41 <karthikp_> o/
16:04:53 <smcginnis> I think the only thing not in place is that one of the requirements is to have devstack deploy Cinder-api that way.
16:05:17 <smcginnis> I put up a patch to switch devstack to do that here: https://review.openstack.org/#/c/441266/
16:05:27 <smcginnis> But I haven't had a chance to go back and see what I missed. :)
16:05:48 <smcginnis> So if anyone has interest and knowledge there, I'm sure there's something basic that I overlooked.
16:05:56 <eharney> does this mean that will be the only supported way to deploy Cinder?
16:06:01 <smcginnis> #link https://review.openstack.org/#/c/441266/ Patch to switch devstack to use Apache.
16:06:25 <mlakat> it's cinder-api only, right?
16:06:30 <smcginnis> eharney: I don't believe so. I think we can still deploy the way we are today, but this moves it in that direction.
16:06:36 <guyr-infinidat> we have a number of pending patches for the infinidat driver, we kinda need those merged in before we do implement the generic volumes api
16:06:41 <smcginnis> mlakat: Correct, this is just the cinder-api interface.
16:06:54 <smcginnis> guyr-infinidat: Great, but that has nothing to do with APache. :)
16:07:16 <smcginnis> #info Goal #2: Python 3 Support
16:07:23 <smcginnis> #link https://governance.openstack.org/tc/goals/pike/python35.html Python 3.5 support
16:07:37 <smcginnis> This is another one where we are pretty much there, but just missing a couple things.
16:07:53 <smcginnis> Mainly, having functional tests run using py35.
16:07:56 <hemna> any talk about switching over CI to python 3 ?
16:07:59 <guyr-infinidat> yeah I know sorry for jumping in late :)
16:08:11 <smcginnis> hemna: I don't want to go there yet, but we will need to at some point.
16:08:14 <smcginnis> guyr-infinidat: ;)
16:08:24 <hemna> or using ubuntu 16.x as the supported ubuntu (which uses python 3 by default)
16:08:35 <smcginnis> Our functional tests are actually failing right now, at least last I looked.
16:08:58 <smcginnis> hemna: It must be 16.04 now. 14.04 support was blocked in devstack by default now.
16:09:16 <smcginnis> That was one of several things causing the complete meltdown of our CIs over the last week or two.
16:09:35 <hemna> ok, isn't that mean that python 3 is the default?  It takes a bit of effort to get python 2.x working there....
16:09:36 <smcginnis> So I think e0ne had said he was working on fixing the functional tests.
16:09:44 <smcginnis> He's not here right now, so need to follow up.
16:10:02 <smcginnis> But I think once we have those working again, probably trivial to switch to using py35 to run them.
16:10:31 <smcginnis> hemna: No, even on 16.04 py27 is the default. But it is nice in that py35 is there by default.
16:11:33 <smcginnis> So to meet the Pike community goals, we are pretty close. Just a couple minor things to get in place and I think we can state Cinder has met those goals.
16:11:38 <jungleboyj> smcginnis: But pep8 runs with py3 by default in 16.04
16:11:51 <smcginnis> jungleboyj: Oh? I hadn't noticed. That's good.
16:12:14 <jungleboyj> Yeah, I pushed up a patch to fix stuff that was broken as a result.  It merged a while ago.
16:12:15 <smcginnis> The ultimate goal is to move completely to py3 and eventually discontinue py27 support.
16:12:30 <jungleboyj> So, we are good there.
16:12:35 <smcginnis> I think 2020 or something is the planned end of life for Python 2, so we want to get there before then.
16:12:41 <eharney> the gate pep8 job still runs on py27
16:13:02 <jungleboyj> eharney:  Yeah, but if you are using 16.04 locally, it didn't work for Cinder.
16:13:36 <smcginnis> OK, just wanted to make sure everyone was aware of the goals. Feel free to help in any way.
16:13:41 <smcginnis> #topic Reuse API v2 for v1 calls
16:13:47 <smcginnis> #link https://review.openstack.org/#/c/440794/ Refactoring patch
16:13:59 <smcginnis> This one discussed briefly at the PTG.
16:14:01 <jungleboyj> Can me make 2020?  ;-)
16:14:06 <smcginnis> But e0ne had some comments on there.
16:14:13 <smcginnis> I wanted to clarify what's going on with that.
16:14:24 <smcginnis> So we have deprecated API v1 for a while now.
16:14:27 <smcginnis> A long while.
16:14:56 <smcginnis> It sounds like we may finally be able to remove it soon. I'm hoping to be able to do that in Queens if we can get the buy in from folks.
16:15:05 <smcginnis> This is mainly people outside of Cinder.
16:15:09 <smcginnis> In the meantime..
16:15:25 <smcginnis> jgriffith pointed out that v1 and v2 were basically identical.
16:15:45 <smcginnis> There are just a few minor changes as far as which fields are returned and some 200 codes vs 202s.
16:16:06 <smcginnis> So rather than having all that duplicated code, we should be able to just reuse most of the logic.
16:16:14 <eharney> i just wonder if the risk of introducing behavior changes in v1 is worth the benefit of refactoring this if we're going to remove it soon anyway
16:16:16 <smcginnis> v2 is frozen as well. And now deprecated too.
16:16:46 <smcginnis> eharney: v2 is frozen, so if we introduce a behavior change, we've screwed up.
16:16:58 <mlakat> I would say let's not refactor something that will be deleted soon.
16:17:53 <smcginnis> I'd rather get rid of the code as much as possible. And honestly, at this point I don't care if a minor change sneaks through. But if we want to just wait for full removal, we can do that too.
16:18:17 * jungleboyj plays devil's advocate
16:18:39 <mlakat> sorry, just to put me in context: what would be the exact refactoring action?
16:18:42 <jungleboyj> Wouldn't we want to try the removal with v1 and see what happens rather than cutting off a whole limb taking v1 and v2 at once?
16:18:44 <hemna> jungleboyj, hail satan
16:19:01 <mlakat> make v1 and v2 use the same code?
16:19:01 <smcginnis> mlakat: https://review.openstack.org/#/c/440794/ Refactoring patch
16:19:04 * jungleboyj grows horns and starts shooting fire.
16:19:32 * mlakat whoops
16:19:36 <smcginnis> jungleboyj: Not sure I follow.
16:19:41 <hemna> mlakat, no, but the external interface is basically the same
16:20:27 <jungleboyj> smcginnis:  Oh, I am sorry, I am thinking wrong here because we won't take v2 out when we delete v1.
16:20:49 <Swanson> Whack em all at once.
16:20:52 <smcginnis> jungleboyj: Right. So when we remove v1 completely, it will just be a small patch.
16:21:16 <smcginnis> Or we leave it, carry around this extra duplicate code until we can remove it all.
16:21:39 <smcginnis> I just don't want to hold on to things with the expectation that something may or may not happen in the near future.
16:21:46 <jungleboyj> Well, I am all for getting the duplicate code out.
16:21:55 <smcginnis> We do that, then it's two years later and we're still talking about it. :)
16:22:06 <jungleboyj> smcginnis: ++
16:23:15 <smcginnis> Well, please comment on the patch with anything. At least I've stated the case. :)
16:23:25 <smcginnis> #topic Database Indexes
16:23:32 <smcginnis> mlakat: ALl yours.
16:23:41 <mlakat> yep, so we have seen slow performance on postgres
16:23:47 <smcginnis> #link https://review.openstack.org/#/c/437313/ Add Indexes for PostgreSQL
16:23:54 <smcginnis> #link https://bugs.launchpad.net/cinder/+bug/1671135
16:23:54 <mlakat> it turned out that indexes were missing on foreign keys.
16:23:55 <openstack> Launchpad bug 1671135 in Cinder "Model schema column type mismatches" [Undecided,In progress] - Assigned to Mate Lakat (mate-lakat)
16:24:10 <mlakat> So I added a change for that, and I think we had a nice discussion on the change.
16:24:16 * bswartz is here late (couldn't get IRC working over 4G today)
16:24:33 <mlakat> conclusion is: I'll add a test that runs on every migration test to make sure all foreign keys have indexes on each db backend.
16:24:47 <mlakat> Any comments on that?
16:25:10 <smcginnis> mlakat: So it will just check for indexes at the end of a migration and make sure they are in palce?
16:25:13 <smcginnis> *place
16:25:23 <mlakat> smcginnis, it'll be a test.
16:25:44 <jungleboyj> mlakat: Did you find where you would put that?
16:25:46 <mlakat> So if you submit a migration that adds a foreign key, but you do not take care of indexes, it'll break the gate?
16:25:46 <hemna> any thoughts on the idea of just automatically ensuring FK indexes at the end of cinder-manage db sync ?
16:26:29 <jungleboyj> hemna:  I was looking at db sync and I don't see that it does anything else like that.
16:26:43 <jungleboyj> hemna:  Doesn't mean we can't do it.  :-)
16:26:53 <eharney> is ensuring that all foreign keys have indexes actually a correct goal?
16:27:02 <mlakat> hemna, I still think that somehow violates the principles of versioned database migrations.
16:27:15 <jungleboyj> eharney:  Why would we not want that?
16:27:21 <hemna> mlakat, maybe.
16:27:33 <eharney> jungleboyj: will have to brush up on my db knowledge again and get back to you
16:27:37 <mlakat> mysql does that automagically
16:27:46 <hemna> but I think adding a test to ensure that the indexes are there after migrations are tested is probably just as good
16:27:52 <mlakat> eharney, keep me posted on your findings.
16:27:58 <jungleboyj> eharney: :-)  As far as I know it is just a performance improvement.
16:28:09 <hemna> eharney, too many indexed columns are a problem
16:28:20 <hemna> but not having any index on a FK is also a problem for performance
16:28:21 <mlakat> yeah, indexes have cost
16:28:35 <smcginnis> I do think pg should be the same as mysql.
16:28:49 <geguileo> smcginnis: +1
16:28:51 <mlakat> ok, then all fk must have an index.
16:28:57 <hemna> mlakat, +1
16:29:08 <mlakat> Ok, let me add a test first, and see where we get with that.
16:29:13 <mlakat> The other change
16:29:13 <smcginnis> There is some discussion going on right now of dropping pg support and mandating mysql, but that should not prevent us from doing anything for now to make sure they are consistent.
16:29:31 <hemna> whoa
16:29:33 <mlakat> https://bugs.launchpad.net/cinder/+bug/1671135 <- just something that I detected
16:29:33 <openstack> Launchpad bug 1671135 in Cinder "Model schema column type mismatches" [Undecided,In progress] - Assigned to Mate Lakat (mate-lakat)
16:29:44 <mlakat> https://review.openstack.org/#/c/443165/
16:29:46 <eharney> postgres is essentially untested in the gate now, right?
16:29:49 <hemna> isn't the pg license better for shipping it w/ OS ?
16:30:03 <smcginnis> eharney: Some projects do still have tests, but I think most do not.
16:30:13 <mlakat> Yeah, let's not open that can of worms now.
16:30:23 <smcginnis> mlakat: Right! ;)
16:30:30 <mlakat> Fact is: I think having those index checks wouldn't hurt.
16:30:32 <smcginnis> Just making sure folks are aware of that.
16:30:37 <mlakat> Ah yes.
16:30:43 <mlakat> I'm really interested where that is going.
16:30:45 <bswartz> -1 for dropping pg support
16:30:56 <mlakat> Yeah, that would be my view as well bswartz
16:31:03 <smcginnis> I agree.
16:31:06 <bswartz> it shouldn't be hard to to this for mysql and pg both
16:31:25 <bswartz> but not having pg gate tests means you risk breakage when you add the feature
16:31:34 <smcginnis> The reasons cited are lack of test coverage and the desire to use mysql specific optimizations, but I don't fully agree with that.
16:31:53 <smcginnis> But that's besides the point.
16:32:04 <smcginnis> I think starting with a test for these indexes would be good.
16:32:20 <jungleboyj> smcginnis: +1
16:32:28 <mlakat> So ladies and gentlemen, some mismatches have been found between models.py and the database schema, please check https://review.openstack.org/#/c/443165/
16:33:00 <smcginnis> mlakat: Thanks, I'll add to me list of tabs to go through.
16:33:10 <mlakat> Ok, that would be all from me, thanks for all the help on the changes and the comments. Hopefully new patch coming tomorrow
16:33:23 <smcginnis> mlakat: Great, thank you.
16:33:33 <jungleboyj> mlakat: Thank you for your efforts there!
16:33:35 <smcginnis> #topic Support regexp based filter
16:33:41 <tommylikehu_> thanks smcginnis
16:33:43 <smcginnis> tommylikehu_: Hey, you're up.
16:33:47 <tommylikehu_> I think this is a good one for cinder, also not sure we had discussed this before.
16:33:56 <smcginnis> #link https://review.openstack.org/#/c/442982/
16:34:17 <tommylikehu_> so any thoughts on this ?
16:34:46 <eharney> is there a reason to do full regexes and not something simpler like globs?
16:35:05 <smcginnis> Duncan makes a good point there. I wonder if anything could be done to mitigate that.
16:35:12 <eharney> globs :)
16:35:15 <jungleboyj> eharney: Do globs have less of a security impact ?
16:35:23 <jungleboyj> Ah ... Thank you.
16:35:36 <tommylikehu_> smcginnis: yes, I havn't noticed that when writing this
16:36:08 <tommylikehu_> smcginnis: maybe add some restrcits
16:36:53 <smcginnis> tommylikehu_: OK, good.
16:37:13 <eharney> i'm not sure you're going to have a way to really restrict regexes when these are being performed at the db layer
16:37:35 <tommylikehu_> eharney: not sure, trying to find
16:37:42 <eharney> there could also be issues with different db engines behaving differently...
16:37:51 <tommylikehu_> but in general I think this is good and flexible
16:37:55 <mlakat> eharney, exactly
16:38:03 <tommylikehu_> eharney:  mentioned in the spec
16:38:35 <eharney> well, different db engines giving different behaviors at the API is a problem
16:39:09 <mlakat> eharney, I think it's already happening?
16:39:24 <mlakat> See the thread on ML about postgres
16:39:33 <mlakat> The same can we closed :-)
16:39:53 <jungleboyj> mlakat: The can really wants to be opened.
16:39:58 <jungleboyj> ;-)
16:40:05 <mlakat> I know we'll need to do that.
16:40:17 <eharney> i also don't really get the idea of "use regexes, but fall back to LIKE if necessary", because those will take different syntaxes...
16:40:41 <tommylikehu_> eharney:  that is kind of restrict
16:41:34 <tommylikehu_> even eventually we only allow LIKE operation at the db level, it is a good start
16:42:16 <mlakat> is that a standard?
16:42:24 <eharney> some concrete use case examples would be interesting to understand the value of regexes over just adding wildcard support
16:42:59 <mlakat> starting with something simple makes sense.
16:43:11 <tommylikehu_> eharney: does anyone knows why nova introduced regexp as well as wildcard?
16:43:23 <tommylikehu_> mlakat: +1
16:43:34 <tommylikehu_> we can start from simple case
16:43:43 <smcginnis> Should be able to look at the git blame to find the patch that added it. Hopefully it would link to a spec with more detail.
16:43:45 <eharney> if Nova is using regexes in a similar fashion it would be good to note that in the references in the spec, i don't know what they do
16:44:24 <tommylikehu_> eharney: yes they do
16:44:35 <tommylikehu_> and a lot of project support this
16:45:17 <tommylikehu_> smcginnis: I wil add this in spec
16:45:19 <eharney> maybe it's all fine then... just note the precedents in the spec
16:45:27 <mlakat> if a lot of projects do this, maybe this is something that should be oslo-ed ?
16:46:28 <tommylikehu_> mlakat: not sure about that
16:46:46 <jungleboyj> Yeah, not sure that it would be appropriate for oslo-db.
16:47:00 <jungleboyj> I like the term 'getting oslo-ed'  :-)
16:47:13 <smcginnis> Caption Oslo. :)
16:47:32 <jungleboyj> smcginnis: Caption Oslo?  That is a new one.  :-p
16:47:47 <smcginnis> Hah! Captain.
16:47:50 * jungleboyj removes my cape and starts writing copations
16:48:01 <jungleboyj> *captions
16:48:06 <jungleboyj> I give up.
16:48:08 <smcginnis> All around failure. :)
16:48:31 <smcginnis> tommylikehu_: OK, I think that's how to move forward. If you can get references in there to other implementation, that will help.
16:48:41 <tommylikehu_> smcginnis: sure
16:48:45 <mlakat> +1
16:48:54 <jungleboyj> smcginnis:  +1
16:49:03 <tommylikehu_> please add comments if you have any other concerns
16:49:18 <smcginnis> #topic Open discussion
16:49:22 <xyang2> smcginnis: I have a question
16:49:32 <smcginnis> xyang2: Sure, what's up?
16:49:46 <xyang2> we are planning to remove an existing driver and replace with a new one in Pike
16:50:03 <xyang2> so if we remove the old driver, how do we fix bugs in it in Ocata?
16:50:09 <hemna> xyang2, not deprecate?
16:50:27 <xyang2> already deprecated
16:50:34 <xyang2> so to backport, we first need to fix in the master
16:50:35 <jungleboyj> Yeah, seems like you would want to deprecate first and then you would have something to backport fixes with.
16:50:44 <xyang2> if it is removed from master, how to fix it
16:51:03 <smcginnis> xyang2: Patches can be proposed directly to Ocata for situations like that.
16:51:20 <xyang2> smcginnis: ok, good
16:51:38 <jungleboyj> xyang2: Just please remember to make note as to why it isn't a cherry-pick in the commit message.
16:51:48 <smcginnis> xyang2: The main point of that policy is to make sure the current code has the fix before the old code. But in this case there is no current code, so it would just go right to the stable branch.
16:51:51 <xyang2> jungleboyj: ok, thanks
16:51:54 <smcginnis> jungleboyj: +1
16:52:28 <jungleboyj> smcginnis:  +1
16:52:39 <jungleboyj> xyang2:  Which driver?
16:52:46 <xyang2> VMAX
16:53:03 <jungleboyj> What is the replacement?
16:53:13 <xyang2> still VMAX:)
16:53:28 * jungleboyj is confused .
16:53:30 <xyang2> it will be written using REST API
16:53:31 <xyang2> instead of SMI-S
16:53:42 <jungleboyj> xyang2:  Oh ... That is good.
16:55:03 <xyang2> smcginnis: I'm good now
16:55:18 <smcginnis> xyang2: Thanks. Any have anything else?
16:55:27 <xyang2> smcgninis: all set
16:55:35 <smcginnis> guyr-infinidat: Do you have links to those patches you were looking for reviews for?
16:55:42 <guyr-infinidat> yes
16:55:54 <guyr-infinidat> 1 sec
16:56:09 <mlakat> expired
16:56:39 <guyr-infinidat> https://review.openstack.org/418451 https://review.openstack.org/#/c/422277/ https://review.openstack.org/#/c/436989/
16:57:26 <smcginnis> mlakat: ;)
16:57:39 <smcginnis> OK, just a couple minutes, but sounds like we're done.
16:57:42 <smcginnis> Thanks everyone.
16:57:50 <mlakat> thanks!
16:57:51 <tommylikehu_> thanks
16:57:58 <smcginnis> #endmeeting