| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-06-05 | |||
| 14:39:20 | mriedem | when all you really wanted to do was update the compute_node.stats field | |
| 14:39:31 | dansmith | stephenfin: well, I don't see how the tests would cause this to be broken.. I think it has to be broken in the code and we just never poke this (and notice) in tests right? | |
| 14:39:44 | dansmith | mriedem: yeah, the more I think about it, that'd be a better approach | |
| 14:39:52 | mriedem | OR | |
| 14:39:53 | dansmith | mriedem: and would remove a ton of unit test noise I had to add | |
| 14:40:48 | mriedem | RT.abort_instance_claim could do the build failure count | |
| 14:40:58 | mriedem | but, if we fail before we get to the instance claim, then that won't count the build failure | |
| 14:41:09 | mriedem | but abort_instance_claim does update the compute node record | |
| 14:41:20 | dansmith | yeah, that runs before where I'm increasing right? | |
| 14:41:27 | mriedem | yes | |
| 14:41:31 | dansmith | that's why Isaid in my comment just now that the next instance failure would update it for us | |
| 14:41:50 | dansmith | which I think is reasonable lazy update behavior to avoid the synchronous update | |
| 14:41:53 | dansmith | plus, 60s by default | |
| 14:44:46 | mriedem | yeah that makes sense | |
| 14:45:14 | dansmith | aight, well, that likely removes this from my path, | |
| 14:45:26 | dansmith | although I still am concerned that something ain't right in that numa accounting path | |
| 14:46:00 | dansmith | although it will make my testing a little harder to make deterministic | |
| 14:46:23 | dansmith | you know, I had this thing for functional tests that let me trigger periodics reliably that got shot down with fire....... | |
| 14:47:54 | mriedem | i'll assume i wasn't on the firing end of that | |
| 14:49:20 | dansmith | nope | |
| 14:53:31 | efried | melwitt: We got a dashboard or anything for spec reviewing? | |
| 14:58:47 | stephenfin | efried: I use https://goo.gl/EjSxKw | |
| 14:58:57 | efried | stephenfin: Thanks! | |
| 14:59:08 | stephenfin | efried: The tail end | |
| 14:59:32 | efried | is that all the specs we got? That ain't so bad :) | |
| 14:59:45 | stephenfin | It's limited at 20, so no :) | |
| 15:00:11 | stephenfin | there's the full one https://review.openstack.org/#/q/project:openstack/nova-specs+status:open | |
| 15:08:33 | openstackgerrit | Surya Seetharaman proposed openstack/nova-specs master: Handling a down cell https://review.openstack.org/557369 | |
| 15:19:22 | openstackgerrit | Theodoros Tsioutsias proposed openstack/nova-specs master: Add PENDING vm state https://review.openstack.org/554212 | |
| 15:25:14 | mriedem | this is my specs review dashboard http://tiny.cc/2g1sty | |
| 15:36:45 | stephenfin | Can someone with +2 for nova-specs put this in? The guy has been refreshing it pretty much continuously since March and it's valid IMO https://review.openstack.org/#/c/551802/ | |
| 15:59:23 | mdbooth | mriedem: If I've read it correctly, AttachVolumeMultiAttachTest.test_volume_swap_with_multiattach seems to test something which I'd expect us to forbid, because it can't work. | |
| 15:59:54 | mdbooth | It looks like we attach a volume to 2 servers | |
| 16:00:23 | mdbooth | Then do a swap volume to a new volume on one of the servers | |
| 16:01:03 | mdbooth | The problem is, IIUC that's a block migration, and there's no way for the block migration on server 1 to know about writes on server 2 | |
| 16:01:37 | mdbooth | So the resulting data on volume2 attached to server1 is going to be inconsistent, and there's nothing you can do about it in either guest | |
| 16:02:51 | mriedem | stephenfin: done | |
| 16:02:53 | mdbooth | forbade? | |
| 16:03:11 | stephenfin | mriedem: cheers | |
| 16:03:15 | mdbooth | forbode? No, definitely not. Perhaps appropriate, though. | |
| 16:03:19 | mriedem | mdbooth: cinder forbids a retype to/from multiattach capability for an in-use volume | |
| 16:03:36 | mriedem | also on a call atm | |
| 16:03:42 | mriedem | THE call | |
| 16:03:46 | mdbooth | mriedem: np | |
| 16:13:29 | openstackgerrit | Merged openstack/nova-specs master: Follow the new PTI for document build https://review.openstack.org/551802 | |
| 16:29:46 | melwitt | efried: yeah, no specific dashboard. just whatever you normally like to use for specs | |
| 16:35:54 | openstackgerrit | Merged openstack/nova master: Fix bug for hypervisors https://review.openstack.org/572063 | |
| 16:36:01 | openstackgerrit | Merged openstack/nova master: Fix bug to doc https://review.openstack.org/572268 | |
| 16:40:59 | mriedem | dansmith: i know i say this every 3 days, but i think stephenfin's numa aware vswitches spec is ready to go https://review.openstack.org/#/c/541290/ | |
| 16:41:56 | efried | We should really have a spec review day so we can merge that thing. | |
| 16:42:21 | dansmith | mriedem: can I skip this one and wait three days until it's ready again? | |
| 16:42:30 | mriedem | no | |
| 16:45:27 | dansmith | damn | |
| 16:48:02 | dansmith | I feel like the proof is in the code anyway and I'm out of steam on the spec | |
| 16:48:08 | dansmith | so, damn the torpedoes | |
| 16:49:02 | mriedem | yeah i already went through the code series this morning and made notes on the patches about what needs to change | |
| 16:49:04 | mriedem | at a high level | |
| 16:52:55 | openstackgerrit | Merged openstack/nova stable/queens: Allow cinderv2 endpoints within the request context catalog https://review.openstack.org/572213 | |
| 16:56:54 | openstackgerrit | Merged openstack/nova-specs master: Add 'numa-aware-vswitches' spec https://review.openstack.org/541290 | |
| 17:10:24 | efried | bhagyashri_s: Greate work on https://review.openstack.org/#/c/560459/ -- just a couple of minor things and I think it's ready to go. I'm going to have to let other cores (probably bauzas and jaypipes) approve it, since I contributed to the fix. | |
| 17:31:13 | openstackgerrit | Chris Friesen proposed openstack/nova-specs master: Add support for emulated virtual TPM https://review.openstack.org/571111 | |
| 17:45:10 | openstackgerrit | Matt Riedemann proposed openstack/nova master: api-ref: mention that you can't re-parent a resource provider https://review.openstack.org/572501 | |
| 18:18:04 | openstackgerrit | Dan Smith proposed openstack/nova master: Change consecutive build failure limit to a weigher https://review.openstack.org/572195 | |
| 18:55:50 | mriedem | dansmith: melwitt: tssurya: on the handling a down cell spec, https://review.openstack.org/#/c/557369/ - i'm thinking that if the user is filtering or paging while listing instances, we need to ignore down cells | |
| 18:55:57 | mriedem | because we can't provide accurate results in that case | |
| 18:56:23 | mriedem | not that we're providing much in the way of results with just a flat 'nova list', but at least it's a dump of all instances (under the default limit) with minimal fields specified | |
| 18:56:56 | melwitt | yeah, either that or do the minimal shell of an instance thing for those in down cells | |
| 18:56:56 | dansmith | you mean sorting? | |
| 18:57:02 | tssurya | mriedem: yes makes sense, | |
| 18:57:08 | mriedem | dansmith: no, filtering | |
| 18:57:19 | dansmith | if they're sorting we can't really show the shells, | |
| 18:57:20 | dansmith | since we don't know what the sort key value is | |
| 18:57:21 | dansmith | same for filtering I guess | |
| 18:57:21 | mriedem | sorting kind of goes out the window as well | |
| 18:57:40 | mriedem | melwitt: the minimal shell is the problem when filtering | |
| 18:57:46 | tssurya | if its the default sorting, I think it uses created_at, | |
| 18:57:55 | melwitt | oh | |
| 18:57:55 | mriedem | if i'm doing 'nova list --status PAUSED', don't show me 4 PAUSED instances and 100 UNKNOWN instances | |
| 18:58:00 | melwitt | yeah | |
| 18:58:02 | dansmith | right | |
| 18:58:15 | mriedem | if i do: nova list --all-tenants | |
| 18:58:19 | mriedem | then sure, give me the dump | |
| 18:59:00 | mriedem | tssurya: yes by default we sort by created_at in desc order https://github.com/openstack/nova/blob/master/nova/api/openstack/common.py#L143 | |
| 18:59:07 | mriedem | which we can achieve with the instance mappings records | |
| 18:59:15 | tssurya | mriedem: yes | |
| 18:59:38 | mriedem | if i'm sorting by status and i have UNKNOWN minimals, then i guess those just get sorted too | |
| 18:59:43 | tssurya | so let's just support simple and plain nova list and if any additional arguments are specified, we give 500 ? | |
| 19:00:04 | tssurya | so that its consistent | |
| 19:00:19 | mriedem | no, i'm saying if there are filter parameters, we do what we can with the cells that respond, but ignore the ones that dont | |
| 19:00:47 | mriedem | i honestly don't know how this is going to be used in all cases, or what's really needed | |
| 19:01:03 | tssurya | right, | |
| 19:01:13 | mriedem | like, does the caller need some indication that there are down cells? if so, would returning a new header help with that? | |
| 19:01:18 | mriedem | but a non-admin user shouldn't know there are down cells | |
| 19:01:46 | dansmith | I think the most important thing is to avoid a 500 when there's a down cell, | |
| 19:01:52 | dansmith | the more info we can return the better, | |
| 19:02:16 | dansmith | but even if we skip everything in a down cell, that's incremental progress, then returning shells when they wouldn't be otherwise filtered out is progress on top of that, etc | |
| 19:02:46 | mriedem | agree, and i think the first part of that could be done as a bug fix and backported | |
| 19:02:55 | dansmith | yep | |
| 19:03:01 | tssurya | users shouldn't know anything about cells, however in order to avoid 500, if like mriedem said about filters, -> i.e to still process output from up cells, it would be inconsistent right ? | |
| 19:03:07 | tssurya | I mean not the right results | |