| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-01-12 | |||
| 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. | |
| 21:52:23 | mriedem | smcginnis: i guess here http://docs.openstack.org/developer/cinder/devref/api_microversion_history.html | |
| 21:52:35 | smcginnis | mriedem: Beat me to it. | |
| 21:52:48 | ildikov | mriedem: it's 3.23, I test jgriffith's new attach API patch, which adds the 3.24 | |
| 21:53:01 | jgriffith | 3.24 | |
| 21:53:06 | jgriffith | as of an hour ago | |
| 21:53:19 | melwitt | rpodolyaka: there are use of the savepoint modifier in nova/db/sqlalchemy/api for several things that catch/expect DBDuplicateEntry | |
| 21:53:31 | mriedem | ildikov: so my guess is, | |
| 21:53:42 | rpodolyaka | melwitt: ah, then I missed those | |
| 21:53:57 | mriedem | the cinderclient shell code does some version discovery with the cinder api and gets the latest available microversion from the server and negotiates that with what the client understands, | |
| 21:54:03 | mriedem | that's how novaclient works | |
| 21:54:45 | dansmith | rpodolyaka: cool, thanks | |
| 21:54:49 | mriedem | and right now the max version in cinderclient is 3.15 i guess? https://github.com/openstack/python-cinderclient/blob/master/cinderclient/api_versions.py#L32 | |
| 21:54:50 | rpodolyaka | np! | |
| 21:54:53 | ildikov | mriedem: that sounds reasonable | |
| 21:54:56 | mriedem | not sure how that works if you're getting it to use 3.24 | |
| 21:55:28 | ildikov | mriedem: I'm setting OS_VOLUME_API_VERSION=3.24 | |