| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-03-05 | |||
| 14:01:49 | gibi | bauzas: so it is only in ussuri but not in train | |
| 14:02:08 | bauzas | gibi: correct, that's a PEBKAC when I said Train | |
| 14:02:37 | bauzas | oh wai | |
| 14:02:40 | gibi | sean-k-mooney: it is like we don't know yet how the alembic migration will look like so I think we have to do the safe move and add reservation | |
| 14:02:43 | bauzas | man, this is confusing | |
| 14:02:47 | bauzas | https://review.opendev.org/c/openstack/nova/+/686411/2/nova/tests/functional/db/api/test_migrations.py | |
| 14:03:08 | bauzas | this was merged just after the branch but we name them train migrations | |
| 14:03:29 | bauzas | so, hence my confusion, placeholders go last | |
| 14:03:39 | bauzas | and just after we cut | |
| 14:03:40 | gibi | yeah it is like placeholder for migration backported from ussuri to train | |
| 14:03:47 | gibi | so both name can be correct :) | |
| 14:03:48 | bauzas | gibi: so you were right, we should hold until the branhc | |
| 14:03:58 | sean-k-mooney | gibi: yep we should for that reason and because whilte stephenfin want to do the move next cycle something might come up which means we cant finish it | |
| 14:04:10 | sean-k-mooney | so we need to prepare for backport in anycase | |
| 14:04:13 | gibi | sean-k-mooney: correct | |
| 14:04:40 | bauzas | sean-k-mooney: gibi: like I said to stephenfin, before discussing it at the PTG, I'd like to dig why we didn't like that before | |
| 14:07:08 | sean-k-mooney | alembic | |
| 14:07:17 | bauzas | yup | |
| 14:07:34 | sean-k-mooney | it was manly because we did not feel the effort of the move was worth it similar to removing mox | |
| 14:07:44 | bauzas | I recall this | |
| 14:07:51 | bauzas | but IIRC there were other concerns | |
| 14:08:02 | bauzas | that's what I'd like to dif | |
| 14:08:03 | sean-k-mooney | but like mox that ment the nova team was signing up to maintian sqlalchemy-migrate indefinetly | |
| 14:08:05 | bauzas | dig* | |
| 14:08:24 | bauzas | sure, but this prove to work ? | |
| 14:08:42 | bauzas | we would just pull another dependency we loose control | |
| 14:08:46 | bauzas | over their releases | |
| 14:08:56 | sean-k-mooney | bauzas: we are using a dep that neutron uses already | |
| 14:09:04 | bauzas | I know | |
| 14:09:23 | sean-k-mooney | bauzas: and the maintiner of sqlachmey asked us to move | |
| 14:09:25 | bauzas | but that just increases the potential flaw, it doesn't prove it works | |
| 14:09:40 | sean-k-mooney | it does prove it works they have used it for years | |
| 14:10:01 | sean-k-mooney | im pretty sure neutron moved beofre we created placment | |
| 14:15:48 | bauzas | oh yes, I remember | |
| 14:15:54 | sean-k-mooney | neutron dont seam to have placeholder migration anymore https://github.com/openstack/neutron/tree/b04dfcee192cf7ddcb69854e760739fdcfa7839e/neutron/db/migration/alembic_migrations/versions | |
| 14:16:03 | bauzas | I actually never remember neutron using sqalchemy-migrate | |
| 14:16:15 | bauzas | even when they were named quantum | |
| 14:16:16 | sean-k-mooney | and they have tooling to auto genrate the migrations | |
| 14:16:27 | sean-k-mooney | bauzas: i tought quantum did but maybe not | |
| 14:16:51 | bauzas | well, i played with quantum with the Folsom release, and I don't think they were using migrate | |
| 14:17:01 | bauzas | but that's been a while | |
| 14:17:11 | sean-k-mooney | actully hevan used it alredy | |
| 14:17:15 | sean-k-mooney | so i guess not | |
| 14:17:41 | bauzas | they were incubated during Essex, so I didn't use it in my prod | |
| 14:17:52 | bauzas | can't speak for Essex | |
| 14:18:19 | bauzas | oh gosh, remembrance times... | |
| 14:20:25 | sean-k-mooney | looking at the eol tags | |
| 14:20:36 | sean-k-mooney | up to fulsom they had not migration in repo | |
| 14:21:19 | sean-k-mooney | so the first migration they added was grizzly and that used alembic | |
| 14:21:35 | sean-k-mooney | so for as long as they had migghration yes they used alembic to handel it | |
| 14:21:58 | bauzas | glad to know my memory haven't vanished :) | |
| 14:22:06 | bauzas | hasn't* | |
| 14:22:09 | sean-k-mooney | if it woks well enough for neutron which allows exptions to also have addtional db tables i think it should be flexably enough ot use in nova | |
| 14:22:24 | bauzas | again, this is not news | |
| 14:22:35 | sean-k-mooney | the main issue for use will be learning how to write them correctly and review them | |
| 14:22:36 | bauzas | that's my point | |
| 14:23:19 | bauzas | neutron uses it for ages, but by the times we discussed this for nova, we nacked the idea a couple of times, so I just want to refresh my memory before jumping into conclusions | |
| 14:23:57 | sean-k-mooney | sure but for me continuing to use it is like continuting ot use mox or python2 | |
| 14:24:28 | sean-k-mooney | sure they work but it woudl be better to simply the dendencis fo openstack an converg on alembic | |
| 14:37:53 | openstackgerrit | sean mooney proposed openstack/nova master: support per port numa policies with sriov https://review.opendev.org/c/openstack/nova/+/773792 | |
| 15:09:15 | melwitt | lyarwood, elod: thank you for the reviews :) fyi the stable/ussuri changes for the same bug, I stacked them on top of this other bug (2 patches) fix to eliminate merge conflicts. I figured it was a useful fix for stable anyways https://review.opendev.org/c/openstack/nova/+/777216 and https://review.opendev.org/c/openstack/nova/+/777217 | |
| 15:40:50 | claudiub | Hello, so, I have been looking a bit into the NUMA node 0 overcommit issue I was having. As a reminder, I had ~400 instaHello, so, I have been looking a bit into the NUMA node 0 overcommit issue I was having. As a reminder, I had ~400 instances in NUMA node 0, while NUMA node 1 was mostly empty. | |
| 15:40:58 | claudiub | Turns out, there seems to be an issue when live-migrate / host-evacuate-live is called with a target destination. | |
| 15:41:07 | claudiub | It seems that the CPU allocation and the memory allocation ratio is ignored in this case, leading to the overcommit, and even OOM, apparently. | |
| 15:41:24 | claudiub | So, from what I saw, the live_migrate task will eventually call check_can_live_migrate_destination on the destination node (https://github.com/openstack/nova/blob/c8a6f8d2e502e139c2212fb20cc0f89de816aeb1/nova/compute/manager.py#L7931), | |
| 15:41:33 | claudiub | which will then call _live_migration_clain and then self.rt.live_migration_clain. In there, a new claims.MoveClaim is created (https://github.com/openstack/nova/blob/c8a6f8d2e502e139c2212fb20cc0f89de816aeb1/nova/compute/resource_tracker.py#L298), | |
| 15:41:34 | stephenfin | You mean e.g. 'openstack server migrate --live HOST SERVER' ? | |
| 15:41:51 | claudiub | hich will then call hardware.numa_fit_instance_to_host to get the new instance_topology (https://github.com/openstack/nova/blob/c8a6f8d2e502e139c2212fb20cc0f89de816aeb1/nova/compute/claims.py#L138), which will then set it in self.claimed_numa_topology. In the hardware.numa_fit_instance_to_host call, | |
| 15:42:02 | claudiub | because the given limits is None, there is no check that CPU / memory is not overcommited in numa cell 0, so it gets scheduled there, since it's first match. https://github.com/openstack/nova/blob/5b52db409b00e6257e83598efa0d8528ebe380e0/nova/virt/hardware.py#L1168 | |
| 15:42:09 | claudiub | And indeed, the limits are None, from what I've seen in the extra logging I've added. | |
| 15:42:24 | claudiub | stephenfin: yep. :) | |
| 15:42:48 | stephenfin | How about if you do 'openstack server migrate --live-migration --host HOST SERVER'? | |
| 15:43:09 | stephenfin | '--live HOST' bypasses the scheduler which means the NUMATopologyFilter never triggers to populate limits | |
| 15:43:32 | stephenfin | (it's deprecated and we're removing it in the next version of OSC, fwiw) | |
| 15:43:43 | stephenfin | in fact the patch to do so merged yesterday :) | |
| 15:46:03 | claudiub | stephenfin: sorry, it was openstack server migrate --live --host dest_host --block-migration inst_uuid | |
| 15:46:22 | stephenfin | ah, sorry, okay, that means this is likely an issue so | |
| 15:47:13 | stephenfin | claudiub: I won't have time to look at this in depth today (on RFE review/rework duty pending feature freeze) but I could help work on this the end of next week | |
| 15:47:21 | stephenfin | any chance you could open a bug? | |
| 15:47:30 | claudiub | also, I saw that the NUMATopologyFilter was populating the limits. Why is that? Couldn't that just be initialized in the HostState anyways? It's not dependent on a request anyways. | |
| 15:48:05 | claudiub | stephenfin: will do. :) | |
| 15:49:16 | claudiub | I was wondering if, on the nova-compute side, if we get a check_can_live_migrate_destination call from somewhere and we don't get any limits, couldn't we just use the CPU / mem allocation ratios in the nova.conf file as the limits? | |
| 15:50:14 | stephenfin | I assume we could. I think the reason we package the limits up is so we can check them in the scheduler (since we don't want to use the limits of the nova-scheduler service) | |
| 15:53:51 | claudiub | ok then, will open up that issue, and I'll check how it goes if we use the nova.conf cpu / mem allocation ratios as limits if no limits are given. :) | |
| 15:54:15 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Consolidate device detach error handling https://review.opendev.org/c/openstack/nova/+/778978 | |
| 15:54:20 | stephenfin | sounds good | |
| 15:54:43 | stephenfin | lyarwood: Can you look at my two replies here before I pushed the rebased series? https://review.opendev.org/c/openstack/nova/+/775689 | |
| 15:55:00 | sean-k-mooney | claudiub: the limit not being poplulated is the corrct behaivor | |
| 15:55:00 | lyarwood | stephenfin: yup can do, just need to grab a tea first | |
| 15:55:03 | stephenfin | ack | |
| 15:55:50 | sean-k-mooney | if you do not have hw:cpu_policy=dedicated or hw:mem_page_size set then we dont really expect it to be set | |
| 15:55:59 | sean-k-mooney | although there is a sperate bug fix that is relevent | |
| 15:56:41 | sean-k-mooney | claudiub: https://review.opendev.org/c/openstack/nova/+/769614 | |
| 15:57:16 | sean-k-mooney | claudiub: the limits are depended on the request at least for cpus | |
| 15:58:09 | elod | melwitt: the functional job failure seems legit in the patch: https://review.opendev.org/c/openstack/nova/+/777218/ | |
| 15:58:19 | sean-k-mooney | claudiub: none of the behviaor your are discibing is new to me | |
| 15:58:51 | sean-k-mooney | claudiub: this is why i said added hw:numa_nodes with no other numa parmateters is not valid | |
| 15:59:00 | elod | melwitt: otherwise it would be clean & OK o:) | |
| 15:59:29 | melwitt | elod: oh, sorry I completely missed that and only saw the pep8 cherry pick fail. thanks for pointing it out, I will fix :) | |
| 16:00:01 | sean-k-mooney | claudiub: the limits are not in general related to the nova.conf allocation ratios | |