Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-08-27
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
17:23:19 melwitt I was surprised too and thought maybe it was a coincidence but once it merged everything was hitting the bug
17:23:26 dansmith and if that's really it, I sure hope we're not in for crazy pain if that made the transaction like waaaay longer or something
17:24:17 melwitt yeah, same. I haven't seen something like this before
17:37:31 sean-k-mooney i know that i have seen conflict in ci jobs form update avaiable resouces before the consome types change
17:38:01 sean-k-mooney so there was already some conficts happening
17:38:18 sean-k-mooney i dont think it caused test failures but i have seen it in the logs
17:43:15 dansmith oh definitely
17:57:57 opendevreview Ghanshyam proposed openstack/nova master: Convert features not supported error to HTTPBadRequest https://review.opendev.org/c/openstack/nova/+/806294
18:28:37 melwitt yeah, I know it was occurring prior to consumer types, just saying that on the consumer types series it hit the bug often and after it merged it's happening more often
18:34:49 melwitt and re: "yeah, same. I haven't seen something like this before" I haven't seen grouping writes into a single db transaction cause a significant timing difference before
18:53:26 dansmith melwitt: it failed zuul, but I don't see any conflcit messages in there, although it is one of these "hung until timeout" sorts it seems
18:53:44 dansmith maybe that multicell failure is known and some other pattern?
18:56:18 melwitt dansmith: you're talking about the failure on the force kwarg patch right? I would expect fails on that to not be the conflict bug and at a glance it looks like it is indeed something different
18:58:36 melwitt unrelated to that, this is a new error in the controller compute log on that run: Remote error: DBReferenceError (pymysql.err.IntegrityError) (1452, 'Cannot add or update a child row: a foreign key constraint fails (`nova_cell1`.`instance_info_caches`, CONSTRAINT `instance_info_caches_instance_uuid_fkey`
18:58:46 melwitt https://zuul.opendev.org/t/openstack/build/575784eb62da4365ab8a1942da0353bc/log/controller/logs/screen-n-cpu.txt#30972
18:59:04 dansmith I saw that too
18:59:35 dansmith maybe that dropped an update and that's why the test waited until timeout?
19:00:20 melwitt Aug 27 17:45:54.463587 ubuntu-focal-rax-ord-0026150414 nova-compute[111496]: ERROR nova.compute.manager [instance: 99c8640e-cb21-4fa9-bb90-d3361201ce7c] Traceback (most recent call last):
19:00:20 melwitt and on the instance from the failed tempest test: [instance: 99c8640e-cb21-4fa9-bb90-d3361201ce7c] Failed to allocate network(s): nova.exception.VirtualInterfaceCreateException: Virtual Interface creation failed

Earlier   Later