Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-10-12
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
22:33:39 dansmith so after your change we're doing the targeting in threads which would increase contention for that section that needs to be locked, not reduce it
22:34:12 dansmith what I thought was happening when I read this change the first time was that we were targeting the context, passing it (assuming by value) and then re-targeting for the next iteration
22:34:18 dansmith since you say set_target_cell in the commit message
22:34:20 dansmith but we weren't
22:34:40 dansmith to link this back,
22:35:34 dansmith in pike's instance list, we're single-threadedly doing each list, and then moving on to the next.. so there should be no overlapping of the execution of set_target_cell() which would mean if that was the problem, it wouldn't show up in list on pike
22:35:48 melwitt well, if one of the threads were populating the cache, without the change, there would be no synchronization of them doing that. right?
22:36:35 dansmith I'm not sure what you mean.. if one of the threads in the scatter?
22:36:52 melwitt yeah if the threads in the scatter were racing to populate the cell cache
22:36:59 dansmith each one is populating a different entry, right? so they're all synchronizing on a different cell_mapping.uuid, so they're not actually locking each other out
22:37:10 melwitt that must have been what I was seeing?
22:37:16 dansmith I don't think so
22:37:20 dansmith because ^
22:38:33 melwitt yeah, that's true. locally, what I was seeing was that an instance.save() was colliding with an instance get
22:39:06 dansmith if that's really happening, we have a huge problem I think, but I don't understand how that could be
22:39:13 melwitt during the instance read, instance write was trying to happen in the same DB transaction
22:39:33 melwitt and I saw the read was happening in the quota check (scatter gather)
22:40:07 melwitt so while it was checking quota, the instance.save() of the resize (I think it was) failed mid-transaction bc it was trying to upgrade the read from the quota check to a write for the instance save
22:40:16 dansmith if this is a problem, it should be a problem with a single cell right?
22:40:28 melwitt yeah, the test setup was cell0 and cell1
22:41:07 dansmith what I mean is before we were doing any cell switching
22:41:36 melwitt oh. yeah I mean I definitely see your point there and I currently don't understand how that isn't an issue
22:41:39 dansmith I don't think this has anything to do with the number of cells you have since it's all per-cell and as we established above, the cache isn't even synchronized across all, it's just on the one cell you're trying to populate
22:41:56 melwitt like, is there something different about us rolling our own green threads via spawn?
22:42:04 dansmith no

Earlier   Later