| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-08-27 | |||
| 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 | gibi | btw the microversion history doc states | |
| 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: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 | 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 | |
| 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:31 | melwitt | https://zuul.opendev.org/t/openstack/build/575784eb62da4365ab8a1942da0353bc/log/controller/logs/screen-n-cpu.txt#33831 | |
| 19:02:17 | melwitt | server failed to spawn. the info cache error was on a different instance | |
| 19:04:48 | melwitt | [instance: 217a81e4-b9cf-4e9e-97a9-edcb0fab8349] Can not refresh info_cache because instance was not found | |
| 19:05:06 | melwitt | trying to refresh info cache on an instance that's gone.. that's odd | |
| 19:25:24 | melwitt | oh, it looks like a network event went to the compute/cell that the instance was not in, so when it tried to refresh the info cache, it was not found | |
| 21:20:53 | opendevreview | Merged openstack/nova stable/wallaby: Avoid modifying the Mock class in test https://review.opendev.org/c/openstack/nova/+/805759 | |
| 21:21:04 | opendevreview | Merged openstack/nova master: [func test] move port creation to the NeutronFixture https://review.opendev.org/c/openstack/nova/+/787206 | |