| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-09-04 | |||
| 13:37:20 | sean-k-mooney | at least not in this context | |
| 13:43:24 | jkulik | regarding scale: we have a patch downstream that makes the server-group listing much faster with lots of server-groups around (e.g. when an admin lists them all). We changed the DB queries to actually do the pagination instead of doing it after fetching _all_ DB objects. | |
| 13:43:52 | jkulik | Would that be something I can propose just like that - with a bug-report - or does that need some special process? | |
| 13:45:03 | dansmith | bauzas: what do you want me to look at? | |
| 13:45:17 | sean-k-mooney | jkulik: yes for master, it would depend on the actual cahnge if it woudl be backprotable | |
| 13:45:41 | bauzas | not specifically looking at, just being there for accepting a solution | |
| 13:46:02 | jkulik | sean-k-mooney: I don't need backports, as we have it running. would just be nice to share if possible :) | |
| 13:46:08 | sean-k-mooney | jkulik: its alwasy valid to reprot performance bug and fix them on master. weither its backproabel will depend on if your using any feature form newer sqlachme ectra or if ti has any other upgrade impact | |
| 13:46:23 | bauzas | tl;dr: by 2.88, we now call every compute if you call hypervisor detailed list and we await the compute OS for returning us an uptime | |
| 13:46:47 | dansmith | bauzas: oh yeah I read the bug | |
| 13:52:00 | dansmith | bauzas: seems like the solution there _has_ to be to start reporting uptime somewhere we can persist it and fix that to query that location instead of making the (egads) rpc call | |
| 13:53:21 | bauzas | something that could then be backportable to Epoxy if we can | |
| 13:53:22 | dansmith | I agree we can't just stop doing it, and making it only present if you don't list, or don't list enough is also not great | |
| 13:53:51 | bauzas | if we can't backport it, then 2.88 will have the same issue | |
| 13:54:23 | sean-k-mooney | the hypervior api is already doing an sqlquery to the compute nodes db table and geting the stats | |
| 13:54:45 | sean-k-mooney | we can put it in that json bob which is entirly unversioned today | |
| 13:54:55 | sean-k-mooney | so if we want to backprot it we can | |
| 13:55:10 | dansmith | yeah was just going to say, ComputeNode has several places we could stash it I think | |
| 13:55:41 | dansmith | but, it will be absent during an upgrade so we probably need to fall back to the old behavior for nodes that aren't upgraded | |
| 13:55:59 | sean-k-mooney | the bit im currntly checking is it purly the time or are we includign the load info too | |
| 13:56:32 | sean-k-mooney | im hoping it just the former but some of our docs implies the latter | |
| 13:57:47 | dansmith | doesn't much matter | |
| 13:58:00 | sean-k-mooney | well its just changes what we need to stash | |
| 13:58:14 | sean-k-mooney | but ya it does not matther in the long run | |
| 13:58:20 | sean-k-mooney | the depercated api https://docs.openstack.org/api-ref/compute/#show-hypervisor-uptime-deprecated | |
| 13:58:22 | bauzas | if we can stuff the uptime value somewhere, sure | |
| 13:58:28 | sean-k-mooney | was the rwaw output of uptime | |
| 13:58:41 | bauzas | without needing to have a new DB field, cool | |
| 13:58:41 | sean-k-mooney | but i think in 2.88 its just the time in second | |
| 13:59:01 | sean-k-mooney | but i just need to check that on a master devstack to confirm | |
| 13:59:01 | bauzas | sean-k-mooney: my concern is not with hypervisor show, again. | |
| 13:59:12 | sean-k-mooney | bauzas: yep i know | |
| 13:59:19 | bauzas | if we continue to call the OS for hypervisor show, meh | |
| 13:59:47 | bauzas | even if that's bad, that's not really an issue | |
| 14:00:18 | bauzas | but, if we are able to persist some uptime value for detailed-list, then of course we could modify hypervisor show | |
| 14:01:08 | jkulik | I don't see anything parsing the `uptime` command output, so it seems to be indeed the full one | |
| 14:01:17 | opendevreview | ribaudr proposed openstack/nova stable/2024.2: Reproducer for bug 2114951 https://review.opendev.org/c/openstack/nova/+/959525 | |
| 14:01:18 | opendevreview | ribaudr proposed openstack/nova stable/2024.2: Fix bug 2114951 https://review.opendev.org/c/openstack/nova/+/959526 | |
| 14:01:39 | sean-k-mooney | it is yes i just confirmed that | |
| 14:02:17 | sean-k-mooney | https://paste.opendev.org/show/bJ59n0G92NoP06hDsfAu/ | |
| 14:02:38 | jkulik | including a newline? :D | |
| 14:02:54 | sean-k-mooney | so i cant jsut retirn the tiem btu i can just stash the value in the stats and skip the rpc call if its there | |
| 14:03:06 | sean-k-mooney | and ya it also has a leadign space | |
| 14:03:43 | dansmith | can't what? | |
| 14:04:09 | sean-k-mooney | sorry, i cant just retive the time since boot and return that value in seconds | |
| 14:04:18 | bauzas | dansmith: given we return the whole uptime, we can't just calculate it | |
| 14:04:20 | sean-k-mooney | dansmith: jkulik suggeted doing that to aovid calling the uptime command | |
| 14:04:46 | sean-k-mooney | in a futre microverion we can jsut not in the backportabel fix | |
| 14:04:48 | dansmith | ack, sorry I just wasn't getting that meaning from the translation | |
| 14:05:02 | bauzas | this is honestly super weird to have that in our API | |
| 14:05:23 | sean-k-mooney | ya that was a pebkac, my keyboard was not aligent so i was typing off center | |
| 14:05:26 | bauzas | I don't disagree with sean-k-mooney when he said we should have stopped to provide it | |
| 14:05:53 | bauzas | but now we have it in detailed-list instead of a specific API call... so that's even worst | |
| 14:06:11 | dansmith | yep, just got to fix it, not the end of the world | |
| 14:08:44 | gibi | ^^ +1 | |
| 14:09:51 | bauzas | dansmith: using a periodic that would cache some raw value ? | |
| 14:10:33 | dansmith | bauzas: just use whatever periodic we use to update the metrics and/or RT IMHO | |
| 14:12:04 | bauzas | I guess we would need to modify the API docs to say that the uptime value can be a bit stale | |
| 14:12:57 | dansmith | does the api guarantee or suggest that it's live? | |
| 14:13:28 | jkulik | with the call taking 40s, I guess it wasn't live already for a lot of them :D | |
| 14:13:39 | dansmith | jkulik: excellent point :D | |
| 14:19:26 | bauzas | dansmith: good point, /me looks at the existing docs | |
| 14:21:35 | bauzas | well, nothing is specifically said, so I guess we can just fix it silently | |
| 14:21:44 | dansmith | :) | |
| 14:23:12 | bauzas | "stats" can be a good CN object field candidate for persisting the value | |
| 14:23:22 | bauzas | that's a dict of strings | |
| 14:23:31 | bauzas | sean-k-mooney: ^ | |
| 14:24:33 | dansmith | IIRC both stats and metrics are actually used for something, but I don't remember all the details | |
| 14:25:11 | opendevreview | Rajesh Tailor proposed openstack/nova stable/2024.2: Fix case sensitive comparison https://review.opendev.org/c/openstack/nova/+/959534 | |
| 14:27:29 | bauzas | dansmith: I just remember now where stats is called | |
| 14:27:49 | sean-k-mooney | bauzas: you realise i litrally said that many many times | |
| 14:27:59 | bauzas | first good news, that's a periodically updated field using the RT update_avail_resources() | |
| 14:28:55 | sean-k-mooney | dansmith: the api refe just say "The total uptime of the hypervisor and information about average load." | |
| 14:29:02 | bauzas | sean-k-mooney: well, then I missed the many times when you said that | |
| 14:29:11 | sean-k-mooney | and everythin else is cached at the period of the update_aviable_resouces perodic | |
| 14:30:01 | dansmith | sean-k-mooney: yeah, it's unfortunate that's in our api ref at all, especially since all that is very os-specific | |
| 14:31:22 | bauzas | anyway, sounds like we have a plan | |
| 14:31:33 | sean-k-mooney | that what the deprecate api says and thsi is what the hypervior deail api say ```The total uptime of the hypervisor and information about average load. Only reported for active hosts where the virt driver supports this feature``` | |
| 14:31:40 | bauzas | we can fix 2.88 and drop that field in a later microversion | |
| 14:32:07 | bauzas | if ops complain about that missing field, then we can tell them to use whatever tool for calling the OS by themselves | |
| 14:32:19 | sean-k-mooney | bauzas: just for extra context gibi and i talk about this a few weeks ago and before that i talk to others about this too. | |
| 14:32:22 | bauzas | ansible, prometheus or nagios, I don't care | |
| 14:32:36 | sean-k-mooney | so this is not the first converation we have had upstream or downstream on how to fix this | |
| 14:32:37 | bauzas | few weeks ago, I was on PTO :) | |
| 14:32:42 | sean-k-mooney | you were on pto for the prior ones | |
| 14:33:12 | bauzas | so I had no context, but the good thing is that we went to the same solution, isn't that good ? | |
| 14:33:16 | sean-k-mooney | yes im aware but we were not activly fixing it because of time pressuer for FF | |
| 14:33:29 | bauzas | I think this is a reasonable bugfix for RC1 | |
| 14:33:54 | bauzas | not RC2 of course as that's a not a Flamingo regression per say | |
| 14:34:00 | bauzas | but I know you know | |
| 14:34:01 | dansmith | bauzas: ++ for rc1 | |
| 14:34:03 | sean-k-mooney | maybe ill see if i can fix it tomrrwo or at least ahve a draft for review | |
| 14:34:07 | dansmith | if it's reasonable to do so | |
| 14:34:43 | bauzas | extending the Stats class by stuffing a new keypair sounds a reasonable solution | |
| 14:35:05 | bauzas | but sean already found the solution, I leave him tell whether he's OK | |
| 14:36:00 | sean-k-mooney | lets just review the patch and we can backprot as normal. if its ready for rc1 cool if not we chan cherry pick after we release 2025.2 | |
| 14:38:01 | dansmith | sean-k-mooney: there's no patch up to review yet, right? just one in your head? | |
| 14:38:21 | sean-k-mooney | exactly im still chekcing if any other virtdriver ever supproted this rpc | |
| 14:39:28 | sean-k-mooney | basicly im not sure i want to stash a sentenal value in the stats for other virt driver | |
| 14:40:08 | sean-k-mooney | if i can show its only libvirt by inspecting the code i can skip that and gard the rpc call behidn both the virt type adn the absence of the key in the dict | |