| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-10-12 | |||
| 18:57:53 | melwitt | couple of easy reviews if anyone can look https://review.openstack.org/#/c/509968 and https://review.openstack.org/#/c/510823 | |
| 18:59:02 | melwitt | gibi might be interested in the first one ^ | |
| 19:06:58 | mriedem | done and done | |
| 19:08:58 | melwitt | thx mriedem | |
| 19:52:14 | openstackgerrit | Merged openstack/nova master: Add snapshot id to the snapshot notifications https://review.openstack.org/453077 | |
| 20:31:42 | openstackgerrit | Merged openstack/nova master: Elevate existing RequestContext to get bandwidth usage https://review.openstack.org/509968 | |
| 20:32:22 | openstackgerrit | Merged openstack/nova master: Make expected notifications output easier to read in tests https://review.openstack.org/510823 | |
| 20:59:12 | mriedem | nova tastic meeting in 1 minute | |
| 20:59:32 | melwitt | be there or be a rhombus | |
| 21:05:04 | openstackgerrit | Ed Leafe proposed openstack/nova master: Add Selection objects https://review.openstack.org/499239 | |
| 21:05:04 | openstackgerrit | Ed Leafe proposed openstack/nova master: Add alternate hosts https://review.openstack.org/486215 | |
| 21:05:05 | openstackgerrit | Ed Leafe proposed openstack/nova master: Change RPC for select_destinations() https://review.openstack.org/510159 | |
| 21:05:05 | openstackgerrit | Ed Leafe proposed openstack/nova master: Return Selection objects from the scheduler driver https://review.openstack.org/495854 | |
| 21:05:06 | openstackgerrit | Ed Leafe proposed openstack/nova master: WIP - Make conductor pass and use host_lists https://review.openstack.org/511358 | |
| 21:05:06 | openstackgerrit | Ed Leafe proposed openstack/nova master: Move the claim_resources method to scheduler utils https://review.openstack.org/511357 | |
| 21:27:26 | openstackgerrit | Merged openstack/nova master: Nix bug msg from ConfGroupForServiceTypeNotFound https://review.openstack.org/511261 | |
| 21:29:53 | efried | woot. 5 rechecks. | |
| 21:35:11 | takashin | mriedem: Would you take a look at https://review.openstack.org/#/c/334732/ ? I fixed response codes in the spec. | |
| 21:36:47 | mriedem | takashin: i'll add it to my list | |
| 21:37:26 | takashin | mriedem: Thank you. | |
| 21:38:04 | mriedem | takashin: btw, you might be interested in this https://review.openstack.org/#/c/511351/ | |
| 21:38:14 | mriedem | really anything related to migration status i'm going to send your way now | |
| 21:39:32 | takashin | mriedem: okay. I will check it. | |
| 21:47:13 | openstackgerrit | Matt Riedemann proposed openstack/nova-specs master: Spec for flavor description https://review.openstack.org/501017 | |
| 21:48:55 | sapd_ | Hi everybody! | |
| 21:50:56 | sapd_ | I'm upgrade from openstack ocata to pike version. But I had an error when I use admin to list all server in all tenants | |
| 21:51:10 | sapd_ | The error is: TypeError: Can't upgrade a READER transaction to a WRITER mid-transaction | |
| 21:51:17 | mriedem | melwitt: ^ | |
| 21:53:03 | sapd_ | Please help me! @@ | |
| 21:54:41 | mriedem | sapd_: i think this https://review.openstack.org/#/c/511538/ | |
| 21:54:57 | mriedem | apply that patch | |
| 21:55:23 | melwitt | ah, yep. | |
| 21:55:31 | sapd_ | I saw this path. I use context.py from master branch, But It can't help me :(( | |
| 21:56:46 | melwitt | sapd_: did you restart services after applying the patch? did you apply it to all of your API hosts? | |
| 21:56:59 | mriedem | did you clean out stale pycs? | |
| 21:57:41 | sapd_ | yes. I did. | |
| 21:58:40 | sapd_ | But It doesn't change anything. | |
| 22:00:21 | mriedem | melwitt: would that also be needed in conductor since we check quota there too? | |
| 22:00:27 | melwitt | sapd_: do you have a trace you can pastebin? one thing that's weird is how there would be a write during a instance list | |
| 22:01:01 | melwitt | mriedem: yeah, you're right | |
| 22:01:22 | melwitt | sapd_: did you patch your nova-conductors too? | |
| 22:03:29 | sapd_ | No I just path on nova-api! does it matter? | |
| 22:04:16 | melwitt | sapd_: yes. nova-conductor runs the same code during instance create, so that needs to be patched too | |
| 22:06:47 | sapd_ | Hi melwitt, I got another error after apply that path: This service is older (v16) than the minimum (v22) version of the rest of the deployment. Unable to continue. | |
| 22:07:18 | dansmith | melwitt: that backport to pike won't affect instance listing will it? | |
| 22:07:45 | sapd_ | This service is older (v16) than the minimum (v22) version of the rest of the deployment. Unable to continue. (HTTP 400) (Request-ID: req-308498b1-848c-4c2d-833a-9341e6f6586e) | |
| 22:07:45 | sapd_ | root@capt-admin-1:/opt# openstack server list --project CS-Labs | |
| 22:08:28 | dansmith | sapd_: you have some unupdated api nodes I think | |
| 22:09:05 | melwitt | dansmith: thinking ... about whether a concurrent boot request could cause that in an instance list. it seems like not because it should be the request that wanted to write that fails | |
| 22:09:29 | sapd_ | I have three nova-api nodes, But I stop two nodes, Only one node up now. | |
| 22:09:31 | melwitt | I'm currently not understanding how it's the instance list that could fail with an attempt to upgrade to write | |
| 22:09:33 | dansmith | melwitt: instance list didn't use scatter/gather in pike | |
| 22:10:15 | mriedem | sapd_: you're getting that when trying to start up those other 2 services? or the one that is upgraded? | |
| 22:11:31 | mriedem | although we filter out osapi_compute services from that api... | |
| 22:11:54 | dansmith | mriedem: it's also the kind of thing that could be misinterpreted | |
| 22:13:58 | melwitt | dansmith: yeah, I was thinking if an instance list is trying to use the cell cache and some other boot request comes in and mucks with the DB transaction context it could cause instance list to fail | |
| 22:14:20 | melwitt | even if instance list isn't using scatter gather, the quota check for the boot request does | |
| 22:14:46 | dansmith | I'm not sure what is shared between those two operations that would cause such a thing | |
| 22:14:57 | dansmith | it'd be a pretty big problem if they were related like that no? | |
| 22:15:01 | sapd_ | The one is upgraded, others node is not upgrade yet | |
| 22:15:28 | melwitt | the cell cache contains the DB transaction context manager and each transaction is supposed to be one thread at a time | |
| 22:15:47 | openstackgerrit | OpenStack Proposal Bot proposed openstack/os-vif stable/newton: Updated from global requirements https://review.openstack.org/373293 | |
| 22:15:50 | dansmith | melwitt: the context manager but not the context | |
| 22:16:11 | dansmith | melwitt: just like when we had only one context manager, multiple requests from multiple users don't trample on each other | |
| 22:16:30 | dansmith | melwitt: your fix is just making sure that we don't hand a request context to one thread an then modify that context whilst running | |
| 22:17:52 | melwitt | dansmith: sorry, it's confusing. but what I found is that each decorated (with context manager) DB API function creates a transaction context and before the locking was in the right place, that transaction context would be hijacked by another thread, mid-transaction | |
| 22:18:38 | dansmith | melwitt: that's because we switched the context.db_connection it was using right? | |
| 22:18:58 | dansmith | so it started something with one and then entered another oslo context managed method with a different one | |
| 22:19:21 | dansmith | melwitt: what you're describing would mean that two users hitting the api simultaneously aren't properly isolated from each other | |
| 22:21:20 | melwitt | dansmith: yeah ... I see what you're saying. I was looking through the oslo.db code and put print statements in there, and saw the transaction context object that's created-on-the-fly and cached per transaction was getting used by another DB access before it was finished | |
| 22:21:46 | dansmith | melwitt: right, because we were handing them contexts that were being modified later | |
| 22:21:53 | dansmith | melwitt: so we hand the first thread a context, | |
| 22:22:11 | dansmith | then we change that context when we hand the second/last one its context | |
| 22:22:30 | dansmith | so the first one and the second one are using the same context.db_connection because they share the same context object | |
| 22:22:32 | dansmith | after your change, | |
| 22:22:42 | dansmith | the target_cell yields a new copied context, with the change made, | |
| 22:22:51 | dansmith | and they start their db session with that context | |
| 22:23:03 | dansmith | *they each start | |
| 22:24:14 | dansmith | let me say that again but hopefully more clear: | |
| 22:24:29 | dansmith | let's say we have two cells | |
| 22:24:33 | melwitt | haha, sorry. I have found the whole thing very confusing, even while I was working on it | |
| 22:25:19 | dansmith | hang on a sec, I'm not sure the above is quite right | |
| 22:25:28 | dansmith | it doesn't change the point, but just a sec | |
| 22:25:34 | melwitt | k | |
| 22:26:39 | dansmith | actually, I'm not sure about your fix really | |
| 22:27:32 | dansmith | yeah, so I think the premise in your commit message is wrong | |
| 22:28:06 | dansmith | you say we synchronize access to the cell cache to avoid two things using the context manager at the same time, | |
| 22:28:08 | dansmith | but that's not true, | |
| 22:28:33 | dansmith | we synchronize it so two threads racing to populate the cache won't both create and set the cache entry | |
| 22:28:53 | dansmith | if the cache is already populated, we just grab the results, drop out of the lock, and set it on our context | |
| 22:29:03 | dansmith | then we *use* it later at will, no synchronization | |
| 22:29:33 | dansmith | # Synchronize access to the cache by multiple API workers. | |
| 22:29:47 | dansmith | just the cache to avoid a storm of threads trying to be the first to set the cache entry | |
| 22:30:23 | dansmith | your change should be yielding out a targeted copy of the context which is passed to the thread and never used again, | |
| 22:30:32 | dansmith | and shouldn't be altered by the other iterations of the loop | |
| 22:30:58 | dansmith | so I'm not sure why moving target_cell inside the thread would change anything | |
| 22:31:33 | melwitt | well, wasn't what you said what was happening before my change? yielding a targeted context and passing to the thread? | |
| 22:32:06 | dansmith | I didn't parse that | |
| 22:32:29 | melwitt | sorry. isn't that what was happening before my change? passing a targeted copy of the context to the thread? | |
| 22:32:58 | dansmith | sorry, I meant "before your change" above | |