| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-06-10 | |||
| 13:29:42 | sahid | but yes previously it was like that | |
| 13:30:06 | sean-k-mooney | right but im saying i dont think the feature added in zed is good for scaling | |
| 13:30:42 | sean-k-mooney | or at least it not implemted how i would do that | |
| 13:30:47 | sahid | it is good from an user point of view | |
| 13:31:11 | sean-k-mooney | not if it causes schduign issue by an expoltion of aggreates meaning they cant use it | |
| 13:32:30 | sahid | interesting point :-) perhaps the design used by placement is not so ideal ... ;D | |
| 13:33:01 | sean-k-mooney | or neutron is not useing it properly | |
| 13:33:17 | sean-k-mooney | sahid: im really not inclidned to extend nova ot supprot this neutron feature | |
| 13:33:36 | sean-k-mooney | to me its breakign the desing of how l3 routed networks was intended to work | |
| 13:38:37 | sahid | sean-k-mooney: not really sure, the datastructure in neutron was originaly exposed to handle multiple-segments | |
| 13:39:39 | sahid | but anyway, back in the fix proposed, it is not going to change nova behavior for small deployments and helps large deployments | |
| 13:39:40 | sean-k-mooney | neutorn often add extentsion without addign the nova support. | |
| 13:39:58 | sean-k-mooney | for example noava still does nto supprot the multi physnet extention i belive | |
| 13:40:30 | sahid | sean-k-mooney: yes in some situation it always take the first subnet | |
| 13:40:52 | sahid | I this the issue is when you specify a fixed ip when creating the vm | |
| 13:40:56 | sean-k-mooney | sahid: it is a change in behavior as today we only make 1 palcmenet query per schduilng request (ignoriang a deprected config option) | |
| 13:41:18 | sahid | sean-k-mooney: it will alqys be the same one request | |
| 13:42:01 | sahid | the limit of 200 is basically the limit we have observed | |
| 13:42:08 | sahid | (about 220) | |
| 13:42:09 | sean-k-mooney | the fix is raisign an exaction based on a hard coded max query size | |
| 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 | gibi | that is not AND, that is OR | |
| 14:13:48 | sean-k-mooney | well member_of=in:% | |
| 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 | |