Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-01-12
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
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 mriedem https://github.com/openstack/nova/blob/stable/newton/nova/objects/resource_provider.py#L413
21:32:18 bauzas for stable/newton
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
21:35:43 mriedem excluding whatever mlavalle is doing
21:35:48 dansmith rpodolyaka: we did another migration like this that I think we've since purged, so if this doesn't work then that didn't either
21:35:49 rpodolyaka dansmith: mysql will tell you the transaction is invalid and must be rolled back. just like with any other error in a transaction
21:36:04 rpodolyaka you sure that were not independent transactions?
21:36:07 bauzas mriedem: either way, you have lots of other comments, will respond to those
21:36:13 dansmith rpodolyaka: yeah
21:36:16 rpodolyaka hmm
21:36:36 dansmith rpodolyaka: well, I don't know anything about mysql making you do that, but surely willing to believe you know more than me
21:37:07 openstackgerrit Sivasathurappan Radhakrishnan proposed openstack/nova: Refactor _post_live_migratiion https://review.openstack.org/413021
21:37:17 bauzas anyway, calling it a end of day
21:37:19 rpodolyaka dansmith: heh, I'm a bit sleepy, so maybe I'm missing something obvious :P
21:37:24 dansmith rpodolyaka: not sure we were filtering for dupes before, which I guess is the wrinkle
21:37:49 dansmith melwitt: either way, if rpodolyaka is right, a save point before each insert and a revert in a generic handler is not hard right?
21:38:08 melwitt yeah, I couldn't find a current migration that does a loop in one transaction, to use as an example
21:38:18 dansmith melwitt: also, do you need the dupe handling if you apply them in a transaction?
21:38:55 mriedem jaypipes: what do you mean this is unreadable?! (?:[A-Z0-9_]+\:[0-9]+),?
21:39:02 melwitt dansmith: hm, I dunno. maybe not
21:40:07 openstackgerrit Sarafraj Singh proposed openstack/nova: Add service_token for nova-glance interaction https://review.openstack.org/419662
21:42:48 mlavalle code^^^
21:42:48 mlavalle mriedem, bauzas: I am not using any filters with resource providers in my core. Feel free to go ahead
21:47:06 mriedem bauzas: jaypipes: ok i replied to the replies. :) i still think that we can enforce the format of the uuid parameter to be a uuid, as it's not being used with in:
21:47:13 mriedem "in:" is only used with member_of
21:48:07 mriedem https://review.openstack.org/#/c/392569/26/nova/api/openstack/placement/handlers/resource_provider.py@289
21:48:33 openstackgerrit Sarafraj Singh proposed openstack/nova: DNM: Test service token in Glance https://review.openstack.org/419666
21:49:10 mriedem ildikov: ok so "-H "OpenStack-API-Version: volume 3.24""
21:49:22 dansmith rpodolyaka: okay, I found some references to what you're describing
21:49:26 rpodolyaka dansmith: shit, it works on mysql - but fails on postgresql without savepoints... http://paste.openstack.org/show/594775/
21:49:37 dansmith hah
21:50:10 dansmith rpodolyaka: seems like enginefacade could manage this for us.. automatically doing savepoints or something, if we nest calls
21:50:14 mriedem ildikov: yeah so the "OpenStack-API-Version" header isn't in the paste from the nova-api call
21:50:29 dansmith rpodolyaka: but regardless, gotta handle it for now I guess
21:50:37 mriedem ildikov: but i don't think nova is supposed to be picking the microversion is it, cinderclient is, at least by default it would be 3.0
21:51:04 mriedem ildikov: is there a difference in how cinderclient's shell creates the client object and makes requests for v3 vs how nova is doing it?
21:51:28 mriedem smcginnis: cinder is up to microversion 3.24 already?
21:51:39 smcginnis mriedem: Sounds right.
21:51:40 rpodolyaka dansmith: yeah, it's just that we do not create a SAVEPOINT for each nested call - so we've got a modifier - https://github.com/openstack/oslo.db/blob/master/oslo_db/sqlalchemy/enginefacade.py#L860
21:51:53 rpodolyaka which should be used in such cases
21:51:55 mriedem smcginnis: where do you guys document your REST API version history?
21:52:02 mriedem not finding it here http://docs.openstack.org/developer/cinder/
21:52:05 rpodolyaka although, we don't probably use it anywhere in nova
21:52:06 smcginnis Document?
21:52:09 ildikov mriedem: that's an amazing question, I have no clue :)
21:52:14 smcginnis mriedem: Kidding. One moment.

Earlier   Later