Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-01-12
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 bauzas mriedem: not really but yeah
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:25 bauzas mriedem: for example, Newton is not accepting it for 1.0
21:31:29 bauzas AFAIK
21:31:31 ildikov smcginnis: me neither, but I also don't know what should put it there :)
21:31:47 rpodolyaka dansmith: lemme give it a try on a small snippet
21:31:51 smcginnis ildikov: :)
21:31:55 mriedem bauzas: oh because member_of wasn't in allowed_filters until ocata
21:32:02 bauzas zactly
21:32:13 bauzas just verifying it
21:32:18 bauzas for stable/newton
21:32:18 mriedem https://github.com/openstack/nova/blob/stable/newton/nova/objects/resource_provider.py#L413
21:32:25 jaypipes mriedem: responded to your review comments on the patch.
21:32:29 bauzas mriedem: yup that
21:33:02 bauzas jaypipes: lemme work on a next patchset tomorrow
21:33:08 dansmith rpodolyaka: my understanding is that an insert will still fail with DBDuplicate when you do it, and the thing that does the abort/commit of the transaction is the top-level facade decorator,
21:33:12 openstackgerrit Jay Pipes proposed openstack/nova: placement: RT now adds proper Ironic inventory https://review.openstack.org/404472
21:33:13 openstackgerrit Jay Pipes proposed openstack/nova: rt: explicitly pass compute node to _update() https://review.openstack.org/398472
21:33:16 openstackgerrit Jay Pipes proposed openstack/nova: rt: use a single ResourceTracker object instance https://review.openstack.org/398473
21:33:23 bauzas jaypipes: given it's the end of your afternoon :)
21:33:30 jaypipes bauzas: no problemo. I'm flying all day :)
21:33:44 dansmith rpodolyaka: so if you appear to exit your function cleanly, it will commit and you save anything you successfully inserted, unless you hit *another* dupe because you raced with another transaction
21:33:55 bauzas jaypipes: heh, seems terrible :)
21:34:28 bauzas mriedem: so, confirmed, stable/newton is not accepting member_of https://github.com/openstack/nova/blob/stable/newton/nova/api/openstack/placement/handlers/resource_provider.py#L187
21:34:47 rpodolyaka dansmith: the problem is that it's the very same transaction - once you have an integrity error you must roll it back at this point
21:34:52 bauzas cdent: ^ see that
21:34:59 mriedem yeah ok...
21:35:08 dansmith rpodolyaka: why must you roll back?
21:35:13 cdent bauzas: what are you trying to tell me?
21:35:16 mriedem so it might be fine to bend the rules and make the schema validation strict again
21:35:25 bauzas cdent: I think that if we silently accept member_of, it's not zero folks impacted
21:35:33 mriedem i.e. we didn't allow it, then we allowed it for a short window, then we didn't again
21:35:37 mriedem and the only user is internal to nova right now

Earlier   Later