17:00:48 <alaski> #startmeeting nova_cells
17:00:52 <openstack> Meeting started Wed May 25 17:00:48 2016 UTC and is due to finish in 60 minutes.  The chair is alaski. Information about MeetBot at http://wiki.debian.org/MeetBot.
17:00:54 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
17:00:57 <openstack> The meeting name has been set to 'nova_cells'
17:01:07 <melwitt> o/
17:01:09 <doffm> \o/
17:01:33 <alaski> small crowd, but let's get going
17:01:39 <alaski> #topic Testing
17:01:40 <dansmith> o/
17:01:55 <alaski> no cellsv1 breaks that I know of
17:02:18 <alaski> auggy has kindly offered to work on cellsv2 testing
17:02:37 <alaski> she was researching what it would take to get a grenade job going on that
17:02:52 <doffm> Thanks auggy.
17:03:06 * bauzas waves a bit late
17:03:17 <alaski> one point of interest that came up was whether we encourage deployers to migrate as part of an upgrade, or after
17:03:39 <alaski> and should grenade test M->N plus migrate, or N->N plus migrate
17:04:10 <alaski> I leaned towards thinking that deployers will want to do it in two steps
17:04:17 <bauzas> yup
17:04:22 <alaski> so they would migrate to N, then at some later point migrate to cells
17:04:28 <melwitt> that's my thought as well
17:04:38 <bauzas> that depends on if we want to support deployers not yet using cells v2
17:04:54 <alaski> bauzas: so far I've been working under that assumption
17:05:31 <bauzas> so, the question is : should we accept to have operators deploying Newton without using cells v2 ?
17:05:34 <alaski> and I don't think there's anything to prevent migrating to cells during an upgrade, but that won't be the tested/recommended path
17:06:05 <alaski> bauzas: yes, I think so
17:06:12 <bauzas> and then, the correlated question is: if we accept Newton deployments without cells v2, should we deprecate it for Ocata then ?
17:06:19 <woodster_> o/
17:06:24 <doffm> Yes. We don't want that to go on too long.
17:06:27 <alaski> I would like to put a hard cut off for O
17:06:38 <bauzas> okay, so the point is
17:06:46 <alaski> i.e. must migrate to cellsv2 before upgrade to O
17:07:15 <bauzas> okay, so we answered the question you had
17:07:16 <doffm> Absolutely. We (developers) are at some point going to make assumptions that cellsv2 is set up.
17:07:36 <bauzas> ie. grenade for M-N and then N->N + cells V2
17:07:39 <alaski> bauzas: yep. we all seem to be on the same page
17:08:06 <bauzas> saying that at the GA for Ocata, all the deployers will be using cellsv2
17:08:13 <alaski> doffm: that will make some code much easier, right now I have conditionals everywhere
17:08:32 <doffm> The migration code is basically all conditionals.
17:08:33 <bauzas> (ie. at least running the main command)
17:08:53 <bauzas> I mean the one-for-all command
17:09:01 <alaski> yep
17:09:09 <bauzas> okay, I'm fine
17:09:15 <alaski> the assumption I would like to make is that instance and host mappings are all set up
17:09:47 <alaski> cool, let's move on
17:09:51 <alaski> #topic Open Reviews
17:09:53 <alaski> https://etherpad.openstack.org/p/newton-nova-priorities-tracking
17:10:10 <alaski> be better than me and try to keep that updated :)
17:10:15 <doffm> I haven't put it on the etherpad. (I will) but bauzas had some questions about: https://review.openstack.org/#/c/296597
17:10:36 <doffm> Basically I think there is a race condition when migrating and updating at the same time.
17:10:48 <doffm> I'll put additional comments up, but would like feedback on how best to proceed.
17:10:49 <bauzas> yeah it was my question
17:11:13 <alaski> okay, I will need to read through it
17:11:16 <dansmith> I haven't looked, but will
17:11:17 <dansmith> either way,
17:11:35 <dansmith> admin-only operations that could be racy without damage during an upgrade don't concern me as much
17:11:46 <doffm> No, I made that point, no damage caused.
17:11:47 <dansmith> if it's something a user can do and have issues, that's a probem
17:11:53 <alaski> good point
17:11:55 <doffm> And operator only.
17:11:56 <bauzas> good point yeah
17:12:01 <dansmith> but an admin can follow renos and not do things like that during an upgrade
17:12:12 <dansmith> or at least, we can blame an admin who doesn't :P
17:12:15 <bauzas> given the aggregate API is an admin one, then please nevermind my question
17:12:22 <bauzas> because I agree with dansmith
17:12:24 <doffm> Ok.
17:12:29 <alaski> I still want to take a look, but I agree
17:12:36 <dansmith> yep, and I will
17:12:47 <dansmith> we don't want to make too many of those kinds of excuses,
17:12:51 <bauzas> meaning that only operators modify the aggregates, so they should know when they migrate
17:12:55 <dansmith> but weigh the relative payoff vs. the complexity
17:13:22 <alaski> yeah
17:13:30 <doffm> I just have to make sure that all the update operations actually fail, rather than write to wrong db successfully.
17:13:44 <doffm> I think that is the case.
17:13:49 <doffm> Just have to make sure.
17:13:59 <alaski> if we could have an easy check of "migration has started but not completed so all actions fail momentarily" that seems good. but anything more complex may not be worth it
17:14:19 <dansmith> like flavors
17:14:24 <alaski> right
17:14:28 <dansmith> anything in api db means no updates
17:14:34 <dansmith> er, anything in either, means no updates
17:15:00 <doffm> I can add the 'ensure_migrated' check to update ops.
17:15:02 <alaski> yeah.
17:15:08 <alaski> cool
17:15:30 <alaski> #topic Open Discussion
17:15:31 <bauzas> cool for me yeah, we could just say "sorry dude, you should know why you have this exception"
17:15:52 <auggy> o/ hi!
17:16:05 <alaski> hi!
17:16:11 <bauzas> aïl
17:16:18 <auggy> for the grenade test, i have been updating the testing etherpad and am currently working on setting up a target devstack
17:16:26 <alaski> awesome
17:16:28 <auggy> so i will probably have questions ;)
17:17:01 <melwitt> I proposed the devstack transport_url patch https://review.openstack.org/320746 before seeing that sileht had one up already https://review.openstack.org/317958
17:17:18 <alaski> auggy: great. that should be a good opportunity for documenting things that will help someone getting up to speed on cells
17:17:29 <melwitt> his doesn't handle the cells v1 case so I was hoping we could merge the two. mine passes the cells job in the experimental queue
17:18:18 <alaski> melwitt: excellent
17:18:59 <alaski> I will take a look and comment
17:19:26 <melwitt> cool, thanks
17:19:43 <auggy> yeah the docs question was going to be the next thing, since i'm basically fresh eyes on this whole process
17:20:07 <alaski> I want to mention something that dansmith, bauzas, and I discussed yesterday but might benefit from further thinking
17:20:21 * dansmith doesn't even know what this is
17:20:26 <doffm> I have completely forgotten what docs we have. :)
17:20:27 <dansmith> oh, delete
17:20:29 <alaski> auggy: there's a cells doc in the devref that could be updated
17:20:29 <bauzas> plusone
17:20:31 <alaski> dansmith: yep
17:20:42 <alaski> it gets even better though
17:20:57 <auggy> alaski: ok cool i'll take a look and put together a patch based on my experiences with the grenade/devstack setup
17:21:17 <alaski> so instance delete currently blocks all progress on ongoing actions through the expected_task_state check on instance.save()
17:21:39 <alaski> but instance delete when all we have is a buildrequest does not block further actions
17:21:47 <alaski> like creating an instance in a cell after scheduling
17:22:41 <alaski> right now my approach is going to be that instance delete removes the buildrequest and instance_mapping if there is no instance in a cell yet, and their existence should be checked when writing to a cell
17:23:08 <alaski> which mostly works, but might leave a small window for a race
17:23:39 <alaski> so if anyone has thoughts on improving that I'm open to suggestions
17:23:55 <doffm> The race would be a very odd failure though. Would the instance possibly boot wile the instance_mapping was empty?
17:24:28 <dansmith> I'm still missing where there is a race
17:24:31 <alaski> once the RPC call goes to a compute the instance_mapping won't matter
17:25:08 <dansmith> if we create in the cell db before we send the build rpc call, then a delete can hit the instance and block progress the same way it does today
17:25:19 <dansmith> if we create instance, delete buildreq in that order,
17:25:32 <alaski> the delete won't hit the instance until instance_mapping points to it
17:25:33 <dansmith> then I would think we can clean up the instance if buildreq is gone, before we do the cast, so we're good
17:26:06 <dansmith> we just have to make sure we create and delete things in the right order I think
17:26:13 <dansmith> mapping, instance, delete buildreq
17:26:21 <dansmith> should let us detect the delete before the rpc call I think
17:26:32 <alaski> so if we write the instance, confirm buildreq/instancemapping exist,* ,  remap to instance, send RPC
17:26:34 <dansmith> once the instance and mapping are in place, then the normal delete procedures will work at any point
17:26:37 <alaski> the * is the race I think
17:27:01 <dansmith> write instance, write mapping, delete the buildreq, send rpc
17:27:56 <dansmith> also,
17:28:03 <dansmith> if we race at your *,
17:28:09 <dansmith> then delete will be re-issue-able, right?
17:28:13 <alaski> if we write mapping at the same time as the delete comes through it might remove the buildreq after we check
17:28:25 <alaski> yes, the delete is re-issue-able
17:28:44 <alaski> so not the worst thing, as long as our data stays consisten
17:28:46 <dansmith> I don't get that last thing
17:28:46 <alaski> t
17:29:13 <dansmith> create mapping and instance in one transaction,
17:29:21 <dansmith> delete the buildreq in another
17:29:38 <alaski> mapping and instance are in two dbs
17:29:41 <dansmith> if delete fails because it's missing, then we undo the instance
17:29:50 <dansmith> okay, right, but it doesn't matter
17:29:53 <dansmith> create the instance first
17:30:00 <dansmith> the instance isn't findable until the mapping is there
17:30:08 * bauzas sorry, got distracted
17:30:12 <dansmith> so mapping in one transaction, delete the buildreq in a separate transaction
17:30:26 <dansmith> if buildreq is gone, delete the mapping and instance, never send the rpc
17:30:42 <dansmith> I think we could whiteboard this in PDX and figure out a plan
17:30:48 <dansmith> not sure it's worth obsessing over right now
17:30:49 <alaski> a whiteboard would help
17:31:10 <alaski> we can move on and discuss this outside the meeting, but I wanted to bring it to everyones attention
17:31:14 <alaski> there's one further issue
17:31:26 <alaski> instance tags, which can be created at any point in the instance lifecycle
17:31:35 <doffm> alaski: Is there a patch where this race is introduced... for comments.
17:31:37 <alaski> as in, when no instance exists
17:31:58 <bauzas> I just think the instance mapping should be the very last thing to delete, given any problem we could have
17:32:09 <alaski> doffm: this is the closest https://review.openstack.org/#/c/319379/
17:32:12 <bauzas> so we could just ask the user to delete again
17:32:39 <doffm> alaski: Thanks.
17:32:52 <alaski> bauzas: we could
17:32:58 <dansmith> alaski: you mean tag creation before the instance is scheduled?
17:33:00 <melwitt> asking the user to delete again, I hope we can avoid that because I think it'll happen a lot more than we are expecting here
17:33:32 <alaski> dansmith: right. so it's looking like we need to have that code update the buildreq instance
17:33:34 <dansmith> melwitt: agree we should try to avoid it, but if the tiny race window still allows reissuing of the delete, then that's the least worst
17:33:44 <bauzas> melwitt: sure, I'm just saying that if we still have the instance mapping, we can still delete again, which is not possible when the row is removed
17:33:45 <dansmith> alaski: oye
17:33:46 <alaski> melwitt: dansmith +1
17:34:24 <alaski> dansmith: yeah, I'm hitting a few places where api code needs to be aware of if it got instance from a buildreq and do different things
17:34:49 <melwitt> yeah, I know it's least worst. just that it's not okay IMO. I think it'll happen a lot and will cause a lot of pain for users
17:35:12 <dansmith> alaski: seems like we might be able to argue for a "busy try again in a minute" status code for that
17:35:20 <dansmith> melwitt: you think the race will?
17:35:46 <alaski> melwitt: it would need to be a delete call almost immediately after issuing the boot request
17:36:09 <alaski> melwitt: I would really like to solve this. we need a lock or sync point, and I'm not sure if we have the right things in place to do that yet
17:36:23 <doffm> Sounds like something that could happen in testing repeatedly. Which isn't the worst thing, but could cause repeated alarms in automated tests.
17:36:31 <alaski> we previously used task_state
17:36:44 <dansmith> alaski: that's really not airtight though
17:36:52 <alaski> true
17:37:04 <dansmith> alaski: it eventually stops the thing, but not always on the first try, AFAIK
17:37:45 <alaski> yeah. I'm going to think on it more, and I just wanted to get you smart people thinking on it as well
17:37:53 <melwitt> dansmith: yes. I think I've seen a lot of bugs in launchpad over the years of people deleting before the instance schedules. perhaps I'm thinking of the old cells v1 bug with delete racing with boot that got fixed when I converted things to instance objects
17:38:00 <alaski> but for now I'm going to try relying on checking buildreq delete
17:38:02 <dansmith> I think it's fully resolvable and not that big of a deal
17:38:15 <melwitt> that race happened often in the tempest job
17:38:33 <dansmith> melwitt: sure, I know that kind of thing happens, but I wouldn't say it "affects a lot of our users" I guess
17:38:38 <bauzas> I tend to agree with dansmith here, because cells v1 delete is a very different problem
17:38:49 <bauzas> given how we call top/down and bottom/up
17:38:55 <bauzas> (for cells v1)
17:38:56 <dansmith> or a lot of pain or whatever
17:39:15 <dansmith> I think the average horizon user probably clicks delete three times per delete anyway
17:39:18 <dansmith> like an elevator button :D
17:39:21 <bauzas> just making some assumptions when issuing a delete doesn't sound hard to do
17:40:18 <alaski> I think we need to take this to the review, or a whiteboard in pdx
17:40:32 <melwitt> well, I have seen enough things like that, people boot an instance and immediately delete it. and to have the situation of "oh yeah, sometimes it doesn't really delete it, just delete it again"
17:40:38 <alaski> I have something to try, but further suggestions definitely welcome
17:41:02 <melwitt> I'm just saying I think it's worse than the consensus here is saying. just my opinion
17:42:13 <alaski> I agree it's crappy, but potentially addressable with the current direction
17:42:24 <alaski> I need to see it visually to convince myself either way
17:42:49 <doffm> We have reached the cognitive limits of IRC.
17:42:56 <melwitt> heh
17:43:09 <alaski> dansmith: so back to your "try again later" status code
17:43:14 <alaski> doffm: yep
17:43:47 <alaski> that touches on the crux of this, which is that there is now a big distinction between pre/post scheduled instances
17:44:17 <alaski> to avoid exposing that in the API we sort of need to hide that, but should we really do that
17:44:35 <dansmith> um
17:44:41 <dansmith> too many "thats" in there for me to grok
17:44:49 <alaski> heh
17:45:02 <alaski> should users be aware that an instance isn't really an instance until after scheduling
17:45:11 <alaski> and some actions may not work until then
17:45:26 <dansmith> the problem is it's not an action right?
17:45:30 <alaski> like updating tags or metadata
17:45:38 <dansmith> an action of shelve doesn't work on a pre-schedule instance,
17:45:46 <dansmith> but tags have their own CRUD operations, right?
17:46:01 <alaski> right. it's not instance actions, it's more actions on data attached to an instance
17:46:14 <alaski> s/attached/associated/
17:46:34 <dansmith> alaski: so we *can* load modify and save the buildreq, right? the problem is,
17:46:45 <alaski> we can yes
17:46:48 <dansmith> high danger of losing writes when multiple things are happening together
17:47:07 <dansmith> we could use a generation field to at least signal that it happened instead of lying,
17:47:11 <alaski> that. and that some code now needs two paths
17:47:19 <dansmith> but that is the same as the "try back later" right?
17:48:17 <alaski> yes
17:48:32 <alaski> I'm not sure it's much, or any, worse than today with an instance
17:48:48 <alaski> my main concern is special casing the code around buildrequests
17:49:04 <alaski> but I may be convincing myself that it's only a few things and not that bad
17:49:20 <dansmith> right but blocking things is less duplication than allowing it to complete in two ways
17:50:00 <alaski> yes. but if we can avoid blocking I would like to
17:50:32 <dansmith> so wait
17:50:57 <dansmith> you want to (a) avoid blocking (b) avoid doing it two ways (c) not move tags to the api db?
17:51:06 <dansmith> please god don't say it's (c)
17:51:25 <alaski> (a)
17:51:38 <alaski> but also (c), why would we move tags
17:51:41 <dansmith> so you want to load/store the buildreq, add a version field?
17:52:07 <dansmith> I guess I thought above you said you didn't want to do that
17:53:24 * dansmith notes seven minutes remain
17:53:39 <alaski> what I'm hoping to avoid is too much complex if buildreq then foo elif instance then bar
17:53:53 <alaski> if it can be abstracted nicely I'm not concerned
17:54:18 <alaski> yeah, I don't think we'll solve this here
17:54:21 <dansmith> I dunno, I think you're likely going to have it be pretty complex,
17:54:26 <dansmith> because tags are not stored in the instance,
17:54:46 <dansmith> so you're going to need to store them with the buildreq instance, and make sure instance create handles that right
17:54:55 <dansmith> maybe it will, I'd have to go look, I don't recall you being able to do that though
17:55:11 <alaski> it doesn't, I would have to add that
17:55:11 <dansmith> I was thinking tags was a regular @property, but maybe it's a proper field
17:55:18 <alaski> it's a field
17:55:30 <alaski> but it's not normally updated that way
17:55:34 <alaski> maybe not ever
17:55:38 <dansmith> it is yeah
17:55:42 <dansmith> and yeah, it's a weird one
17:56:21 <alaski> well, I'll put something together and we can discuss on a review
17:56:45 <alaski> that's all I have today
17:56:51 <dansmith> tags are not created in create()
17:56:55 <alaski> anyone want to use up the last 3 minutes?
17:56:56 <dansmith> anyway
17:57:24 <alaski> dansmith: yeah, they're a bit odd
17:57:27 <alaski> okay, thanks all!
17:57:30 <doffm> Thanks.
17:57:32 <alaski> #endmeeting