Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-01-12
20:33:13 sean-k-mooney melwitt: unless you ment it is allready decoated with @db_api.pick_context_manager_writer
20:36:53 melwitt sean-k-mooney: yeah, I mean the "create in db" method is already decorated with @db_api.pick_context_manager_writer and there's a loop doing several "create in db". so to do all of them in one transaction, I thought I have to move the decorator, or can they be nested? I hadn't seen nesting before
20:38:17 sean-k-mooney i dont know. if the decorate is opening the transation then it will do so when you enter the fucntion so all the actions in the loop should be in one transation
20:38:33 sean-k-mooney that said i have not read the code you are refering too
20:38:48 sean-k-mooney and gerrit is currently down so that is just a guess
20:40:47 melwitt sean-k-mooney: yeah, that's my understanding. I was thinking I can't just slap another decorator on the outside of the loop when there's one already on the method I'm calling (so that they nest). I'm not sure though
20:41:33 sean-k-mooney ah ok can you apply decoratos to loops directly? i have never tried that before
20:41:37 melwitt so I was thinking I would have to create a small wrapper function with the decorator for when I want to call it non-loop and move the decorator there
20:42:10 melwitt no, sorry, I meant apply the decorator to the outer function. the one that is doing a loop
20:42:58 mriedem melwitt: i'd ask rpodolyaka
20:43:03 sean-k-mooney ah ok. ya without seeing the code or trying it im not sure either
20:43:15 mriedem i see some stuff in the transaction factory code that looks like it can pull an existing session from the context
20:44:42 melwitt this is what it looks like, snippet http://paste.openstack.org/show/594765/
20:45:34 openstackgerrit Sarafraj Singh proposed openstack/nova: Add service_token for nova-neutron interaction https://review.openstack.org/410394
20:45:58 melwitt I haven't seen any nesting in db/sqlalchemy/api before, so I thought it wasn't a thing
20:46:08 rpodolyaka melwitt: they can be nested - the inner calls will participate in the ongoing transaction
20:46:30 melwitt rpodolyaka: ah, thank you. that makes it easy then
20:46:57 openstackgerrit Sarafraj Singh proposed openstack/nova: DNM: Test service token https://review.openstack.org/413232
20:47:37 rpodolyaka melwitt: they only thing you can't do is to go from async to writer, writer -> writer is fine
20:47:47 rpodolyaka np!
20:47:50 rpodolyaka *the
20:47:59 mriedem https://github.com/openstack/oslo.db/blob/4.15.0/oslo_db/sqlalchemy/enginefacade.py#L556
20:48:04 melwitt gotcha, learned something new
20:49:40 sean-k-mooney ok cool so each of the _create_limit_in_db will inherit the transaction from migrate_quota_limits_to_api_db and unless they all complete succefully they will all be rolled back
20:51:05 melwitt yeah, that was the suggestion in the review
20:51:25 rpodolyaka ah, there is one catch, though
20:51:27 sean-k-mooney well the outer trasaction will not be commited unless all the inner creates that inherit it complete sucessfully rather then rolling back if it fails
20:51:53 sean-k-mooney rpodolyaka: :) and that is?
20:51:57 rpodolyaka when is raised db_exc.DBDuplicateEntry the transaction must be rolled back
20:52:19 mriedem gerrit is back btw
20:52:49 rpodolyaka so I guess what you want in http://paste.openstack.org/show/594765/ is SAVEPOINT's
20:52:59 rpodolyaka or independent transactions
20:53:02 sean-k-mooney ok so in migrate_quota_limits_to_api_db you need to catch db_exc.DBDuplicateEntry and roleback?
20:53:22 melwitt here's the actual review now that gerrit is back https://review.openstack.org/#/c/410946/6/nova/objects/quotas.py@463
20:54:16 rpodolyaka sean-k-mooney: you can, but you'd effectively roll back limits saved for previous iterations of the loop
20:56:15 rpodolyaka you could use the savepoint() modifier -https://github.com/openstack/oslo.db/blob/master/oslo_db/sqlalchemy/enginefacade.py#L860
20:56:28 rpodolyaka to create a SAVEPOINT for each iteration of the loop
20:56:48 rpodolyaka so that ROLLBACK happens only for the one that caused DuplicateError
20:57:06 melwitt oh, cool
20:59:12 sean-k-mooney is that the intended behavior or if you have a duplicatate shoudl they all be roled back?
20:59:52 sean-k-mooney e.g. is a partal migration with removal of duplicate the desired outcome
21:01:00 melwitt duplicate should just be passed over/ignored
21:01:22 sean-k-mooney ok cool just checking :)
21:01:26 melwitt ignored for writing to the api db, and should be deleted from the main db if present
21:14:11 dansmith melwitt: what mriedem said yeah
21:15:55 dansmith melwitt: just a method that is "take this list of stuff and jam it into place" that is wrapped in one decorator will ensure that they all get applied or none do
21:15:56 ildikov mriedem: hi
21:16:55 ildikov mriedem: I'm trying to test the new Cinder API with Nova and experiencing odd things, which might be because of API microversioning in Cinder in the sense of something might be missing on the Nova side
21:18:46 ildikov mriedem: I'm less familiar with how microversioning works between OpenStack services. Is there anything I should theoretically set up in Nova to have the calls towards the latest Cinder API microversion work?
21:18:59 mriedem bauzas: cdent: jaypipes: edleafe: posted some comments in the resource providers handler code for https://review.openstack.org/#/c/392569/
21:19:11 mriedem the -1 is really about the fact that i think we're making a backward incompatible change
21:19:43 mriedem ildikov: i'd expect cinderclient to be passing any microversion header to cinder api when using the v3 client
21:20:24 melwitt rpodolyaka: do I need savepoint considering DBDuplicateEntry is caught and ignored in the loop? or does it still cause some sort of rollback to happen?
21:20:25 ildikov mriedem: the same call succeeds if I call it from the Cinderclient and fails if I call it from Nova
21:21:11 mriedem ildikov: do you have the cinderclient call output with --debug?
21:21:14 mriedem to see the headers?
21:21:55 rpodolyaka melwitt: it still does - the fact that it was raised in the first place means that a DB query failed with an integrity error - you need to rollback a transaction at this point
21:22:19 melwitt rpodolyaka: oh, I see. thanks
21:22:27 rpodolyaka the only way to avoid that is to create a SAVEPOINT before the potential fail
21:23:22 ildikov mriedem: I have now :)
21:24:06 bauzas mriedem: you mean https://review.openstack.org/#/c/392569/26/nova/api/openstack/placement/handlers/resource_provider.py@a170 ?
21:24:24 mriedem bauzas: yup
21:25:01 bauzas mriedem: accepted
21:25:12 ildikov mriedem: that contains the API microversion
21:25:19 dansmith rpodolyaka: but we want it to roll back all the way in this case
21:25:24 mriedem ildikov: can you put it into a paste?
21:25:25 dansmith rpodolyaka: so no savepoint, as you said
21:25:53 melwitt dansmith: I thought we just wanted to skip duplicates? and commit all the non-duplicates in one transaction
21:26:17 bauzas mriedem: so, about the member_of query
21:26:18 dansmith melwitt: sure, but if something else happens you want to unwind all of them
21:26:28 melwitt oh
21:26:47 bauzas mriedem: we first added the member_of field in allowed_filters without microversing it
21:27:02 dansmith melwitt: otherwise you've got some moved and not others, which will make the code start to look at just the subset, instead of going back to the main db while you figure out what to do
21:27:14 ildikov mriedem: here it is: http://paste.openstack.org/show/594772/
21:27:21 bauzas mriedem: so, before that, we were not accepting member_of and then we accepted it
21:27:40 melwitt dansmith: yeah, I guess I'm confused about what happens then if we don't use a savepoint and get a duplicate, that it would rollback the entire transaction?
21:27:46 openstackgerrit Sarafraj Singh proposed openstack/nova: Adopts keystoneauth with glance client. https://review.openstack.org/412634
21:27:53 bauzas mriedem: for both 1.0 and 1.1
21:28:04 openstackgerrit Sarafraj Singh proposed openstack/nova: Adopts keystoneauth with glance client. https://review.openstack.org/412634
21:28:14 dansmith melwitt: you're ignoring duplicates, right?
21:28:32 melwitt dansmith: yeah, but rpodolyaka said it would still roll back everything?
21:28:34 bauzas mriedem: and then, cdent added the v1.3 microversion for still accepting member_of even if that was not this version
21:28:45 dansmith melwitt: no, he said it would roll back everything if you allow an exception to be raised,
21:28:47 bauzas which is IMO terrible but okay
21:28:56 dansmith which is what we want for everything other than dupes, which you catch and swallow
21:29:03 cdent we discussed it at length and decide that it impacted approximately zero real people
21:29:39 rpodolyaka dansmith: the problem is that you can't have one transcation and swallow duplicate errors
21:29:40 melwitt dansmith: yeah, I actually asked if I still need savepoint considering I'm catching and ignoring DBDuplicateEntry and the answer was yes. maybe there's a misunderstanding
21:29:42 ildikov mriedem: this is the n-api log for the same attachment-create call: http://paste.openstack.org/show/594773/
21:29:46 rpodolyaka unless you use savepoints
21:29:58 rpodolyaka but the rollback in the outer transcation will still rollback everything
21:30:23 bauzas cdent: mriedem: anyway, I'll -W it and work tomorow morning my time for a next PS
21:30:24 dansmith rpodolyaka: I don't know why that is
21:30:36 mriedem bauzas: yeah so up until 1.4 we always accepted member_of,
21:30:47 mriedem but if < 1.3 we don't filter on member_of
21:31:08 smcginnis ildikov: I don't see "Openstack-Api-Version: volume 3.24" in there.
21:31:10 mriedem but it's not an error to put it in the query string, because of the aforementioned missing of the versioning
21:31:10 bauzas mriedem: not really but yeah
21:31:25 bauzas mriedem: for example, Newton is not accepting it for 1.0
21:31:29 bauzas AFAIK

Earlier   Later