15:01:46 <bswartz> #startmeeting manila
15:01:47 <openstack> Meeting started Thu Dec 22 15:01:46 2016 UTC and is due to finish in 60 minutes.  The chair is bswartz. Information about MeetBot at http://wiki.debian.org/MeetBot.
15:01:48 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
15:01:49 <bswartz> hello all
15:01:51 <tommylikehu_> Merry xmas
15:01:51 <openstack> The meeting name has been set to 'manila'
15:01:51 <cknight> Hi
15:01:56 <vponomaryov> hello
15:01:58 <tommylikehu_> hi
15:01:59 <ganso> hi
15:02:02 <tbarron> hi
15:02:04 <ravichandran> hi
15:02:09 <markstur> hi
15:02:16 <xyang1> hi
15:02:32 <bswartz> #topic announcements
15:02:46 <bswartz> We passed the driver proposal freeze this monday
15:02:58 <gouthamr> hello o/
15:02:59 <bswartz> it looks like we have 3 new drivers to consider this cycle
15:03:52 <bswartz> also due to the holidays I think we should cancel the meeting next week
15:04:05 <bswartz> I know at least 5 cores will be on vacation
15:04:15 <bswartz> and I'm guessing the rest will probably be too
15:04:19 <xyang1> I am on vacation too
15:04:31 <tommylikehu_> cool
15:04:31 <bswartz> okay so it's an easy decision
15:04:41 <bswartz> #info Dec 29 meeting cancelled
15:05:11 <bswartz> Next meeting should be Jan 5 then
15:06:01 <bswartz> I know some people will still be on vacation that week but with FPF coming up Jan 9 we need to meet that week
15:06:25 <bswartz> #agenda https://wiki.openstack.org/wiki/Manila/Meetings
15:06:31 <bswartz> #topic specs implementaion status
15:06:35 <ganso> bswartz: isn't FPF Jan 12th?
15:06:38 <vkmc> o/
15:06:42 <vkmc> hi hi
15:06:51 <bswartz> ganso "week of" Jan 9th
15:06:55 <ganso> bswartz: oh ok
15:07:31 <bswartz> personally I'd prefer we do freezes on mondays instead of thursdays but I didn't announce an exact date this cycle
15:07:51 <bswartz> so it can be thursday
15:08:11 <bswartz> okay let's talk about specs
15:08:24 <bswartz> thankfully we got our specs straightened out last week due to a last minute push
15:08:41 <bswartz> the bad news is that we have almost no time to review the implementations
15:08:55 <bswartz> and implementation of some specs is blocked by dependencies
15:09:27 <bswartz> the goal of the specs process was to limit what we focus on to conserve review time, but we're in danger of still having too much code to review
15:09:56 <bswartz> please prioritize review of high priority spec implementations over low priority ones
15:10:50 <bswartz> #link https://github.com/openstack/manila-specs/blob/master/priorities/ocata-priorities.rst
15:10:55 <bswartz> THOSE ARE HIGH PRIORITY SPECS
15:11:00 <bswartz> HOLD ON CAPS LOCK IS STUCK
15:11:12 <ganso> lol
15:11:14 <bswartz> that's better
15:11:21 <markstur> :)
15:11:26 <vponomaryov> =)
15:12:11 <bswartz> vponomaryov: do you have any changes related to the scenario tests specs that are waiting?
15:12:19 <vponomaryov> bswartz; no
15:12:20 <xyang1> bswartz: have you submitted any patches for eliminate race conditions?
15:12:31 <bswartz> xyang1: that one is next
15:12:35 <vponomaryov> bswartz: both changes I had has been merged
15:12:37 <xyang1> ok
15:12:51 <vponomaryov> s/has/have/
15:12:54 <bswartz> vponomaryov: so everything for ocata is done?
15:13:10 <vponomaryov> bswartz: according to time ranges, Iguess yes
15:13:17 <bswartz> okay
15:13:30 <bswartz> regarding race conditions, this is the next patch that needs to land: https://review.openstack.org/#/c/318336/
15:13:42 <bswartz> I'm working on that review
15:14:14 <bswartz> we need some others with tooz experience (other than tbarron and gouthamr, who are authors on that change)
15:14:32 <xyang1> do we have an etherpad that lists all the code patches associated with high priority specs?
15:14:46 <bswartz> xyang1: no, would that be helpful?
15:14:51 <xyang1> yes
15:15:07 <bswartz> do we already have an etherpad we can reuse?
15:15:11 * bswartz goes looking
15:15:30 <markstur> https://etherpad.openstack.org/p/manila-ocata-code-review-focus
15:15:58 <xyang1> we could add code patches to this spec
15:16:07 <tommylikehu_> ok
15:16:16 <bswartz> awesome
15:16:33 <tommylikehu_> it's not the high priority ones
15:17:07 <bswartz> yeah I need to add the specst hat merged last week at the top of this list
15:17:15 <bswartz> I won't take up our meeting time doing that though
15:17:57 <bswartz> okay access rules
15:18:03 <bswartz> gouthamr: any patches waiting for review?
15:18:27 <gouthamr> bswartz: nope.. i haven't pushed one of the patches and the parent patch needs an update too
15:18:44 <gouthamr> bswartz: so, not ready yet. hoping to get it in soon
15:18:48 <bswartz> okay and ipv6 is on my list for review
15:18:51 * bswartz finds link
15:19:26 <bswartz> #link https://review.openstack.org/#/c/312321/
15:19:44 <ganso> bswartz: are we going to discuss spec details in this meeting?
15:20:02 <bswartz> ganso: there shouldn't be any details to discuss...
15:20:12 <bswartz> ganso: oh you're referring to the proposed changes to mountable snapshots
15:21:01 <ganso> bswartz: not only that, there is one detail I don't fully agree in the access rules spec, we could discuss the possibility of a change
15:21:13 <ganso> bswartz: there was some discussion in the channel yesterday
15:22:02 <ganso> bswartz: we could come back to it after going through all the specs
15:22:04 <bswartz> okay well one of the goals of the specs process was to get all the design decisions out of the way before the spec merged -- so in theory we should leave the spec the way it is
15:22:14 <bswartz> however we do make mistakes
15:23:30 <bswartz> ganso: okay we can come back to that topic
15:23:40 <bswartz> regarding ipv6, any other patches looking for review?
15:24:16 <tommylikehu_> sure
15:24:32 <tommylikehu_> I will add these in the etherpad
15:25:38 <bswartz> okay
15:25:42 <bswartz> we can move on then
15:26:06 <bswartz> #topic 24 hour rule
15:26:11 <bswartz> tbarron: you added this?
15:26:22 <tbarron> Recently I was reminded of the manila "24 hour agreement" w.r.t. +A workflow.  I either hadn't been told it or forgot it, and it seems other cores were in my same boat, so it seemed we should briefly review it here.
15:26:37 <tbarron> just a quick statement for common understanding
15:26:52 <tbarron> vponomaryov: ^^
15:27:15 <gouthamr> 24 hour agreement?
15:27:22 <bswartz> this agreement isn't just for manila it's across the whole community right?
15:27:27 <xyang1> I have not heard about its before
15:27:29 <tommylikehu_> what's that
15:27:33 <ravichandran> whats that
15:27:53 <bswartz> maybe the 24-hour rule only exists in a few people's heads
15:27:55 <markstur> it's from vponomaryov's secret club rules
15:27:56 <vponomaryov> long time ago we agreed to not merge changes before 24 hours pass after its uploading
15:27:56 <tbarron> bswartz: seems to be something old-timers know and new-timers don't
15:28:24 <vponomaryov> with exception of merging blocker fixes
15:28:34 <cknight> tbarron: so you're saying you're a new-timer?
15:28:35 <xyang1> I don't think I am that new:)
15:28:41 <vponomaryov> the goal - allow people from various time zones to react
15:28:48 <tbarron> cknight: and old-old-timers forget
15:28:54 <cknight> xyang1: You're newer than tbarron :-)
15:29:04 <xyang1> am I?:)
15:29:21 <bswartz> yeah this is one of those "common courtesy" rules which is that since many of us are on strange time zones, in order to give everyone a chance to at least look at new patches we don't merge new things in less than 24 hours
15:29:29 <tbarron> xyang1: in OpenStack years you are my elder :D
15:29:35 <vkmc> is there documentation wrt reviewing in Manila (including this rules)?
15:29:40 <vkmc> maybe in some wiki
15:29:42 <tommylikehu_> 24-hours delay?
15:29:48 <xyang1> tbarron :)
15:29:56 <bswartz> it's mainly to avoid stuff slipping by, except for gate fixes
15:30:15 <ganso> is it for changes or patchsets?
15:30:27 <xyang1> I remember long time ago eharney had said something like this in Cinder
15:30:28 <bswartz> it's mostly about making sure everyone has a chance to look at changes if they want to
15:30:31 <vponomaryov> ganso: it is for "+A" button
15:30:49 <ganso> if it is for changes, a new patchset that changes drastically the code could get merged
15:30:51 <vponomaryov> ganso: and mostly was about commit
15:30:54 <gouthamr> only seen trivial fixes being merged within a short time.. is this a new concern here?
15:30:57 <bswartz> vponomaryov: I think the question is what about a change that's been up for a while, and then a new patchset comes in
15:31:20 <bswartz> are you expecting us to wait 24 hours after the last patchset? or just 24 hours since the change was created?
15:31:34 <gouthamr> i mean TrivialFixes usually merge within 1 or two patchsets, and without having to pass all third party CIs
15:31:43 <vponomaryov> bswartz: personally, I consider diff with previous PSs
15:31:54 <vponomaryov> bswartz: for other reviewers and my own
15:32:11 <bswartz> because lots of times a change has been up for a while, then someone reviews it and -1 and the author addresses the comments right away
15:32:30 <bswartz> then asks for +2 based on changes
15:32:31 <xyang1> vponomaryov: do you have a specific example that the patch was merged too quickly?  I think this could depends on what the patch is about
15:33:04 <vponomaryov> xyang1: recently - one of commits merged by tbarron
15:33:12 <vponomaryov> xyang1: only by him
15:33:24 <bswartz> ninja'd?
15:33:27 <vponomaryov> tbarron: no blame, just fact )
15:33:37 <ganso> vponomaryov: but that is breaking another rule mostly
15:33:39 <markstur> he named names
15:34:04 <gouthamr> okay tbarron, no gifts for you this year
15:34:05 <markstur> but this is how we learned about the mythical 24h rule
15:34:27 <vponomaryov> gouthamr: maybe he is the one who presents gifts? ))
15:34:28 <ganso> we may need to call the OpenStack Police Department
15:34:37 <tbarron> I don't remember the change, it didn't seem a big deal at the time :D
15:34:38 <markstur> vponomaryov: LOL
15:34:53 * gouthamr tbarron is the one who knocks
15:34:55 <tbarron> but I have not problem following the rule if we all know what it is
15:35:04 <vponomaryov> tbarron: it was the hand of destiny, now everyone know the RULE
15:35:11 <bswartz> tbarron: personally I think some judgement is needed
15:35:28 <bswartz> there are probably changes that this rule doesn't need to apply do, and some where it's VERY IMPORTANT
15:35:41 <xyang1> bswartz: +1
15:35:44 <vkmc> ganso, lol
15:35:48 <markstur> vponomaryov: can you re-arrange the letters in tbarron to spell santa?  No that'd be too obvious.
15:35:55 <xyang1> bswartz: I think it really depends on what change it is
15:36:08 <tbarron> as a matter of common sense I don't do +2+A on stuff that I think others should examine.
15:36:30 <vponomaryov> ok, everyone disagree with t he rule?
15:36:39 <vponomaryov> /disagree/agree/ =)
15:36:43 <markstur> I think it is good to talk about
15:36:43 <bswartz> the key thing to remember is that some people wake up every day and look at all the new patches, and if you merge something in less than 24 hours you might be denying those people a chance to register their opinion
15:37:07 <xyang1> vponomaryov: I think we have to see what patch it is
15:37:10 <bswartz> so our of courtesy to your fellow cores, don't be in a hurry to workflow new things
15:37:17 <gouthamr> +1 -> applies to TrivialFix as well?
15:37:27 <xyang1> vponomaryov: whether it is controversial or a simple change
15:37:51 <vponomaryov> gouthamr: it happens that commit called trivial is not indeed
15:37:56 <bswartz> this isn't a written-down rules it's more about courtesy
15:38:07 <bswartz> and I agree that "trivial" is a judgement call we might not agree on
15:38:30 <ganso> vponomaryov: if it is not Trivial it shouldn't have been merged as Trivial, the 24h rule is not the problem here
15:38:57 <vponomaryov> ganso: if we have 2 +2 made by mistake in 1 hour?
15:39:07 <vponomaryov> ganso: then it is
15:39:16 <ganso> vponomaryov: which patch is it
15:39:17 <vkmc> IMHO there is place for a lot of subjectivity to play in with this... what is considered trivial here?
15:39:36 <vkmc> why you directly don't impose something like... 2 +2 are needed and then the WF+1 should be done by a different core?
15:39:47 <bswartz> vkmc: the main criteria for a trivial fix is that everyone agrees that it's trivial
15:40:03 <markstur> or just tbarron
15:40:09 <bswartz> vkmc: I don't want to make our rules more complicated
15:40:12 <vponomaryov> vkmc: and to be able to agree or disagree I should catch this train first
15:40:14 <vkmc> bswartz, +1
15:40:26 <ganso> vkmc: that's already imposed
15:40:48 <ganso> vkmc: oh nevermind the different WF +1 is not imposed
15:41:02 <vkmc> ganso, np
15:41:08 <bswartz> okay so have we covered this topic enough?
15:41:10 <vponomaryov> and little clarification
15:41:31 <vponomaryov> not 2 +2, but 2 +2 from 2 different companies
15:41:37 <vponomaryov> at least
15:41:47 <tbarron> yeah, that one I know :D
15:41:49 <vkmc> yup, I knew about that rule :)
15:41:52 <bswartz> yeah that restriction only applies to me, cknight, and gouthamr currently
15:42:28 <bswartz> to prevent netapp from weilding unchecked power
15:42:59 <bswartz> okay I see one other topic was added to the agenda
15:43:06 <bswartz> #topic Manila CI Tempest Test Cases are Failing
15:43:14 <bswartz> ravichandran: what did you want to say about this?
15:43:15 <ravichandran> I am observing few CI tempest test cases are failing randomly . Just want to know whether anything changed in last 2 to 3 weeks related CI which requires individual driver changes.
15:44:26 <vponomaryov> ravichandran: which CI jobs are you talking about?
15:44:26 <bswartz> ravichandran: can you tell us which tests and/or link to some jobs that illustrate this?
15:44:40 <bswartz> stuff has definitely changed, but mostly fixes to make things better
15:44:48 <vponomaryov> ravichandran: we have lots of third-party CIs that fail
15:44:55 <ravichandran> HPE
15:45:31 <ravichandran> oh ok I will email the details
15:45:51 <vponomaryov> email? whom?
15:46:12 <vponomaryov> for such cases, just ask for help in manila chat
15:46:51 <xyang1> email to markstur:)
15:47:01 <ravichandran> :)
15:47:10 <markstur> xyang1: hey!
15:47:12 <vponomaryov> xyang1: he has IBM badge now ))
15:47:12 <bswartz> lol
15:47:18 <xyang1> :)
15:47:35 <bswartz> you can quit HP but you can never leave
15:47:51 <markstur> There was a devstack CI breakage that folks were fixing.  I have a note that says see IRC logs Dec 6.  Checking.
15:47:57 <vponomaryov> HPkudza? )
15:48:18 * markstur is wearing an HP hoodie and vest right now
15:48:52 <bswartz> http://i.kinja-img.com/gawker-media/image/upload/t3evzz4gbm7gql9u3x2t.gif
15:49:04 <vkmc> aahaha
15:49:22 <ravichandran> :)
15:49:30 <bswartz> okay hopefully we can get ravichandran's CI sorted out offline
15:49:33 <bswartz> #topic open discussion
15:49:47 <bswartz> anything else then for today?
15:50:01 <ganso> bswartz: are we going to discuss the access rules spec detail in the channel?
15:50:08 <bswartz> oh crap
15:50:12 <bswartz> yes I said we'd come back to that
15:50:19 <bswartz> let's cover it now
15:50:19 <markstur> ravichandran: http://eavesdrop.openstack.org/irclogs/%23openstack-manila/%23openstack-manila.2016-12-06.log.html
15:50:21 <bswartz> we have 10 minutes
15:50:40 <ganso> ok so, as some of you know, the mountable snapshot spec has a problem regarding access rules
15:50:53 <ganso> it was proposed with the old approach allow/deny driver_interfaces
15:50:56 <ganso> so I am reworking that
15:50:59 <markstur> ravichandran: If it broke back then. Some CIs had to move from "post-extra" to "test-config"
15:51:24 <ganso> while I was doing that, I noticed that the new spec proposed for access rules does not handle recovery mode anymore in scenarios other than driver restarts
15:52:00 <ganso> At Tokyo, one of our main motivations (and one of the main motivations for me to rewrite the old approach to the new update_access approach) is that we needed the recovery mode to fix invalid access rules in the backend
15:52:13 <gouthamr> markstur ravichandran: careful with that change though, you only need to do that on "master" and not stable branches
15:52:21 <bswartz> ganso: we have a long-pending topic about ensure_share and general cleanup actions
15:52:27 <ganso> in my opinion, restarting a service to fix access rules in a backend is too drastic
15:52:48 <ganso> bswartz: I think this is unrelated to ensure_share, at least considering the current scope of that
15:53:05 <bswartz> ganso: IMO it's important that driverse have the capability to correct access errors whether we use it or not
15:53:15 <tbarron> ganso: can you remind us why some backends need recovery mode?
15:53:23 <tbarron> ganso: and how it works?
15:53:31 <bswartz> ganso: mostly I'm thinking about when a manager service is killed while modifying an access rule
15:53:56 <bswartz> ganso: upon restarting, we should easily be able to detect that that happend
15:54:18 <bswartz> ganso: and it's important that we can roll-forward the pending access change
15:54:25 <ganso> tbarron: if a backend cannot add/delete an error-ed rule for some reason
15:55:19 <tbarron> ganso: so it flushes all rules and rebuilds from the DB?
15:55:22 <bswartz> ganso: or at the very least, when the *next* access rule change comes, it should cause the access rules for that share to be rationalized
15:55:36 <markstur> If I try to create an access rule and it fails. Then I have an error status. Then an attempt to delete it, does "recovery"/"resync".
15:55:42 <ganso> tbarron: yes
15:55:59 <tbarron> ganso: so we have two use cases then, yours and bswartz's
15:56:03 <bswartz> guys I'm not talking about cases where bugs cause the rule modifaction to fail outright
15:56:17 * gouthamr is in a standup meeting.. will react to scrollback in a bit
15:56:27 <bswartz> I'm talking about the case where we don't know if the operation completed or not because the service simply died while the thread was still down in the driver
15:56:56 <tbarron> bswartz: yeah, I agree that's a real use case.  Also a driver might for its own reasons want a full refresh.
15:57:46 <ganso> tbarron: we add and remove rules while we don't exactly know what is in the backend, we believe it is always in sync with the manila DB, but it may run out of sync
15:57:47 <tbarron> with the new spec, the DB state is fully set in the API service.  So full state (what is there, plus any pending operations) could be pushed to the driver.
15:58:09 <tbarron> ^^ when there is a need
15:58:14 <bswartz> yes that's the main point of the driver interface change
15:58:18 <tbarron> not for routine updates
15:58:27 <ganso> tbarron: yes, we need to define triggers for that. According to the spec, the only trigger is a driver restart
15:58:36 <ganso> tbarron: s/driver/service
15:58:40 <tbarron> driver could return a code to manager asking for a refresh.
15:58:44 <bswartz> if we always tell the driver the complete set of rules, and the driver is able to push those to the backend, it becomes impossible to get out of sync
15:59:17 <bswartz> the first if is easy -- we've done it
15:59:26 <bswartz> the second if depends on the backend APIs and the driver code
15:59:30 <ganso> bswartz: but the implementations follow one of two paths: they don't look at the full list if they receive a rule to add or delete
15:59:46 * vponomaryov thinks why we still don't have API to force access rule recheck...
15:59:50 <ganso> bswartz: so they don't know if they have run out of sync
16:00:28 <bswartz> ganso: we added an optimization path for drivers where it's extremely slow or expensive to get the current access rule list *cough* ceph *cough*
16:00:50 <gouthamr> time check
16:00:52 <bswartz> however all drivers are required to implement correct behavior
16:01:00 <bswartz> yeah we're past our time
16:01:07 <bswartz> ganso: did we answer your question?
16:01:12 <ganso> bswartz: not really
16:01:14 <gouthamr> is ready to answer ganso on #openstack-manila
16:01:24 <bswartz> okay let's head back to our channel and sort it out there
16:01:35 <bswartz> thanks all and have safe a happy holidays!
16:01:47 <bswartz> #endmeeting