Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-05-17
19:00:31 sdague files
19:00:34 sdague for compat in gate runs
19:01:35 melwitt cool, thanks
19:06:17 openstackgerrit Artom Lifshitz proposed openstack/nova master: Plumbing for tagged nic attachment https://review.openstack.org/415622
19:06:19 openstackgerrit Artom Lifshitz proposed openstack/nova master: Delete nic metadata when detaching interface https://review.openstack.org/435387
19:06:21 openstackgerrit Artom Lifshitz proposed openstack/nova master: Libvirt support for tagged volume attachment https://review.openstack.org/418100
19:06:25 openstackgerrit Artom Lifshitz proposed openstack/nova master: Delete disk metadata when detaching volume https://review.openstack.org/435082
19:06:33 openstackgerrit Artom Lifshitz proposed openstack/nova master: Plumbing for tagged volume attachment https://review.openstack.org/391941
19:06:34 openstackgerrit Artom Lifshitz proposed openstack/nova master: API support for tagged device attachment https://review.openstack.org/376021
19:06:36 openstackgerrit Artom Lifshitz proposed openstack/nova master: Libvirt support for tagged nic attachment https://review.openstack.org/415520
19:10:56 openstackgerrit Steve Noyes proposed openstack/nova master: WIP: update live migration to use v3 cinder api https://review.openstack.org/463987
19:11:46 melwitt sdague: patch has been un-WIP https://review.openstack.org/#/c/465194
19:12:03 sdague melwitt: +2, thanks
19:12:33 melwitt thx
19:16:21 cdent dansmith, bauzas, edleafe I tried to leave some questions on https://review.openstack.org/#/c/465175/ to capture my concerns, but not sure if they are clear
19:17:56 openstackgerrit Matt Riedemann proposed openstack/nova master: Destroy the ReqSpec object when deleting the instance https://review.openstack.org/391060
19:40:09 openstackgerrit Matt Riedemann proposed openstack/nova master: Plumbing for tagged nic attachment https://review.openstack.org/415622
19:40:11 openstackgerrit Matt Riedemann proposed openstack/nova master: Delete nic metadata when detaching interface https://review.openstack.org/435387
19:40:13 openstackgerrit Matt Riedemann proposed openstack/nova master: Libvirt support for tagged volume attachment https://review.openstack.org/418100
19:40:18 openstackgerrit Matt Riedemann proposed openstack/nova master: Delete disk metadata when detaching volume https://review.openstack.org/435082
19:40:23 openstackgerrit Matt Riedemann proposed openstack/nova master: Plumbing for tagged volume attachment https://review.openstack.org/391941
19:40:24 openstackgerrit Matt Riedemann proposed openstack/nova master: API support for tagged device attachment https://review.openstack.org/376021
19:40:27 openstackgerrit Matt Riedemann proposed openstack/nova master: Libvirt support for tagged nic attachment https://review.openstack.org/415520
19:45:21 melwitt cfriesen: had you filed a bug for the increase in DB connections in Newton? I'll need one to refer to in my patch
19:56:05 openstackgerrit Matt Riedemann proposed openstack/nova master: Remove DeviceIsBusy exception https://review.openstack.org/465723
20:01:01 openstackgerrit melanie witt proposed openstack/nova master: Cache database and message queue connection objects https://review.openstack.org/465042
20:06:22 zzzeek melwitt: can you tell me about https://review.openstack.org/#/c/465042/3/nova/context.py
20:07:42 melwitt zzzeek: yes! I was hoping you would be able to comment on this. recently we've seen a significant increase in number of DB connections since we've been doing DB connection switching for cells
20:08:01 zzzeek melwitt: so...did you try changing pool_size and max_overflow ?
20:08:50 zzzeek melwitt: also when you say "switching" - are you retrieving independent connections per cell DB or are you "switching" a single connection to refer to different databases within a single MySQL server?
20:09:10 zzzeek IIUC "cells" would be in different DB servers altogether so they'd have to have independent DB connections
20:09:27 dansmith zzzeek: correct, we're just surprised that when we create and stop using one, they don't seem to be gc'd
20:09:35 melwitt zzzeek: independent connection. to make a connection we create a new oslo.db transaction context manager
20:09:42 dansmith zzzeek: i.e. every time we do that switch (i.e. new connection), we get a connection that never goes away
20:09:50 zzzeek dansmith / melwitt OK, so, the SQLAlchemy engine by default uses a connection pool
20:10:01 zzzeek if you don't want it to pool anything, you need to turn it off
20:10:23 dansmith zzzeek: the pool is keeping references to all the connections then?
20:10:51 zzzeek dansmith: it keeps refernces to as many connections as you've told it to. our default in oslo.db is probably 5 at the moment
20:11:10 dansmith zzzeek: we end up with ~1400 connections after a tempest run
20:11:38 dansmith I'm guessing it doesn't know that each re-connection to the same db is reusable though? so it's keeping 5 connections of every transaction manager?
20:11:50 zzzeek dansmith: we end up with 1.4K connections on an openstack even w/o separate cell DBs if we have dozens of processes. it's kind of a problem to which there are a few solutions
20:11:53 dansmith if that's the case, we should just not do that thing (i.e. do what melwitt has proposed)
20:11:57 melwitt zzzeek: I had thought the pool would hand back an object that is the same DB as before
20:12:10 dansmith zzzeek: well, it doesn't happen with melwitt's patch
20:12:25 zzzeek dansmith: right but, you don't need a patch. just turn down the pooling
20:12:53 zzzeek dansmith: with one exception, which is that to disable the pool entirely, you need to use the "nullpool" implementation and we might need to add a hook to oslo.db if you really want that. but that is not what melwitt's patch is doing.
20:13:02 dansmith well, we get different behavior than what we have today.. meaning we defeat the benefits of the connection pooling , if there are benefits
20:13:39 zzzeek dansmith: just in the last few IRC lines im not sure if all three of us are clear on every concept here, for example, I don't know what a "Transaction manager" is
20:13:52 dansmith zzzeek: the oslo.db context
20:14:02 melwitt it's the _TransactionContextManager class in oslo.db
20:14:06 dansmith https://review.openstack.org/#/c/465042/3/nova/context.py
20:14:10 dansmith L384 there ^
20:14:12 zzzeek also, the idea of a "pool" is that yes, it has a connection that it will reuse as needed
20:14:34 melwitt okay, so I don't know what we're doing to not get the reusability
20:15:05 dansmith melwitt: the pool manager has to know that a repeated connection attempt is "the same" in order to re-use an existing connection
20:15:19 dansmith so I'm wondering if we're confusing it such that it isn't re-using every time we create a new context there
20:15:34 zzzeek dansmith: ok more terms that confuse me, what is "the pool manager"
20:15:35 melwitt right. how does it normally know that I guess? yeah
20:16:04 dansmith zzzeek: I'm sure I'm not using the right term.. whatever the thing is in sqla that does the pooling.. you said "nullpool" above, which would be one of those things
20:16:21 dansmith melwitt: well, it can do connection pooling with one context easily
20:16:31 melwitt right ...
20:16:39 dansmith and that's all we've needed in the past
20:17:01 dansmith but now we're creating lots of those things
20:17:03 zzzeek dansmith: OK. so we are talking aobut SQLAlchemy QueuePool or NullPool. it doesn't know about connection attempts being "the same" or anything like that, it basically is asked for a DB connection, and it gives you one from its pool. if it doesnt have any, it either decides to give you an ad-hoc connection based on what you've configured "max_overflow" towards, or, it makes you wait
20:17:06 melwitt zzzeek: this is how we establish db connections (static) and what we did to "switch" is just create a new one of those each time https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L79-L80
20:17:07 mriedem i'd think it would key off the connection url
20:17:24 melwitt mriedem: that's what I had thought
20:17:31 dansmith maybe, but that has creds in it
20:17:47 dansmith zzzeek: but is that pool global, or per-context or what?
20:18:21 zzzeek dansmith: if nova is architected here per best practices, you'd have one pool per unique database URL
20:18:42 dansmith heh
20:18:45 zzzeek dansmith: if OTOH you are creating brand new engines every time you get a requset, then you're making new pools each time and that is the wrong way to do that
20:19:01 dansmith we're a long ways away from the pool in nova, so it's hard to say
20:19:27 zzzeek dansmith: so in this patch i see "db_connection", and if that is in fact an "Engine" object, or say the enginefacade factory around that, then that's OK, that's what should happen here.
20:19:40 zzzeek that's not a "connection", it's a factory for connections
20:19:47 zzzeek if that's the case, I dont really know what this is
20:20:20 dansmith enginefacade.transaction_context()
20:20:25 dansmith it's that ^
20:20:28 melwitt it's a _TransactionContextManager
20:20:41 zzzeek dansmith / melwitt ok
20:20:56 melwitt I modeled it off how it's done in our db/sqlalchemy/api.py for the static connections
20:21:32 zzzeek melwitt: OK then that's fine let me update my thing
20:21:55 efried sdague Thinking I should blast the ML to let 'em know about journalctl -a, yah?
20:21:59 zzzeek melwitt: to clarify, previosly, everyime new cell DB were used, you'd build up whole new _TransactionContextManager per request ?
20:22:22 efried I mean, it's in the doc now (when does that get flushed to docs.openstack.org?) but who's gonna go reread it?
20:23:03 melwitt zzzeek: yeah. and that was based on my limited knowledge of how to use oslo.db, if I want to establish a new connection from only a connection url, it seemed like that's the only way?
20:23:23 melwitt is to use enginefacade.transaction_context()
20:23:44 zzzeek melwitt: OK so, is that version of the code what's released in production and all that? because that will perform very, very badly
20:23:51 cfriesen who knows neutron and live-migration? Do we really need to process "network-changed" events on the dest compute node during a migration when we're going to explicitly call network_info = self.network_api.get_instance_nw_info() anyways in post_live_migration_at_destination()?
20:24:16 melwitt zzzeek: yes. that's why at the moment, it seems like caching these is the way to fix this, and backport it all the way to newton
20:24:52 zzzeek melwitt: it is. So, background, the transactioncontextmanager is a facade around the concept of the SQLAlchemy engine
20:25:07 mriedem cfriesen: we probably don't need to, but does refreshing the network info twice do any harm?
20:25:53 zzzeek this is why it has a method "get_engine()". when you use it as a decorator or context manager, it goes in to a "block" in which scope it actually procures an ORM session which uses an actual sqlalchemy.engine.Connection object
20:26:52 zzzeek melwitt: the concept of the Engine is described at http://docs.sqlalchemy.org/en/rel_1_1/core/engines.html http://docs.sqlalchemy.org/en/rel_1_1/core/connections.html http://docs.sqlalchemy.org/en/rel_1_1/core/pooling.html and would provide some background on what's going on
20:27:44 melwitt zzzeek: thanks
20:27:57 zzzeek melwitt: the history of "enginefacade" itself is that when I came to work on openstack, there was already an "EngineFacade" local to openstack / oslo that evreyone was using, so the curent oslo.db version tries to improve upon that. however, like all layers of indirection, it is confusing. I wrote it and I never remember how it is organized when I go back and look at it, because it had to work on top of all these existing
20:27:57 zzzeek patterns
20:28:57 cfriesen mriedem: turns out that it does when you have a lot (like 16) vifs on an instance. The live migration code can end up blocking for like 35+ seconds waiting to get the 'refresh_cache-%s' lock because it's racing against the incoming network-change events
20:29:11 zzzeek melwitt: so short story, if you make a new engine (and therefore a new connection pool) on every request, then yes, the "pool" is not working because you are making a whole new pool each time
20:29:19 melwitt zzzeek: I see. I too, find confusing :)

Earlier   Later