| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-08-27 | |||
| 14:28:54 | sean-k-mooney | it has some motivation for it in the commit but not a fully explanation | |
| 14:30:01 | sean-k-mooney | the main motivation i got form that was to intneionally be able to put the instance into error on delete if it was in an inconsitent state | |
| 14:30:16 | sean-k-mooney | to presumable give the operator or user a change to investigate | |
| 14:30:39 | sean-k-mooney | *chance | |
| 14:30:48 | dansmith | originally before we were using migration allocations I think both sides fought over who was going to claim the resources and delete the other | |
| 14:30:49 | artom | sean-k-mooney, that's for instance deletion, dansmith is talking about moves needing generations | |
| 14:31:12 | dansmith | and of course same-host migrations made that hard | |
| 14:31:19 | artom | ... so we did a thing in placement because Nova was dumb? | |
| 14:31:36 | artom | Ah, resize to same host... | |
| 14:31:38 | sean-k-mooney | isnt that why placment was created :) | |
| 14:31:42 | dansmith | artom: no dude, placement was/is accessed by multiple services at once | |
| 14:31:45 | artom | Hah | |
| 14:32:39 | sean-k-mooney | but here this is a distibuted lock problem | |
| 14:32:50 | sean-k-mooney | in that we dont have one | |
| 14:32:58 | artom | dansmith, right, I know that. So was this something like Nova updating the VCPUs or whatever, while Neutron concurrently tries to update something about the port? | |
| 14:33:00 | dansmith | well, generation is the lock | |
| 14:33:07 | sean-k-mooney | so generations were added ^ yep | |
| 14:33:12 | dansmith | I'm trying to remember how this works, but is the generation on the consumer actually? | |
| 14:33:20 | dansmith | such that one instance moving between two RPs shares a generation? | |
| 14:33:40 | sean-k-mooney | i think the generation is on the allocation | |
| 14:34:02 | sean-k-mooney | i dont know if they share the same generation when we have both in a migration case | |
| 14:35:01 | dansmith | we don't when migration is holding the resources on the other, | |
| 14:35:26 | dansmith | but the generation is on the allocation for the consumer across both compute nodes (if you do it that way) | |
| 14:35:46 | dansmith | so even with both compute nodes being "separate" they both had to update the allocation for the instance, | |
| 14:35:57 | dansmith | to claim resources on incoming and drop the resources from the outgoing | |
| 14:36:26 | dansmith | but this delete is actually _deleting_ the allocation | |
| 14:36:48 | dansmith | so this is from after that concept of a single allocation against both providers | |
| 14:37:23 | sean-k-mooney | ah so we dont have 2 indepented allcoations | |
| 14:37:36 | dansmith | right | |
| 14:37:37 | sean-k-mooney | i tought we used the migration uuid to create an indepent allcoation | |
| 14:37:45 | dansmith | oh we do no, | |
| 14:37:46 | dansmith | *now* | |
| 14:37:58 | sean-k-mooney | ah ok | |
| 14:38:05 | dansmith | I'm saying before we did that, we were creating allocations for the instance against both computes | |
| 14:38:06 | sean-k-mooney | you were discibing your initall implemenation | |
| 14:38:10 | sean-k-mooney | before you refactored it | |
| 14:38:14 | dansmith | which required both to modify it during/after the migration | |
| 14:38:24 | dansmith | not mine, but the, yes | |
| 14:38:30 | dansmith | mine was the "use migration allocation" change | |
| 14:38:54 | sean-k-mooney | right ok that aligns with what i recall | |
| 14:39:20 | sean-k-mooney | so you think this was really only need before the move to seperate allocations? | |
| 14:39:33 | sean-k-mooney | or do you think its still useful now. the put instead of delete | |
| 14:40:35 | dansmith | I think the PUT came after we were already using two separate allocations, but not positive | |
| 14:40:37 | sean-k-mooney | for migrations that is | |
| 14:41:03 | sean-k-mooney | ok im not sure of the time line but that sound right | |
| 14:41:04 | dansmith | gibi wrote that initial patch you linked above to move *away* from delete specifically to get generation stuff, | |
| 14:41:10 | dansmith | and it was part of "nested allocation candidates" | |
| 14:41:20 | sean-k-mooney | the migration allcotion change was a long time ago relitivly speaking | |
| 14:41:34 | dansmith | right | |
| 14:41:50 | sean-k-mooney | oh it was for nested resouce providers | |
| 14:42:08 | sean-k-mooney | so maybe for port resouce requests? | |
| 14:42:34 | dansmith | well, that's kinda what I was eluding to above, | |
| 14:42:43 | dansmith | with multiple things accessing here | |
| 14:42:57 | sean-k-mooney | ack | |
| 14:43:05 | dansmith | like does PUT {} avoid nuking a sub-allocation that neutron hasn't yet cleaned up? | |
| 14:43:30 | sean-k-mooney | i dont think we technially support netron modifying the allocation directly | |
| 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 | |