| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-01-12 | |||
| 19:40:10 | mriedem | sorry it's taking awhile, i keep getting distracted | |
| 19:40:17 | bauzas | mriedem: <3 | |
| 19:42:35 | sdague | mriedem: done | |
| 19:45:12 | openstackgerrit | Claudiu Belu proposed openstack/nova: hyper-v: Adds support for VMs with nested virtualization https://review.openstack.org/398509 | |
| 19:45:13 | mriedem | sdague: thanks | |
| 19:56:36 | jaypipes | bauzas: I told mriedem yesterday I didn't feel comfortable +2/W'ing that one because I co-authored it... | |
| 19:56:48 | jaypipes | bauzas: was hoping either mriedem or dansmith might have a chance to review it. | |
| 19:56:49 | bauzas | ack | |
| 19:56:58 | bauzas | no worries | |
| 20:05:49 | mriedem | i've got a fresh cup of coffee | |
| 20:05:50 | mriedem | so it's time | |
| 20:10:16 | mriedem | ha saved by the project rename bell | |
| 20:13:28 | openstackgerrit | Mario Villaplana proposed openstack/nova: Add ironic security group support to support matrix doc https://review.openstack.org/419634 | |
| 20:15:46 | melwitt | dansmith: thanks for reviewing the quota objects migration. I hadn't seen an online migration done in a transaction before. how is it done? by passing a session to the methods? | |
| 20:25:49 | mriedem | melwitt: i believe he's en route | |
| 20:25:51 | mriedem | to the great white north | |
| 20:27:10 | mriedem | melwitt: can't that be done by using the @db_api.pick_context_manager_writer decorator? | |
| 20:27:16 | melwitt | mriedem: oh, thanks. I didn't know he left already | |
| 20:30:29 | melwitt | mriedem: I think so, if all the calls stay under one decorator. I have a loop that calls a method that is already decorated, so I guess if I break that out into a separate undecorated function, I could decorate the loop one and a non-loop one separately | |
| 20:30:47 | cfriesen | I'm in the great white north, it's -18F right now. Looking forward to Atlanta. | |
| 20:32:15 | melwitt | yikes -18F | |
| 20:32:54 | sean-k-mooney | melwitt: you can have multple decorators on the same fuction/class | |
| 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 | |