| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-06-20 | |||
| 16:55:57 | dansmith | but at least you continue to see them there where you did ten minutes ago before the outage | |
| 16:56:33 | mordred | I think I prefer option 1 - with status=='unknown' or something like that ... | |
| 16:56:42 | dansmith | okay, potential corollary: | |
| 16:57:05 | mordred | but - trying to think through some of the higher-level shade interactions and how we'd handle them | |
| 16:57:21 | dansmith | option 1a (future): we use memcache or something to stash the last-known image of an instance and could fill in a stale version of the whole thing for things that are down | |
| 16:57:36 | dansmith | 1a could come in the future, but won't be available now | |
| 16:57:43 | mordred | yah - I think 1a would be ideal | |
| 16:57:53 | dansmith | if we did that later, option 2 would become 1a suddenly, and make the unreachable: key never used | |
| 16:58:13 | dansmith | tssurya: all this I'm saying is correct yeah? | |
| 16:58:32 | mordred | out of curiosity ... in the 1a world -if I get a list of stuff and it's got a stale cached server in it - and I do DELETE /servers/{id} ... what happens? | |
| 16:58:34 | tssurya | dansmith: yea makes sense | |
| 16:58:58 | dansmith | mordred: do you want to ask about PUT instead of delete? | |
| 16:59:01 | mordred | if the cell is still down - do I get a 404 for that? or do I get a 'retry again later please' ? | |
| 16:59:05 | dansmith | delete is async right now, and does't go away immediately | |
| 16:59:18 | mordred | dansmith: well - both are questions I guess | |
| 16:59:32 | mordred | will the scheduler accept the async delete if the cell for the server is down? | |
| 16:59:35 | dansmith | DELETE would "succeed" and never happen, like it does today for lots of things | |
| 16:59:49 | tssurya | dansmith: although I never thought of doing 1a), plus we were thinking if we could fully req_spec as of now or not and maybe just go with uuids and not put in flavors and images | |
| 16:59:51 | mordred | so it wouldn't eventually get queued and handled? | |
| 16:59:52 | dansmith | PUT would fail with some code | |
| 16:59:55 | dansmith | mordred: no | |
| 16:59:59 | mordred | nod | |
| 17:00:01 | dansmith | mordred: well, | |
| 17:00:26 | dansmith | mordred: yeah, no. | |
| 17:00:45 | dansmith | tssurya: yeah that makes option 1 more completeish, and 1a a smaller delta | |
| 17:00:50 | mordred | ok - both of those are handleable - just checking/thinking out loud | |
| 17:00:56 | dansmith | mordred: ack | |
| 17:01:02 | mordred | when you said "out of order" - how does that affect pagination? | |
| 17:01:14 | mordred | if I'm iterating over a paginated call and a cell goes down ... | |
| 17:01:22 | dansmith | mordred: if the cell goes down in the middle, probably poorly, | |
| 17:01:25 | mordred | neat | |
| 17:01:28 | tssurya | dansmith, mordred: do see gibi's and mriedem's concerns on going with option 1 here: https://review.openstack.org/#/c/557369/ | |
| 17:01:43 | dansmith | mordred: option 2 is less affected if your marker is an instance in one of the up cells | |
| 17:02:17 | dansmith | mordred: what we're trying to do is avoid the 500 we have right now and show you as much info as we can.. that's a noble goal right? | |
| 17:02:47 | mordred | dansmith: yah - totally! because otherwise cells makes the cloud less rather than more reliable | |
| 17:02:48 | dansmith | tssurya: can you point me to a specific comment there? I skimmed some of that and didn't really hone in on the discussion around this specifically | |
| 17:02:53 | tssurya | L85 | |
| 17:02:55 | tssurya | ^^ | |
| 17:03:10 | tssurya | basically setting stuff to UNKNOWN is not neat | |
| 17:03:19 | tssurya | by stuff I mean fields, | |
| 17:03:22 | dansmith | mordred: well, and we're trying to be more reliable than "our one database went down so we're stuck", just wanted to make sure you preferred that over consistency | |
| 17:04:10 | dansmith | tssurya: I want the state to be unknown, | |
| 17:04:20 | dansmith | tssurya: and the other fields can all be missing I think | |
| 17:04:23 | mordred | dansmith: yeah - I think as long as there's a thing we can flag on for the different behavior, it should be fine | |
| 17:04:52 | tssurya | dansmith: but the current API would complain since for some fields we surely need to have values like they can't be blank | |
| 17:04:53 | mordred | and then yeah - state of unknown to me then says you can either omit the other fields or set their value to null | |
| 17:04:58 | tssurya | (right?) | |
| 17:05:07 | dansmith | mordred: ack | |
| 17:05:12 | mordred | clients are going to need to handle servers in an unknown state regardless - there is no magical thing we can do to make this transparent for existing api consumers | |
| 17:05:20 | dansmith | tssurya: yeah, maybe, but that seems like mechanics we can work around if we need | |
| 17:05:29 | tssurya | mordred: setting it to NULL/empty is kind of like lying and not truth also | |
| 17:05:33 | dansmith | tssurya: mordred is a "real api consumer" which is why I want his opinion on this | |
| 17:05:39 | mordred | so I think saying "if you get a server in state UNKNOWN - here's what that means for the rest of the thing" | |
| 17:05:45 | tssurya | dansmith: ack :) | |
| 17:06:15 | mordred | tssurya: yah - totally - honestly I think we could also remove the whole entry (this is a microversion behavior, right?) | |
| 17:06:25 | mordred | sorry - when I said whole entry - I mean key and value | |
| 17:06:26 | dansmith | tssurya: well, saying we don't know what the value is is definitely more honest than saying it _is_ "unknown" or some fake value | |
| 17:06:47 | dansmith | mordred: yeah, so before a microversion you'd get a 500 if a cell is down I guess | |
| 17:06:47 | tssurya | dansmith: true, | |
| 17:06:57 | mordred | I know there is a concern about hitting key errors - but if you opt in to this microversion, you're saying you can deal with unknown state servers | |
| 17:07:03 | mordred | dansmith: ++ | |
| 17:07:04 | dansmith | which is an odd microversion, but.. | |
| 17:07:26 | mordred | well - it's a "list servers with this microversion because I can handle some of the servers in the list having less keys thn I was expecting" | |
| 17:07:32 | dansmith | it's a pretty well-baited one, because if you don't opt into that one for list, you're basically saying "meh, I can't handle the inconsistency" so we're helping by not showing you anything | |
| 17:07:34 | mordred | whereas existing code is just going to error | |
| 17:07:39 | dansmith | yes | |
| 17:07:45 | mordred | dansmith: exactly | |
| 17:08:13 | mordred | it's the old data consistency thing - it's better for the entire data store to be unavailable than to behave ina half-available state and maybe lose data when you thought it was ok | |
| 17:08:38 | dansmith | yeah, like gibi said, | |
| 17:08:41 | mordred | current behavior is 500 - nobody gets accidentally hosed - they can always retry the query until it stops 500ing | |
| 17:08:41 | tssurya | mordred: right | |
| 17:08:43 | mordred | ++ | |
| 17:08:52 | dansmith | if we report hostId=UNKNOWN, then a client thinks all the instances are suddenly on the same host :) | |
| 17:09:56 | dansmith | tssurya: okay so anyway, I think mordred and I agree on this, so I'll comment on the spec about this convo and maybe we can sync up with mriedem when he's back next week and see what he thinks | |
| 17:10:11 | dansmith | tssurya: are you opposed to making this unified or are you okay with either? | |
| 17:10:12 | tssurya | dansmith: ack | |
| 17:10:15 | dansmith | and which is your preference? | |
| 17:10:25 | tssurya | I am okay with either | |
| 17:10:27 | dansmith | all things equal, I mean, which do you think is better? | |
| 17:10:33 | mordred | dansmith: would we have the addresses dict since those are coming from neutron? or do those get stored in the cells db? | |
| 17:10:40 | tssurya | as long as we give at least the uuid back | |
| 17:10:57 | dansmith | mordred: we would have to hit neutron for it since our _cache_ of those values are in the cell db | |
| 17:11:07 | tssurya | like keeping the current 500 behavior if 1 out of 70 goes down is wrong for sure | |
| 17:11:19 | dansmith | tssurya: yeah okay | |
| 17:11:21 | mordred | nod. I mean - I already have code to hit neutron api if the addresses dict is empty - so I'm fine if it's not | |
| 17:11:43 | tssurya | 1) we fix this with skipping down cells as first step 2) return just the uuids with rest as empty | |
| 17:11:48 | mordred | tssurya: uuid and name (filtering on name client-side is frequent/important) | |
| 17:11:50 | tssurya | makes sense ? | |
| 17:12:17 | mordred | if it's possible | |
| 17:12:18 | dansmith | mordred: we don't have easy access to the name, but we can get it from the reqspec if you think it's important | |
| 17:12:21 | tssurya | mordred: we were thinking of skipping a cell if filters are used | |
| 17:12:40 | dansmith | yeah, if you're filtering, and we don't know the value, we assume no match | |
| 17:12:51 | tssurya | dansmith: we don't have the name in req_spec right ? | |
| 17:12:57 | dansmith | tssurya: the display name? | |
| 17:13:05 | tssurya | yes | |
| 17:13:32 | tssurya | meaning we don't have the display_name | |
| 17:13:36 | tssurya | in req_spec | |
| 17:13:52 | dansmith | oh, I guess I'm thinking of buildreq, which stores it while we're building | |
| 17:13:56 | dansmith | so yeah maybe not | |
| 17:14:31 | mordred | ah - we don't have the name? hrm. | |