| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-04-12 | |||
| 16:41:27 | bauzas | this ^ is for all drivers | |
| 16:42:31 | dansmith | right | |
| 16:42:41 | dansmith | and it's crazy to just silently accept | |
| 16:43:06 | dansmith | since we don't even create the migration context in the current form, I'm sure no migrations could be working in this case | |
| 16:44:23 | bauzas | I also don't see any good reason for doing this, even with the FakeDriver | |
| 16:44:32 | dansmith | ack | |
| 16:44:57 | dansmith | I'll update the comments there to reflect this and proceed unless you have any other checking you want to do | |
| 16:46:28 | bauzas | dansmith: given all we discussed, my only open concern is, shouldn't we rather hardstop the migration call here? | |
| 16:46:39 | bauzas | instead of returning a noop ? | |
| 16:46:52 | dansmith | bauzas: we should, but I just don't want to tangle that up with my effort to get these objects linked in the database | |
| 16:47:11 | dansmith | because like I said, migration_context below is already skipped, and no migration can work in that case either, AFAIK | |
| 16:47:29 | dansmith | (instance.migration_context, even though the migration object is created at the top) | |
| 16:47:55 | bauzas | dansmith: that's the only concern I have, as said | |
| 16:48:05 | bauzas | if we don't return an exception, then the migration will be accepted | |
| 16:48:12 | dansmith | if we weren't creating the migration object at the top before we check for the node, I wouldn't even have had to ask about this, and it's not really related to the rest of the work | |
| 16:48:21 | dansmith | bauzas: right but it already is | |
| 16:48:42 | bauzas | eventually the instance status could be something like 'resize_confirm' | |
| 16:48:58 | bauzas | without having any migration records attached to it | |
| 16:49:08 | dansmith | bauzas: my point is that the create_migration() will proceed even with a missing node, so it's not like that is stopping us from returning the nopclaim | |
| 16:49:18 | bauzas | and revert resize wouldn't work, since no migration record exists | |
| 16:49:39 | dansmith | right but we can't even get that far without instance.migration_context right? | |
| 16:50:25 | bauzas | I have to admit this is out of my knowledge | |
| 16:51:05 | dansmith | actually, I think the conductor will fail earlier than that even, | |
| 16:53:00 | dansmith | okay you know what.. maybe we're missing more history here | |
| 16:53:19 | dansmith | I think maybe this _create_migration() is from before we had conductor-directed migrations? | |
| 16:53:33 | bauzas | honestly, tomorrow I'll setup a 2-node devstack and try doing crazypants migrations | |
| 16:53:37 | dansmith | so maybe we always pre-create the migration in the conductor (at least for resize) now? | |
| 16:53:55 | bauzas | oh, this is surely existing from 9 years | |
| 16:54:14 | dansmith | so maybe we should actually fail here if migration is None and not fall back to creating the migration | |
| 16:54:19 | dansmith | (need to check the other migration types though) | |
| 16:54:27 | bauzas | from what I recall, this was refactored by ndipanov when he wrote the NUMA migrations | |
| 16:54:47 | bauzas | which were something like Liberty IIRC | |
| 16:55:00 | bauzas | or even Kilo | |
| 16:55:24 | bauzas | way before we had the conductor engines managing the migration records (in Newton or Ocata IIRC) | |
| 16:55:42 | bauzas | so yeah you may be true | |
| 16:56:10 | bauzas | maybe all those codepaths aren't walked anyway, if we fail earlier | |
| 16:56:21 | bauzas | or maybe the conductor fails after | |
| 16:56:22 | dansmith | I'm now realizing there is more in the conductor that needs to be node-id-focused | |
| 16:56:35 | dansmith | yeah my concern is live migration | |
| 16:56:40 | dansmith | and evac | |
| 16:57:01 | dansmith | cross-cell definitely happens in the superconductor because it mirrors the source cell migration | |
| 16:57:41 | bauzas | I think we still rely on conductor-based operations for allocations | |
| 16:57:48 | bauzas | for those two move ops | |
| 16:57:54 | dansmith | either way, the node id part is handled on the destination node, so it actually doesn't change anything | |
| 16:58:11 | dansmith | we'll call to the compute and it will stamp the nodename we gave it on the migration even if it doesn't know that node | |
| 16:58:30 | dansmith | the update half of that | |
| 16:59:42 | dansmith | okay you're probably EOD at this point right? | |
| 16:59:54 | dansmith | I have to run to something else for a bit, but maybe think on it a bit | |
| 17:00:17 | dansmith | I guess I could also put a DNM patch to raise there and make sure at least we don't see it in any of our multinode CI tests | |
| 17:01:16 | bauzas | dansmith: tbh, I was wanting to cycle again on your spec | |
| 17:01:27 | bauzas | I started it before the meeting | |
| 17:01:31 | dansmith | yes, that would also be appreciated | |
| 17:01:35 | bauzas | and then you puzzled me :) | |
| 17:01:40 | dansmith | I updated it for some of this migration stuff last week | |
| 17:01:50 | bauzas | so before I call, I'll get it a round | |
| 17:02:21 | dansmith | ack | |
| 17:06:49 | bauzas | and done | |
| 17:06:59 | bauzas | this was quick, I was at the migration object change | |
| 17:09:03 | bauzas | Uggla: you're next in the review list :) | |
| 17:09:12 | bauzas | but this will be done tomorrow | |
| 17:09:16 | bauzas | see ya folks | |
| 17:26:32 | opendevreview | Dan Smith proposed openstack/nova master: Add compute_id columns to instances, migrations https://review.opendev.org/c/openstack/nova/+/879499 | |
| 17:26:32 | opendevreview | Dan Smith proposed openstack/nova master: Populate ComputeNode.service_id https://review.opendev.org/c/openstack/nova/+/879904 | |
| 17:26:33 | opendevreview | Dan Smith proposed openstack/nova master: Add compute_id to Instance object https://review.opendev.org/c/openstack/nova/+/879500 | |
| 17:26:33 | opendevreview | Dan Smith proposed openstack/nova master: Add dest_compute_id to Migration object https://review.opendev.org/c/openstack/nova/+/879682 | |
| 17:26:34 | opendevreview | Dan Smith proposed openstack/nova master: DNM: Look for disabled node claims https://review.opendev.org/c/openstack/nova/+/879687 | |
| 17:26:34 | opendevreview | Dan Smith proposed openstack/nova master: Online migrate missing Instance.compute_id fields https://review.opendev.org/c/openstack/nova/+/879905 | |
| 19:25:44 | opendevreview | sean mooney proposed openstack/nova master: [WIP] add hypervisor version weigher https://review.opendev.org/c/openstack/nova/+/880231 | |
| 23:14:03 | opendevreview | Karl Kloppenborg proposed openstack/placement stable/yoga: chore: back-merge os-traits version update to placement in stable/yoga to support owner traits https://review.opendev.org/c/openstack/placement/+/880249 | |
| 23:21:18 | opendevreview | Karl Kloppenborg proposed openstack/placement stable/zed: chore: update stable/zed OS-TRAITS version to 2.10.0 https://review.opendev.org/c/openstack/placement/+/880250 | |
| #openstack-nova - 2023-04-13 | |||
| 04:34:23 | manuvakery1 | make the server count to 5 added a huge difference in delete server | |
| 04:34:23 | manuvakery1 | nova.delete_servers: 14.675 sec (Avg) | |
| 04:34:23 | manuvakery1 | nova.boot_servers : 11.941 sec (Avg) | |
| 04:34:23 | manuvakery1 | Hi, I have a 2 node cluster and I am running a rally scenario against it to create and delete servers. When i set the server count to 2 I see the response time as follows | |
| 04:34:23 | manuvakery1 | Query repost: | |
| 04:34:24 | manuvakery1 | nova.delete_servers: 42.988 sec (Avg) | |
| 04:34:24 | manuvakery1 | nova.boot_servers: 18.803 sec (Avg) | |
| 04:34:26 | manuvakery1 | the compute nodes are not under any load | |
| 04:34:26 | manuvakery1 | is this expected? | |
| 11:06:47 | opendevreview | Jorge San Emeterio proposed openstack/nova master: WIP: Testing whether tests on bug#1998148 still fail. https://review.opendev.org/c/openstack/nova/+/880135 | |
| 13:34:57 | opendevreview | Jorge San Emeterio proposed openstack/nova master: WIP: Testing whether tests on bug#1998148 still fail. https://review.opendev.org/c/openstack/nova/+/880135 | |
| 13:37:15 | opendevreview | sean mooney proposed openstack/nova master: add hypervisor version weigher https://review.opendev.org/c/openstack/nova/+/880231 | |
| 13:38:11 | opendevreview | sean mooney proposed openstack/nova master: add hypervisor version weigher https://review.opendev.org/c/openstack/nova/+/880231 | |
| 13:39:36 | opendevreview | sean mooney proposed openstack/nova master: add hypervisor version weigher https://review.opendev.org/c/openstack/nova/+/880231 | |
| 13:40:48 | opendevreview | sean mooney proposed openstack/nova master: add hypervisor version weigher https://review.opendev.org/c/openstack/nova/+/880231 | |
| 13:41:20 | sean-k-mooney | bauzas: fyi that should now be ready to go ^ | |
| 13:55:28 | bauzas | sean-k-mooney: yup, I saw the updates :) | |
| 13:56:15 | sean-k-mooney | the ping was more to say im done updatign it if you want to review | |
| 13:57:38 | sean-k-mooney | bauzas: once you are done with that i would also liek to land https://review.opendev.org/q/topic:sqlalchemy-20+project:openstack/nova+status:open if we can form stephenfin | |
| 13:58:03 | sean-k-mooney | gibi: ^ melwitt: ^ if either of ye can review those that would also be good | |
| 14:20:07 | stephenfin | sean-k-mooney: Speaking of, just replied on https://review.opendev.org/c/openstack/nova/+/860850 | |
| 14:22:29 | kashyap | gibi: bauzas: When you can, can you have a look at this from Sean: https://review.opendev.org/c/openstack/nova/+/880231 | |
| 14:23:23 | bauzas | kashyap: already know it :) | |
| 14:23:40 | kashyap | Thank you; I know, I'm slow in catching up :) | |
| 15:16:47 | opendevreview | Merged openstack/os-traits master: Update python testing as per zed cycle testing runtime https://review.opendev.org/c/openstack/os-traits/+/841682 | |
| 16:47:37 | dansmith | bauzas: AFAICT, we don't ever hit this trap in any of our CI jobs: https://review.opendev.org/c/openstack/nova/+/879687/5/nova/compute/resource_tracker.py | |
| 17:09:10 | sean-k-mooney | i think the only way we could try and create a claim on a disabled node is if there was a race | |
| 17:09:37 | sean-k-mooney | specificaly if between the time we selected the node in the schdluer and before the node reached the compute we updated the service to disabled | |
| 17:10:35 | sean-k-mooney | assuming self.diabled is the compute service disabled value? | |
| 17:24:04 | dansmith | sean-k-mooney: no | |