| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-06-10 | |||
| 13:42:25 | sean-k-mooney | that is deployment sepciric | |
| 13:42:37 | sean-k-mooney | we have a custoemr with a similar probelem for a diffent reason | |
| 13:42:45 | sean-k-mooney | they had ~600 aggrates per host | |
| 13:42:57 | sean-k-mooney | before they hit the limit in our installer | |
| 13:43:19 | sean-k-mooney | so add ign a hardcoded check like this would 1 break them and 2 break the workaround they want to use | |
| 13:43:30 | sean-k-mooney | which is to increase the query string limit | |
| 13:43:40 | sean-k-mooney | which they tested and works for them | |
| 13:44:02 | sahid | not sure to understand | |
| 13:44:18 | sahid | the fix split the number of uuid passed to the uri | |
| 13:44:27 | sahid | if it is more than 200 | |
| 13:44:57 | sean-k-mooney | which intoduces multipel quereis | |
| 13:45:32 | sahid | only for env with more than 200 | |
| 13:45:41 | sean-k-mooney | gibi: you were looking into this on our side | |
| 13:46:03 | sean-k-mooney | gibi: didnt you mention we doen seem to need to call _get_sharing_providers in teh comptue manager today ? | |
| 13:46:10 | sahid | so for most of the deployment we don't change the behavior | |
| 13:46:19 | sahid | and for the other, we fix their issue | |
| 13:46:50 | sean-k-mooney | sahid: we cant use a constant fo 200 | |
| 13:47:03 | sean-k-mooney | if we were to do this we need to have it be configurable | |
| 13:47:47 | sean-k-mooney | but the reason im pinign gibi into the conversation is i think they determined this was not curestly used in compute manager where its called | |
| 13:48:38 | sean-k-mooney | sahid: https://bugzilla.redhat.com/show_bug.cgi?id=2283075 | |
| 13:48:52 | sean-k-mooney | sahid: we were considering if we coudl remove this funciton entirely | |
| 13:49:28 | sean-k-mooney | the pagingation fo this query is perhaps somethign we coudl do but i think we would need to make the size configurabel if we did | |
| 13:50:02 | sahid | hum... interesting yes | |
| 13:50:51 | gibi | sean-k-mooney: what I remember it is used in the resource tracker to cache the sharing realtionship into the internal provider_tree. | |
| 13:51:29 | sean-k-mooney | right and today im not sure that cache is actully used in the resouce tracker for anything | |
| 13:52:27 | sean-k-mooney | sahid: if the bug you reported is just with the compute manager/resouce tracker and not the sechduler | |
| 13:52:53 | sean-k-mooney | i guess your propsoed patch is not a behaivor chage when scheduling | |
| 13:53:13 | sean-k-mooney | so the bug you reported is proably valid | |
| 13:53:45 | sean-k-mooney | what im not sure would be valid is making muliple request ot the allcoation candiate api in a similar way | |
| 13:54:01 | sean-k-mooney | that is what i tought you were suggesting but looking at your patch its not | |
| 14:00:48 | gibi | I belive what is proposed is heavy behavior change. Today the returned RP needs to be in all the aggregates passed, in the new code it only needs to be in a subset of those aggregates. | |
| 14:01:20 | gibi | potentially in the last loop the RP only need to be in the last aggregate | |
| 14:01:47 | gibi | we cannot paginat the query itself | |
| 14:02:16 | sean-k-mooney | what is proposed is not affecting the allocation candiate query | |
| 14:02:41 | sean-k-mooney | at least that is what i understood sahid was asserting | |
| 14:03:10 | gibi | it affects the meaning of _get_sharing_providers | |
| 14:03:24 | gibi | we cannot just do that bindly | |
| 14:03:33 | sean-k-mooney | yes i know | |
| 14:03:59 | sean-k-mooney | but but what im saying is we shoudl not be calling that when generating the allcoation candiate request | |
| 14:04:55 | sean-k-mooney | but your right that we have to be carful which is why i was orginally pushing back on shaids approch. but if it only affecting the resouce-tracker cache that less fo a concern | |
| 14:06:33 | gibi | even if it is only used in the tracker we cannot allow to have wrong data in the tracker, maybe tomorrow a new feature will start using that data. | |
| 14:06:47 | sean-k-mooney | sure | |
| 14:07:01 | sean-k-mooney | but i never said we shoudl | |
| 14:07:03 | gibi | so I'm hard -1 in the current version of the patch | |
| 14:08:00 | gibi | I guess I'm not arguing against you but against the patch itself :) | |
| 14:08:02 | sean-k-mooney | so the current verison of the patch should not chagne the behaivor | |
| 14:08:19 | sean-k-mooney | either we would loop over all the aggreates and return the comined set | |
| 14:08:39 | sean-k-mooney | or raise the same excption we raised previosusly | |
| 14:09:11 | sean-k-mooney | the main issues is that there coudl be a chagne in the assocation during the iterations | |
| 14:09:15 | gibi | no, that is my point, we cannot just combine the results | |
| 14:09:41 | sean-k-mooney | this is the set of sharing resouce providers that share with this host | |
| 14:09:48 | sean-k-mooney | why can we not just combine? | |
| 14:09:57 | gibi | no this is all the RPs that in an aggregate | |
| 14:10:10 | gibi | this is not just a single RP | |
| 14:10:20 | sean-k-mooney | no | |
| 14:10:24 | sean-k-mooney | "/resource_providers?member_of=in:%s&required=%s" % ( | |
| 14:10:26 | sean-k-mooney | aggs, os_traits.MISC_SHARES_VIA_AGGREGATE) | |
| 14:10:46 | sean-k-mooney | this is only the resouce providers in teh aggrate with the os_traits.MISC_SHARES_VIA_AGGREGATE triat | |
| 14:10:50 | sean-k-mooney | so only the sharing providers | |
| 14:10:55 | gibi | but not for a single host | |
| 14:11:13 | gibi | there is no 'this host' filter | |
| 14:11:16 | gibi | in the queru | |
| 14:11:18 | gibi | in the query | |
| 14:11:21 | sean-k-mooney | there does not need to be | |
| 14:11:38 | sean-k-mooney | we are asking for all the sharing provider in the aggrate our host is a member of | |
| 14:11:47 | sean-k-mooney | that what the in: query is doing | |
| 14:12:16 | sean-k-mooney | the agg_uuids var is the list of aggreates the current host is a member of | |
| 14:13:12 | sean-k-mooney | we tyically expect only one RP in an aggrate to have MISC_SHARES_VIA_AGGREGATE althogh that is not strictly required | |
| 14:13:34 | gibi | ahh I misremembered the meaning of 'in:' the query | |
| 14:13:48 | sean-k-mooney | well member_of=in:% | |
| 14:13:48 | gibi | that is not AND, that is OR | |
| 14:14:21 | sean-k-mooney | so member_of is "filter by aggartge" and in is the same meaing as container or in in python | |
| 14:14:42 | sean-k-mooney | i.e. filter by aggrage where the aggreate uuid is in "list of uuids" | |
| 14:15:05 | sean-k-mooney | and we prefilter the aggreate list to only the ones that this host is in | |
| 14:16:19 | sean-k-mooney | im still -1 on sahid patch as is because i woudl like to see a repoducer test and then test coverage for this | |
| 14:16:44 | sean-k-mooney | but i think exteranlly the beahivor is the same but internally there is now pagination | |
| 14:17:16 | sean-k-mooney | i dont think hardcodeing the pagination to 200 uuids however is a good thing | |
| 14:17:31 | sean-k-mooney | sicne that depend on yoru loadbalnacer config | |
| 14:21:03 | sahid | sorry guys I had to switch to a meeting. | |
| 14:21:13 | sahid | fair enough I will add a configurable | |
| 14:24:40 | gibi | yeah I was wrong as I thought we query for PRs that are in _all_ the aggregates but we query for RPs that are in _any_ of the aggregates. I will drop my -1 | |
| 14:25:28 | sean-k-mooney | oh you tought the in was an and not an or just read that | |
| 14:25:39 | sean-k-mooney | ya its an or of all the uuids in the list | |
| 14:26:14 | sean-k-mooney | in is basically AnyOf rather then AllOf | |
| 14:27:12 | gibi | yeah | |
| 14:27:36 | gibi | and as it is an OR I agree that we can do multiple queries and combine the results | |
| 14:28:03 | sean-k-mooney | im kind of ok in the preodics | |
| 14:28:39 | sean-k-mooney | im not sure that is ok for the schduler but the schduler scales the query based on teh guest request | |
| 14:28:44 | sean-k-mooney | not the number of aggrates that exist | |
| 14:28:49 | sean-k-mooney | so i think tis less of a problem there | |
| 15:19:08 | opendevreview | Will Szumski proposed openstack/nova stable/2023.2: Use configuration for single enabled mdev type https://review.opendev.org/c/openstack/nova/+/921686 | |
| 15:19:37 | opendevreview | Will Szumski proposed openstack/nova stable/2023.1: Use configuration for single enabled mdev type https://review.opendev.org/c/openstack/nova/+/921687 | |
| 15:50:26 | opendevreview | Sahid Orentino Ferdjaoui proposed openstack/nova master: scheduler: fix _get_sharing_providers to support unlimited aggr https://review.opendev.org/c/openstack/nova/+/921665 | |
| 16:07:03 | melwitt | dansmith: here's one of the kernel panics I got recently https://fbf109e10bffd8a657b2-ace28e47b1213bdab56305960a7f8056.ssl.cf5.rackcdn.com/919986/2/check/tempest-integrated-compute/4b51a9c/testr_results.html | |
| 16:07:49 | melwitt | and this https://86989508e8a17de7a050-16e5a6d00ae68a8470fe17c7f38ebfca.ssl.cf2.rackcdn.com/921586/1/check/tempest-integrated-compute-rbac-old-defaults/8980965/testr_results.html | |
| 16:08:06 | dansmith | okay yeah that first one looks like what we had seen before | |
| 16:08:18 | dansmith | and the second | |
| 16:10:19 | melwitt | this one also looks like https://5edd60feb751137d1041-26db3adf591a82ec37c96a7d3180086f.ssl.cf5.rackcdn.com/870932/41/check/nova-next/be498dc/testr_results.html | |
| 16:11:00 | melwitt | this one is different https://storage.bhs.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_29d/919989/4/check/nova-ceph-multistore/29db296/testr_results.html | |
| 16:11:42 | melwitt | (soft lock up ^) | |