| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-08-27 | |||
| 14:22:39 | dansmith | ah | |
| 14:22:45 | artom | I've only seen yellow so far | |
| 14:23:10 | dansmith | yeah, probably the dark switch, I had one machine in dark the other day | |
| 14:23:19 | artom | Reading the scrollback, what *was* the logic behind PUTing empty allocations instead of outright DELETE? | |
| 14:23:27 | artom | We had consumer generations, so... just because we could? | |
| 14:23:34 | artom | Like, we're deleting, why do races matter? | |
| 14:23:41 | dansmith | artom: well, I'm kinda wondering if it's related to races during move and such | |
| 14:23:53 | dansmith | artom: we delete allocations for all kinds of reasons other than deleting an instance | |
| 14:24:31 | artom | So the idea is - before we delete, make sure we have the latest version so that we can then POST it to a new RP? | |
| 14:24:50 | dansmith | which is why it seems to me like maybe we should only force=true on instance delete, and default to not everywhere else, instead of what is proposed which is always force | |
| 14:25:32 | sean-k-mooney | so just invert the default for force | |
| 14:25:39 | dansmith | we're not really checking the allocations though, so, probably not worth it | |
| 14:26:04 | dansmith | anyway, it just feels like we're removing a fence because it's currently getting in the way :) | |
| 14:26:23 | artom | Yeah - hence the question - why did we put the fence there in the first place? | |
| 14:27:02 | dansmith | well, generations went in to avoid us clobbering allocations all the time | |
| 14:27:10 | dansmith | during things like moves | |
| 14:28:36 | sean-k-mooney | artom: this is the orginial commit that added the put behaivor https://review.opendev.org/c/openstack/nova/+/591597 | |
| 14:28:40 | artom | So, I'm sure there's a legitimate reason for it, but I don't grok it. Even during a move it, why would there be 2 bits of code trying to update allocations concurrently? | |
| 14:28:47 | artom | *move operation | |
| 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 | |