21:00:01 <dansmith> #startmeeting nova_cells
21:00:02 <openstack> Meeting started Wed Jan 24 21:00:01 2018 UTC and is due to finish in 60 minutes.  The chair is dansmith. Information about MeetBot at http://wiki.debian.org/MeetBot.
21:00:03 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
21:00:05 <openstack> The meeting name has been set to 'nova_cells'
21:00:12 <tssurya_> o/
21:00:55 * dansmith pokes belmoreira melwitt mriedem
21:01:03 <melwitt> o/
21:01:09 <belmoreira> o/
21:01:19 <mriedem> o/
21:01:22 <melwitt> sorry I thought maybe we were talking in #openstack-nova instead of meeting
21:01:25 <dansmith> #topic bugs
21:01:39 <dansmith> I think we already said there are no new major bugs to discuss, right?
21:02:01 <mriedem> right
21:02:12 <dansmith> #topic open reviews
21:02:12 <belmoreira> We just run out into something... not sure if its a bug
21:02:16 <dansmith> anything here?
21:03:01 <mriedem> nope,
21:03:01 <dansmith> belmoreira: okay let's just pile all your stuff into open discussion, which I'm assuming we can just roll to
21:03:05 <mriedem> just rechecking the alternate host patch
21:03:06 <dansmith> unless anyone has anything here
21:03:12 <dansmith> cool
21:03:21 <dansmith> #topic open discussion
21:03:36 <dansmith> belmoreira: okay, first off, what's the issue you just hit?
21:04:35 <belmoreira> currently our instance_mappings in nova_api DB doens't have any cell mapping (it's null)... when migrating to cellsV2 will need to do the correct mapping. However, the table is already populated and it fails to update the correct cell (change null to the right one)
21:04:56 <belmoreira> we are in Newton and nova_api DB is required
21:05:14 <belmoreira> are we missing something?
21:05:43 <dansmith> you're using newton nova-manage to do the instance mappings and it's not filling that field, is that right?
21:06:04 <dansmith> with --cell_uuid ?
21:06:35 <belmoreira> We were testing this week with master and it didn't update the cell_uuid
21:06:44 <belmoreira> tssurya_?
21:06:54 <tssurya_> yes, we tested this with master
21:07:03 <tssurya_> and we had provided the --cell_uuid
21:07:35 <dansmith> that seems really odd, I don't know why that would be
21:07:39 <mriedem> probably because of the marker?
21:07:44 <mriedem> saying you've already mapped everything
21:08:05 <tssurya_> yea that or the duplicate entry check ?
21:08:09 <dansmith> does map_instances use a marker like that?
21:08:11 * dansmith looks
21:08:14 <mriedem> it does
21:08:27 <dansmith> ah, then just nuke that and try again I guess
21:08:37 <mriedem> cell-uuid is required for map_instances, so i'm not sure how the instance mappings would have a null cell mapping entry
21:08:57 <mriedem> unless it was a bug in the CLI in newton?
21:09:21 <belmoreira> in Newton we don't have any cellV2 defined
21:09:25 <dansmith> idk
21:09:26 <mriedem> do you see an instance mapping with INSTANCE_MIGRATION_MARKER as the project_id?
21:09:46 <belmoreira> however nova_api DB is required and new instances have an entry in instances_mapping
21:09:48 <mriedem> i don't see how you could run map_instances without a cell_uuid
21:09:51 <mriedem> to a valid cell mapping
21:10:26 <tssurya_> okay so to test the scenario, we ran it on master with a --cell_uuid option
21:10:28 <dansmith> certainly not based on the master code, but maybe it was different in newton
21:10:39 <tssurya_> we manually set the value in the DB as NULL
21:10:55 <tssurya_> to recreate the scenario
21:11:09 <belmoreira> mriedem we will run "map_instances" only when upgrading to Pike
21:11:19 <dansmith> you need it in ocata too
21:11:27 <dansmith> even though it's all one cell
21:11:48 <mriedem> ok i guess you'll have to find a valid recreate for this,
21:11:53 <belmoreira> dansmith, true
21:11:53 <mriedem> that doesn't involve manually messing with the db
21:11:58 <mriedem> b/c i do'nt know how this would happen
21:12:12 <mriedem> are you using simple_cell_setup at all?
21:12:20 <dansmith> I hope not :)
21:12:27 <dansmith> that's toootally not for situations like this :P
21:12:42 <melwitt> to be clear, map_instances isn't going to fill in a NULL field for you. it will only create new instance_mappings records. right?
21:13:02 <belmoreira> but is the same thing. Then we need to map again the instances to the real cells when upgrading to Pike and cellsV2
21:13:14 <tssurya_> melwitt : yes
21:13:16 <belmoreira> in ocata everything will be mapped to only one
21:13:23 <melwitt> so the question is, how did you get any instance_mappings records with a NULL field in the first place
21:13:42 <mriedem> i think i understand what belmoreira is saying now,
21:13:46 <dansmith> ah yeah I guess that's right
21:13:49 <mriedem> in ocata, they have like 50 child cells,
21:13:52 <belmoreira> melwitt newton with any cellV2 defined
21:13:58 <mriedem> but they really only have 1 global cellsv2 cell right?
21:14:07 <mriedem> so if you map instances in ocata, they go to the global single cellsv2 cell,
21:14:09 <mriedem> but in pike,
21:14:16 <mriedem> you want to map the instances in the separate child cells,
21:14:24 <mriedem> but they are already mapped to the global thing
21:14:25 <belmoreira> mriedem yes
21:14:26 <mriedem> correct?
21:14:27 <mriedem> ok
21:14:28 <mriedem> well,
21:14:30 <melwitt> oh, I understand now too
21:14:35 <dansmith> yeah, but at that point you can just nuke all the mappings and recreate them right?
21:14:45 <melwitt> it wasn't NULL but it was all to one global cell
21:15:22 <mriedem> dansmith: yeah, that or provide some --overwrite option to the CLI
21:15:29 <mriedem> to create or update
21:15:46 <belmoreira> dansmith we were thinking on that as well. Just asking if there was a different way, ir the nova-manage instance mapping just could update the cell if already sees the instance
21:15:48 <tssurya_> dansmith : we already thought of nuking and recreating
21:15:53 <dansmith> yeah I actually thought the code would create or get-and-then-save, but it doesn't
21:16:44 <dansmith> updating the cell of an instance is kindof a dangerous thing if it's wrong, which might be why this doesn't do that now, but I can't think of any real reason to not do that
21:16:44 <mriedem> the fake marker in the instance_mappings table will also cause problems
21:16:58 <dansmith> if you ended up with two cells pointing at the same db, for example, you could re-map and mess struff up
21:17:02 <dansmith> oh right
21:17:24 <belmoreira> melwitt all instance_mappings created in Ocata have the cell_uuid of the global but the old entries from newton is null
21:18:06 <dansmith> so we could allow delete_cell --force to remove instance mappings too,
21:18:20 <dansmith> and then when they go to delete their global cell, it will nuke the instance mappings too
21:18:27 <mriedem> i think that's probably safer
21:18:38 <melwitt> belmoreira: oh, odd. there must be a bug in newton then. we need to investigate that
21:18:40 <dansmith> we have --force right now for hosts but not instance mappings
21:19:09 <mriedem> note that the marker mapping has a null cell mapping field
21:19:12 <tssurya_> dansmith : yes mappings go only after archive
21:19:22 <mriedem> so delete_cell --force wouldn't remove the marker either...
21:19:39 <dansmith> yeah the marker is going to be a problem regardless
21:20:09 <dansmith> we probably need a map_instances --reset
21:20:17 <dansmith> which will avoid the marker
21:20:24 <belmoreira> don't know what the marker is. Will check tomorrow
21:20:34 <mriedem> map_instances can run in chunks
21:20:36 <mriedem> until complete,
21:20:41 <mriedem> so there is a marker for knowing where to start next
21:20:57 <belmoreira> mriedem thanks
21:21:03 <dansmith> so, belmoreira tssurya_, can one of you file bugs for these two new flags to the cli?
21:21:16 <tssurya_> dansmith : sure
21:21:32 <dansmith> okay so is that all we need to discuss on this map_instances thing?
21:22:03 <belmoreira> From us I think so
21:22:19 <dansmith> okay, so what else did you have? the cell-is-down thing?
21:22:21 <tssurya_> just to be sure the new flags you mean are reset and force right ?
21:22:32 <mriedem> 1. reset marker on map_instances
21:22:33 <dansmith> tssurya_: for map_instances and delete_cell respectively, yeah
21:22:39 <mriedem> 2. remove instance mappings for a deleted cell
21:22:41 <tssurya_> dansmith : got it
21:22:59 <tssurya_> mriedem thanks
21:23:00 <dansmith> tssurya_: we already have --force on delete_cell but it's only active for hosts, so it needs to do instance_mappings too
21:23:20 <belmoreira> when moving to Pike with cellsV2 we need to have a solution for a cell going down.
21:23:21 <dansmith> I'm quite sure we previously said something about never wanting to let you nuke a cell with instances in it, but alas, whatever :)
21:23:36 <tssurya_> dansmith : yea I remember that patch from takashi
21:24:15 <mriedem> belmoreira: well the solution probably depends on what you need for functionality out of the API
21:24:42 <dansmith> and it depends on what the API people want in the way of communicating a partial result
21:25:04 <belmoreira> understand
21:25:06 <mriedem> yeah because i think all we have in the api db is the instance uuid, cell and host
21:25:27 <dansmith> pretty much yeah
21:25:46 <belmoreira> for us the uuid will be enough
21:25:57 <dansmith> so I expect we'll just need to return {uuid: $uuid, state: UNKNOWN} or something for each instance
21:26:05 <belmoreira> but I can't hide instances from the users
21:26:16 <dansmith> but then the api layer will need to return 234: only part of your stuff was found, yo.
21:26:28 <belmoreira> is looking into the request_specs a crazy idea?
21:26:33 <dansmith> yes
21:26:36 <mriedem> there is a bunch of stuff we'll need to return UNKNOWN for
21:26:37 <dansmith> :)
21:26:39 <belmoreira> :)
21:27:21 <belmoreira> crazy yes, but it could work... just when a cell is down
21:28:05 <mriedem> i guess i forgot about request specs
21:28:11 <dansmith> I don't know how much we could get out of reqspec that would be accurate, so I'd have to see,
21:28:22 <dansmith> but reqspec requires de-json'ing every instance
21:28:31 <dansmith> which would be a lot of CPU overhead to incur when a cell is down
21:28:47 <dansmith> and we couldn't get all the info out of it that we need I think
21:28:53 <mriedem> not so bad for GET on a specific instance,
21:28:58 <mriedem> but for listing instances, yes definitely
21:29:05 <dansmith> and I'm also not sure it'll always be accurate.. I can't remember if we update reqspec after a resize, for example
21:29:24 <mriedem> the accuracy of reqspecs is definitely dubious
21:29:26 <dansmith> since resize happens in the cell late, I think we don't
21:29:34 * mriedem refers to all of the TODOs in the conductor code
21:29:42 <dansmith> yeah, reqspecs are a big mess
21:30:32 <mriedem> looks like we do update the reqspec for the new flavor during resize,
21:30:34 <mriedem> in conductor
21:30:43 <mriedem> and for rebuild if you're changing the image, in the api
21:31:01 <dansmith> and if the resize fails? I guess we have a chance to fix it when you revert or confirm?
21:31:27 <mriedem> yeah i don't see anything about fixing the reqspec if the resize fails, or you revert
21:31:59 <dansmith> anyway, this is about listing and I think listing and de-jsoning all that stuff for a whole cell is a bit of a problem
21:32:19 <dansmith> so one option I guess,
21:32:40 <dansmith> is to just fake out anything we don't have and go forward with that
21:32:48 <dansmith> knowing it's a bit of a lie, but we could do that fairly quickly
21:33:03 <mriedem> yeah i think we're going to have to start with the quick and dirty options so we have *something* if a cell is down
21:33:05 <dansmith> I dunno what to say about things like the flavor
21:33:26 <dansmith> right, well, I thought that's what tssurya_ was going to do
21:33:27 <dansmith> so we had something to argue over :)
21:33:33 <belmoreira> I'm happy with "not available"
21:33:47 <belmoreira> same for VM state, and all the others
21:33:51 <mriedem> and then if we did try to use the requestspec for a single GET /servers/id, we then have to assess how much of the response could be wrong, and if we're going to spend a bunch of time updating code to make sure the reqspec is correct because it's now used in the API
21:33:56 <dansmith> so we currently have a state of "unknown" for if the host is down I think
21:34:03 <dansmith> but we still have things like flavor at that point
21:34:25 <dansmith> mriedem: well, if we lie in the list, then returning different stuff on GET is kinda messy
21:34:44 <tssurya_> dansmith : yes that what we have started with, we have faked stuff, but yes stuck at fields lke flavor
21:34:44 <dansmith> if we return a shell only in the list somehow, then the GET having details seems a little more legit to me
21:34:57 <mriedem> dansmith: i don't mean lie in the list,
21:35:00 <dansmith> tssurya_: ack, makes sense
21:35:16 <mriedem> list would just return uuid and UNKNOWN for everything else,
21:35:31 <dansmith> well, that's not going to work with our schema I'm sure
21:35:39 <mriedem> with the uuid, you could at least to get some information for a specific instance using the reqspec, but that could be all wrong too
21:35:42 <dansmith> we can't return UNKNOWN for a flavor that should be a dict
21:35:53 <mriedem> right, we'd have to use something else that means unknown for non-strings
21:36:04 <mriedem> None or {} or [] or -1 or whatever
21:36:13 <dansmith> or we fill out a flavor object with all UNKNOWN properties,
21:36:22 <dansmith> but we have no sentinel for integers
21:36:30 <mriedem> -1 typically means unlimited :)
21:36:36 <dansmith> which is terrible,
21:36:47 <dansmith> and which will conflict for flavor things that actually use -1 and 0 and stuff
21:37:04 <mriedem> -MAX_INT :)
21:37:05 <dansmith> but that's fine, we can start with that and see how it goes
21:37:15 <dansmith> they can at least use that locally while we argue
21:37:22 <melwitt> cross reference with the flavors table?
21:37:26 <dansmith> melwitt: can't
21:37:49 <dansmith> we don't know which flavor look up in there, unless we decode the reqspec, which as pointed out, may be stale
21:38:00 <dansmith> (and it's a lie since the flavor might have been redefined)
21:38:17 <dansmith> we could pull the first flavor and just say everything in the cell is flavors[0] :P
21:38:36 <mriedem> the reqspec has a serialized versoin of the flavor from when the instance was created,
21:38:37 <melwitt> hehe
21:38:39 <mriedem> like the instance itself
21:38:53 <dansmith> ah yeah so if we're going to decode that we could just use that lie instead of making up a new one :)
21:39:13 <dansmith> tssurya_: so I think you just need to post something, anything, document the things you know are controversial, and then let's go from there
21:39:21 <dansmith> that was the point of the straw man
21:39:35 <tssurya_> sure I have been playing around with the above stuff
21:39:36 <dansmith> we can point api people to it and they can say "I hate this so much" and then we can ask for alternatives :)
21:39:47 <tssurya_> on how to go about with the various fields
21:40:06 <belmoreira> great, we will continue with this approach
21:40:07 <melwitt> dansmith: are you saying post up a rough POC patch?
21:40:15 <mriedem> yeah
21:40:19 <dansmith> melwitt: yes,
21:40:27 <dansmith> that's the thing I'm saying we need to discuss (a strawman)
21:40:32 <mriedem> "post up" isn't that gang slang?
21:40:44 <melwitt> okay. so yeah, tssurya_ if you can post up a very rough draft POC patch to show a way we can do it, we can discuss it there more concretely
21:40:45 <tssurya_> cool then, will open a WIP
21:41:09 <melwitt> gang slang? lol.
21:41:12 <mriedem> https://www.urbandictionary.com/define.php?term=post%20up
21:41:23 <melwitt> "propose"
21:41:32 <mriedem> time to watch the wire again
21:41:39 <tssurya_> another thing we got stuck at was this - https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/extended_volumes.py#L50
21:42:04 <tssurya_> even if we manage to fill fake stuff and come out,
21:42:25 <tssurya_> again the connection failure stuff comes up
21:42:25 <dansmith> that needs to be converted to use scatter/gather and then it can take a similar approach
21:42:40 <tssurya_> dansmith : ok
21:42:44 <melwitt> mriedem: lol, nice. I had never heard of it before
21:42:55 <mriedem> tssurya_: what's the issue? that we don't know which volumes are attached to which servers b/c we can't get BDMs?
21:43:19 <tssurya_> yea
21:43:23 <mriedem> we could use cinder...
21:43:46 <mriedem> but i don't know if there is a way to list volumes by instance_uuid in cinder as a filter, there might be
21:43:50 <tssurya_> basically that code again tries to query the DB which is down of course
21:43:53 <dansmith> that's N calls to cinder for N instances right?
21:44:04 <mriedem> if it's a list, i'm just saying for a show
21:44:10 <mriedem> if we do anything
21:44:26 <dansmith> that is only for list
21:44:28 <mriedem> again, start small and just ignore the down cell and return []
21:44:47 <dansmith> agreed
21:44:48 <mriedem> dansmith: i do'nt think so...
21:44:52 <belmoreira> +1
21:44:58 <mriedem> https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/extended_volumes.py#L40
21:45:12 <tssurya_> mriedem : ok :d
21:45:21 <dansmith> mriedem: doesn't use that helper
21:45:28 <mriedem> ah
21:45:32 <dansmith> only detail does, and it takes a list of servers and iterates over them
21:46:17 <dansmith> the show needs to be graceful about not failing the whole show there though because it tries and fails to do the list
21:46:28 <dansmith> the context is targeted there, so it needs to catch that and not explode
21:46:34 <dansmith> else the returning of fake things isn't going to work
21:48:06 <dansmith> tssurya_, belmoreira: okay what else?
21:48:16 <belmoreira> about quotas...
21:48:29 <belmoreira> What we are thinking is if we can't use the request_specs to calculate the quota of a project (challenging!!!) we will block the creation of new instances if the project has instances in cells that are down.
21:48:35 <belmoreira> makes sense?
21:48:59 <dansmith> that seems like a totally legit workaround to me
21:49:24 <dansmith> it needs to be configurable, in [workarounds]
21:49:25 <dansmith> as not everyone will want that of course
21:50:07 <mriedem> that seems fine as a start to me also
21:50:16 <melwitt> yeah, seems like that could be a good way to best-effort quota while cells are down
21:50:27 <mriedem> although,
21:50:36 <mriedem> when you go to create a server, how do you know any cells are down?
21:50:40 <dansmith> we have to know if cells are down when..
21:50:41 <dansmith> yeah that
21:50:43 <mriedem> since we don't have any status for cell mappings?/
21:50:56 <dansmith> but
21:51:09 <melwitt> the scatter-gather will return sentinels for no response etc at least
21:51:11 <dansmith> the scheduler has to get the host lists from all the cells to start, right?
21:51:24 <dansmith> although not with placement anymore I guess.. not reliably overlapping the down cell
21:51:26 <melwitt> (during the quota check)
21:51:30 <mriedem> unless you're forcing a specific host, yeah
21:51:39 <dansmith> melwitt: ah right because we do the quota check
21:51:45 <dansmith> sweet
21:51:47 <belmoreira> but the quota will be checked
21:52:04 <dansmith> yep, totes
21:52:18 <mriedem> so if the quota check scatter gather returns sentinels, fail?
21:52:29 <mriedem> or, check to see if that project has instances in a downed cell?
21:52:29 <dansmith> ...and this flag is asserted
21:52:52 <dansmith> mriedem: yeah, if sentinels, then go on to check on instances in the down cell
21:53:01 <mriedem> it's almost like we need a service group API heartbeat for cells...
21:53:03 <dansmith> which we can do by checking mappings
21:53:15 <dansmith> because our other servicegroup heartbeat thing works so well?
21:53:20 <mriedem> totally
21:53:23 <melwitt> wouldn't it be if sentinels then fall back to request_specs if workaround?
21:53:25 <mriedem> if it were using zk
21:53:28 <dansmith> either way, we'd have to have one thing in the cell dedicated to pumping that back up, which is a problem
21:53:42 <dansmith> melwitt: no, not using reqspecs at all
21:54:08 <dansmith> melwitt: if we do the quota check and find downed sentinels, then we quickly count the number of instances owned by the project in those downed cells
21:54:11 <melwitt> oh. what do you mean by "check on instances in down cell"
21:54:13 <dansmith> if that number is nonzero, then we refuse the build
21:54:14 <mriedem> dansmith: i think i'm just proposing some kind of periodic at the top (scheduler?) that tries to target the cells every minute or something
21:54:18 <melwitt> oh, I see
21:54:44 <melwitt> for cores and ram you'd need request_spec
21:55:08 <dansmith> mriedem: we have no single thing that can do that, we can have multiple schedulers now, and network partitions may make that not so straightforward
21:55:14 <mriedem> this reminds me that we said in syndey we'd add a type to the allocations / consumers table in placement...and didn't
21:55:17 <melwitt> er, nevermind. I think I'm not following the "nonzero, refuse the build". need to think more
21:55:31 <dansmith> melwitt: we wouldn't care about cores and ram, because they have instances in the downed cell so we just punt
21:55:42 <melwitt> got it
21:55:46 <dansmith> mriedem: we said we would and then got into a big argument
21:55:59 <dansmith> mriedem: and apparently there was a lot of disagreement on it
21:56:04 <dansmith> and jay wasn't there
21:56:14 * mriedem adds it back to the ptg etherpad
21:56:21 <melwitt> yeah, we need the type to be able to use placement for the quota stuff
21:56:26 <dansmith> anyway
21:56:36 <dansmith> tssurya_: belmoreira: I say propose something for that, yeah
21:56:46 <tssurya_> dansmith : yep
21:56:55 <belmoreira> yes, will do
21:56:59 <dansmith> sweet
21:57:06 <dansmith> is there anything else? we're about out of time
21:57:17 <tssurya_> I guess that's all for now ...
21:57:42 <dansmith> okay, anything else can go back to the main channel
21:57:45 <dansmith> last call....
21:57:46 <belmoreira> thanks a lot
21:57:51 <dansmith> ...for alcohol
21:58:03 <tssurya_> thanks!
21:58:05 * dansmith said that for mriedem's benefit
21:58:10 <dansmith> #endmeeting