Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-08-27
14:43:43 sean-k-mooney i think nova always handels that on its behalf
14:43:51 sean-k-mooney but valid question
14:44:05 dansmith okay I thought in some cases it would.. wasn't that the point of that blueprint?
14:44:19 dansmith I wasn't really involved in it so I dunno
14:44:26 gibi it was definitely before port resource request as nested a_c is a prerequisite for that
14:44:30 sean-k-mooney so there was the open question about modifying QOS policies for bound ports
14:44:45 sean-k-mooney which could change the allcoaitons
14:44:46 gibi the only case neutorn modifies allocation ^^
14:44:55 sean-k-mooney but i dont think we support that yet do we
14:45:01 gibi we do
14:45:05 sean-k-mooney oh ok
14:45:28 gibi you can change QoS policy on a bound port, neutron will try to amend the instance allocation according to the new policy
14:45:35 sean-k-mooney so ya change the bandwith or pps qos amount on a bound port would be the only time neutron could modify the allocation i think
14:45:43 gibi yepp I think too
14:45:44 dansmith okay, but this was for nested A-C, meaning another allocation against another provider coming back from placement, not any sort of actual nesting of allocations as stored right?
14:46:20 sean-k-mooney yes i dont think there is really a concept of nested allocations
14:46:27 gibi dansmith: right, we dont have sub allocations we have multiple RPs in an allocaiton
14:46:31 sean-k-mooney jsut a singel allcoation with resouce form multple RPs today
14:47:04 dansmith gibi: so you wrote this initial patch to move from delete to put specifically for "benefit" of generations.. are you thinking that was wrong now?
14:48:03 gibi dansmith: I tried to figure out this mornign the reason why I had that patch
14:48:43 gibi the only thing I know is that bumping the placemenet api to use nested a_cs meant that we got generations as well
14:48:51 gibi so we had to do something with generations
14:49:29 gibi i.e. the nested a_c support was in a bigger placement microversion than the generation support
14:50:26 gibi 1.28 and 1.29
14:51:53 gibi if the request from the user is to delete the instance but that deletion fails due to a parallel operation then I think now that the delete operation should be the one that wins
14:52:47 dansmith I don't think this patch was required just because we did generations and nested a_c, because you're updating the version you pass there,
14:52:48 gibi in the other hand if the user requested restore of soft deleted instance and the timer on the soft delete -> hard delete is racing with that request then the restore should win
14:52:53 dansmith and each call uses whatever version it wants right?
14:53:00 sean-k-mooney gibi: microversion are per cal though
14:53:03 dansmith right
14:53:24 gibi right
14:54:30 gibi I think the warning at the end of https://docs.openstack.org/placement/latest/placement-api-microversion-history.html#consumer-generation-support shows that at that time we was pretty serious about using generations strictly
14:55:44 sean-k-mooney gibi: for the soft delete case we are expectign that this will help because? resotre will update allocation generation and the perodici will fail?
14:56:23 sean-k-mooney for soft delete i assume we continue to claim the resouce with the allocation until the actul hard delete happens
14:56:34 sean-k-mooney so would there be a need to update the allocation at all
14:56:40 sean-k-mooney for either soft delete or restore
14:56:49 gibi sean-k-mooney: if you are right then there could no be race between restore and soft -> hard delete
14:56:50 sean-k-mooney e.g. will the generation actuly cahnge
14:57:01 dansmith gibi: I haven't mapped it out, but is there some reason we can't specifically opt-into the nuke-and-destroy behavior when we're deleting an instance and still fail for other cases?
14:57:01 gibi btw the microversion history doc states
14:57:02 gibi Passing an empty allocations object along with a consumer_generation makes PUT /allocations/{consumer_uuid} a safe way to delete allocations for a consumer.
14:57:57 gibi dansmith: you mean the user should be able to opt into the force?
14:58:03 dansmith no
14:58:06 sean-k-mooney dansmith: am i think the reason is that technially deletes are not allowd to have request bodies and we did not want to use a query arg
14:58:08 dansmith you're force=True by default
14:58:22 sean-k-mooney oh you mean the force default
14:58:27 dansmith yes
14:58:31 gibi dansmith: so just inverting the default
14:58:37 gibi dansmith: that is OK to me
14:58:40 sean-k-mooney we can set that to false and jsut pass true for a user delete
14:58:50 sean-k-mooney ya im fine with inverting too
14:59:08 gibi I think at some point melwitt stated that she used force=True as default as that was the more frequent call
14:59:09 dansmith right, I think that's better signaling, and also better for whoever comes along later and uses that to delete an allocation
14:59:33 dansmith I think making the dangerous activity explicit is worth the verbosity
14:59:39 sean-k-mooney i tihnk its pretty close either way
15:00:01 sean-k-mooney in terms of calls
15:00:21 dansmith ack
15:40:03 dansmith gibi: sean-k-mooney: do we know why the consumer types thing suddenly means we need to do this btw? I imagine the answer is just "more chance for the generation to change" but.. why specifically?
15:42:45 gibi dansmith: exactly the consumer_types series merged the two transaction handling the allocation update into one, causing a longer transaction and hence a longer race window
15:43:40 dansmith really? I wouldn't expect that to be meaningfully long
15:44:15 dansmith as in, I would expect people running this in a larger cloud with lots of stuff going on would have hit this by now and not just in a gate run with few instances at any one point
15:44:29 dansmith I saw that analysis from melwitt in the bug and wasn't so sure
15:44:58 dansmith but I guess if it's really related to that change only then that has to be it
15:45:47 gibi there is a correlation with the consumer_type series landed and the uptick in the conflict failures in the gate
15:46:20 gibi I know correlation is not causation but still it feel relevant
15:46:34 gibi and the consumer_type series had this transaction logic change
15:46:36 dansmith hrm
15:46:50 gibi i have no better explanation at the moment
15:47:21 dansmith ack
15:53:21 sean-k-mooney dansmith: sorry was on internal call
15:54:17 sean-k-mooney ah am no i know there is a collation but not why
16:11:39 opendevreview Balazs Gibizer proposed openstack/nova master: Support boot with extended resource request https://review.opendev.org/c/openstack/nova/+/800086
16:12:39 opendevreview Ghanshyam proposed openstack/nova master: Convert features not supported error to HTTPBadRequest https://review.opendev.org/c/openstack/nova/+/806294
16:13:01 opendevreview Balazs Gibizer proposed openstack/nova master: Support move ops with extended resource request https://review.opendev.org/c/openstack/nova/+/800087
16:14:27 opendevreview Balazs Gibizer proposed openstack/nova master: [func test] refactor interface attach with qos https://review.opendev.org/c/openstack/nova/+/800088
16:15:49 opendevreview Balazs Gibizer proposed openstack/nova master: Support interface attach / detach with new resource request format https://review.opendev.org/c/openstack/nova/+/800089
16:17:10 opendevreview Balazs Gibizer proposed openstack/nova master: [func test] move unshelve test to the proper place https://review.opendev.org/c/openstack/nova/+/793621
16:18:32 opendevreview Balazs Gibizer proposed openstack/nova master: [nova-manage]support extended resource request https://review.opendev.org/c/openstack/nova/+/802060
16:19:26 opendevreview sean mooney proposed openstack/nova master: Remove module level caching https://review.opendev.org/c/openstack/nova/+/806394
16:19:27 opendevreview sean mooney proposed openstack/nova master: db: Handle parameters in DB strings https://review.opendev.org/c/openstack/nova/+/805663
16:19:54 opendevreview Balazs Gibizer proposed openstack/nova master: Reno for qos-minimum-guaranteed-packet-rate https://review.opendev.org/c/openstack/nova/+/805046
16:30:31 opendevreview Balazs Gibizer proposed openstack/nova master: Add force kwarg to delete_allocation_for_instance https://review.opendev.org/c/openstack/nova/+/688802
16:30:39 gibi dansmith, sean-k-mooney, melwitt, lyarwood: based on the above disucssion I inverted the force default value in ^^
16:31:09 gibi lyarwood, stephenfin: I fixed up the nits in the pps series. thanks for the valuable feedback
16:33:39 gibi and with that I end my week. o/
16:42:15 sean-k-mooney gibi: https://review.opendev.org/c/openstack/project-config/+/787523 has now merged so we shoudl have the review priority lable avaiable just an fyi
16:45:37 gmann finally :)
16:46:14 sean-k-mooney i think https://review.opendev.org/q/project:openstack/nova+Review-Priority:1 is how to use it
16:46:30 sean-k-mooney but we dont have any patches with it set
16:46:36 sean-k-mooney https://review.opendev.org/q/project:openstack/nova+Review-Priority:0 get hits however
16:56:07 gmann or event this, https://review.opendev.org/q/project:openstack/nova+Review-Priority:1+label:Verified%253D1++NOT+label:Workflow%253C%253D-1
16:56:08 gmann sean-k-mooney: ^^
16:56:29 gmann *even
16:57:15 sean-k-mooney perhaps we will need at lest one patch marked as a prioroty to test it
16:57:22 gmann it can be appended with NOT+owner:self
16:57:56 gmann sean-k-mooney: you can test with 0 priority to test query
16:58:52 melwitt gibi: ack
17:22:16 melwitt dansmith: I didn't think changing to a single transaction would make that much of a timing difference either but the consumer types patch series hit that bug far more often than anything else did. I didn't see anything else in those patches that could possibly be related, so I guessed about the db transaction change. my guess might be wrong
17:22:38 dansmith I'm not saying you're wrong, I'm just surprised
17:23:05 dansmith and surprised lots of real deployments aren't having trouble with much busier systems before the consumer types patch made the transaction longer

Earlier   Later