Re: [openstack-dev] [all] [oslo] Proposed database connectivity patterns
On Oct 10, 2014, at 12:52 PM, Ihar Hrachyshka wrote: > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA512 > > On 10/10/14 20:21, Joshua Harlow wrote: >> On Oct 10, 2014, at 3:13 AM, Ihar Hrachyshka >> wrote: >> >> On 09/10/14 21:29, Mike Bayer wrote: > So so far, everyone seems really positive and psyched about > the proposal. > > It looks like providing some options for how to use would be > best, that is provide decorators and context managers. > > Now the thing with the context manager, it can be as I > stated: > > with sql.reader() as session: > > or we can even have that accept the “context”: > > with sql.reader(context) as session: > > The latter again avoids having to use thread locals. > > But can I get a feel for how comfortable we are with using > thread local storage to implement this feature? I had > anticipated people wouldn’t like it because it’s kind of a > “global” object, even though it will be hidden behind this > facade (of course CONF is global as is sys.modules, and I > think it is fine). If I just use a tlocal, this whole > thing is pretty simple. >> >> Won't the approach conflict with eventlet consumers that for some >> reason do not patch thread module, or do not patch it early enough? >> I guess in that case we may end up with mixed contexts. >> >>> Eck, this is not something people should be doing (not patching >>> the thread module). >> >>> Example for why @ >>> https://gist.github.com/harlowja/9c35e443dfa136a4f965 (run that >>> and see your program lock up). >> >>> Once you accept/use eventlet, u shouldn't really be accepted half >>> of it, otherwise there be dragons there; especially since >>> openstack doesn't control what external library code does inside >>> those libraries (and rightfully so it shouldn't), and if any >>> library does anything like that gist code, the whole application >>> will lock up... > > Real life sucks. In Neutron, we have at least two files that patch > only some modules (though all of them patch threading). [Not that I > mean that those files should not patch the whole library set; I have > no defined view on the matter.] > Yup it does, and once u accept eventlet it starts to become hard to get off it... It's 'viral', and for better or worse openstack has got the virus... So that means everything that openstack touches/uses also needs to be compatible with that virus... It's to bad guido just didn't accept eventlet/greenlet into mainline python, that would of made our job easier (and then added a python option or something like `python -g` that turns on greenthreading by default when the interpreter is started up)... Thats the crappy part with anything that monkey patches all the things, its not so easy to accept half of a monkey... > Though I agree that consumers with some modules unpatched are not > something that should be seriosly considered, we're still left with > consumers that do not patch their libraries early enough. We had an > issue with tlocal when porting Neutron to oslo.messaging that resulted > in the following patch: https://review.openstack.org/#/c/96791/ The > issue showed up in very weird way hard to debug, so if possible, I > would try to avoid similar situations with oslo.db. I'm unsure on this one, I don't think we should be making oslo.* that succumbs to the same eventlet virus just because openstack projects aren't correctly using eventlet. The same bug that was found in oslo.messaging could of been any third part library that wasn't imported in the right order in all honesty. So it seems like we should fix the projects (as in that review) instead of making the oslo.* libraries less future compatible... If eventlet/greenlet got merged into mainline python then I'd say probably say the reverse, but it didn't. > > /Ihar > -BEGIN PGP SIGNATURE- > Version: GnuPG/MacGPG2 v2.0.22 (Darwin) > > iQEcBAEBCgAGBQJUODkZAAoJEC5aWaUY1u57QrcIANLtMi1jRrfHnwV2xnwYGPOl > QwZWDG6i/cdnzqrwGz+0chqRxu4KWZB5gAfMR4/ralHvGGbaa1vGp9wtzsfIJTB1 > RuyE7XKXUX4rU3WoAOB7F3uF1WzFpI8ourunBG4OCE0t0YT89z9mCLfOTZNNKGxH > OE89n4pnyMd3GXGtoxVINyA6pqQotYuHKG6o6LbhmTZ1UoL3P3rO+Onb1Ma2BF6Z > VO84smdMnOC3yVtv4TGwrW5iBoU3RwOqxooOg4g5Jam6D++kzCKrPT9wdY7TNe+C > A6IvMlad0RbygkhrLyWIWXaqCAg5VzzDvk4z6dywVkmoWS7V861mjiV4unbr0X8= > =5ovh > -END PGP SIGNATURE- > > ___ > OpenStack-dev mailing list > OpenStack-dev@lists.openstack.org > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [all] [oslo] Proposed database connectivity patterns
-BEGIN PGP SIGNED MESSAGE- Hash: SHA512 On 10/10/14 20:21, Joshua Harlow wrote: > On Oct 10, 2014, at 3:13 AM, Ihar Hrachyshka > wrote: > > On 09/10/14 21:29, Mike Bayer wrote: So so far, everyone seems really positive and psyched about the proposal. It looks like providing some options for how to use would be best, that is provide decorators and context managers. Now the thing with the context manager, it can be as I stated: with sql.reader() as session: or we can even have that accept the “context”: with sql.reader(context) as session: The latter again avoids having to use thread locals. But can I get a feel for how comfortable we are with using thread local storage to implement this feature? I had anticipated people wouldn’t like it because it’s kind of a “global” object, even though it will be hidden behind this facade (of course CONF is global as is sys.modules, and I think it is fine). If I just use a tlocal, this whole thing is pretty simple. > > Won't the approach conflict with eventlet consumers that for some > reason do not patch thread module, or do not patch it early enough? > I guess in that case we may end up with mixed contexts. > >> Eck, this is not something people should be doing (not patching >> the thread module). > >> Example for why @ >> https://gist.github.com/harlowja/9c35e443dfa136a4f965 (run that >> and see your program lock up). > >> Once you accept/use eventlet, u shouldn't really be accepted half >> of it, otherwise there be dragons there; especially since >> openstack doesn't control what external library code does inside >> those libraries (and rightfully so it shouldn't), and if any >> library does anything like that gist code, the whole application >> will lock up... Real life sucks. In Neutron, we have at least two files that patch only some modules (though all of them patch threading). [Not that I mean that those files should not patch the whole library set; I have no defined view on the matter.] Though I agree that consumers with some modules unpatched are not something that should be seriosly considered, we're still left with consumers that do not patch their libraries early enough. We had an issue with tlocal when porting Neutron to oslo.messaging that resulted in the following patch: https://review.openstack.org/#/c/96791/ The issue showed up in very weird way hard to debug, so if possible, I would try to avoid similar situations with oslo.db. /Ihar -BEGIN PGP SIGNATURE- Version: GnuPG/MacGPG2 v2.0.22 (Darwin) iQEcBAEBCgAGBQJUODkZAAoJEC5aWaUY1u57QrcIANLtMi1jRrfHnwV2xnwYGPOl QwZWDG6i/cdnzqrwGz+0chqRxu4KWZB5gAfMR4/ralHvGGbaa1vGp9wtzsfIJTB1 RuyE7XKXUX4rU3WoAOB7F3uF1WzFpI8ourunBG4OCE0t0YT89z9mCLfOTZNNKGxH OE89n4pnyMd3GXGtoxVINyA6pqQotYuHKG6o6LbhmTZ1UoL3P3rO+Onb1Ma2BF6Z VO84smdMnOC3yVtv4TGwrW5iBoU3RwOqxooOg4g5Jam6D++kzCKrPT9wdY7TNe+C A6IvMlad0RbygkhrLyWIWXaqCAg5VzzDvk4z6dywVkmoWS7V861mjiV4unbr0X8= =5ovh -END PGP SIGNATURE- ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [all] [oslo] Proposed database connectivity patterns
On Oct 10, 2014, at 3:13 AM, Ihar Hrachyshka wrote: > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA512 > > On 09/10/14 21:29, Mike Bayer wrote: >> So so far, everyone seems really positive and psyched about the >> proposal. >> >> It looks like providing some options for how to use would be best, >> that is provide decorators and context managers. >> >> Now the thing with the context manager, it can be as I stated: >> >> with sql.reader() as session: >> >> or we can even have that accept the “context”: >> >> with sql.reader(context) as session: >> >> The latter again avoids having to use thread locals. >> >> But can I get a feel for how comfortable we are with using thread >> local storage to implement this feature? I had anticipated people >> wouldn’t like it because it’s kind of a “global” object, even >> though it will be hidden behind this facade (of course CONF is >> global as is sys.modules, and I think it is fine). If I just >> use a tlocal, this whole thing is pretty simple. > > Won't the approach conflict with eventlet consumers that for some > reason do not patch thread module, or do not patch it early enough? I > guess in that case we may end up with mixed contexts. Eck, this is not something people should be doing (not patching the thread module). Example for why @ https://gist.github.com/harlowja/9c35e443dfa136a4f965 (run that and see your program lock up). Once you accept/use eventlet, u shouldn't really be accepted half of it, otherwise there be dragons there; especially since openstack doesn't control what external library code does inside those libraries (and rightfully so it shouldn't), and if any library does anything like that gist code, the whole application will lock up... > > /Ihar > -BEGIN PGP SIGNATURE- > Version: GnuPG/MacGPG2 v2.0.22 (Darwin) > > iQEcBAEBCgAGBQJUN7FeAAoJEC5aWaUY1u57sq0IANSM8gKCQZbgEY62uvyhZqzN > 9gRDcFbTrH/yUNMv0tt3+e2vVEbn8VIatDWG4/OYghzuI1BerKzhP7JD5J+mV8yK > sB0fc6ybHmz14T962LhFkAxWKybPW3sJO/0GIy606ty0OEV8QAgPwaYvjW596MUa > BAp0IRAz4/MglT/M80OkT2jFdMV+a9SAFUKvnF/21KSGo/t4qcawA/+B0c3Ownle > eYt+Auk3hcgJnZAvCOwy7bcAb1b12gonk4nIwMl/Mik8IKNR5fnl1IqTiISUO2Jw > PV95/7muukSrt54lY+9u4SW9o/Zf/gaNCMmK3xfDmvMug6tk0SWzkzBZLva3wNc= > =04Wx > -END PGP SIGNATURE- > > ___ > OpenStack-dev mailing list > OpenStack-dev@lists.openstack.org > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [all] [oslo] Proposed database connectivity patterns
On Oct 10, 2014, at 11:41 AM, Mike Bayer wrote: > I’ve been asking a lot about “hey are people cool with thread locals?” and > have been waiting for what the concerns are. > > Since I wrote that email I’ve shifted, and I’ve been considering only: > > @sql.reader > def my_api_method(context, …): > context.session > > def my_api_method(context, …): > with sql.using_reader(context) as session: >session , context.session > > because in fact, if you *want* to use a thread local context, you can, > explicitly with the above: > > GLOBAL_CONTEXT = threading.local() > > def my_api_method(…): > with sql.using_reader(GLOBAL_CONTEXT) as session: >session > > I like that one the best. But again, Keystone folks would need to accept > this explicitness. > > The challenge on my end is not technical in any way. It’s getting every > project to agree on a single approach and not getting bogged down with > idealistics (like, “let’s build a dependency injection framework!”). > Because this “everyone does it their own way” thing is crazy and has to stop. I’ve now pushed these changes, as well as a summation of all the alternatives so far, to the latest release. See https://review.openstack.org/#/c/125181/. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [all] [oslo] Proposed database connectivity patterns
On Oct 10, 2014, at 6:13 AM, Ihar Hrachyshka wrote: > Signed PGP part > On 09/10/14 21:29, Mike Bayer wrote: > > So so far, everyone seems really positive and psyched about the > > proposal. > > > > It looks like providing some options for how to use would be best, > > that is provide decorators and context managers. > > > > Now the thing with the context manager, it can be as I stated: > > > > with sql.reader() as session: > > > > or we can even have that accept the “context”: > > > > with sql.reader(context) as session: > > > > The latter again avoids having to use thread locals. > > > > But can I get a feel for how comfortable we are with using thread > > local storage to implement this feature? I had anticipated people > > wouldn’t like it because it’s kind of a “global” object, even > > though it will be hidden behind this facade (of course CONF is > > global as is sys.modules, and I think it is fine). If I just > > use a tlocal, this whole thing is pretty simple. > > Won't the approach conflict with eventlet consumers that for some > reason do not patch thread module, or do not patch it early enough? I > guess in that case we may end up with mixed contexts. I’ve been asking a lot about “hey are people cool with thread locals?” and have been waiting for what the concerns are. Since I wrote that email I’ve shifted, and I’ve been considering only: @sql.reader def my_api_method(context, …): context.session def my_api_method(context, …): with sql.using_reader(context) as session: session , context.session because in fact, if you *want* to use a thread local context, you can, explicitly with the above: GLOBAL_CONTEXT = threading.local() def my_api_method(…): with sql.using_reader(GLOBAL_CONTEXT) as session: session I like that one the best. But again, Keystone folks would need to accept this explicitness. The challenge on my end is not technical in any way. It’s getting every project to agree on a single approach and not getting bogged down with idealistics (like, “let’s build a dependency injection framework!”).Because this “everyone does it their own way” thing is crazy and has to stop. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [all] [oslo] Proposed database connectivity patterns
-BEGIN PGP SIGNED MESSAGE- Hash: SHA512 On 09/10/14 21:29, Mike Bayer wrote: > So so far, everyone seems really positive and psyched about the > proposal. > > It looks like providing some options for how to use would be best, > that is provide decorators and context managers. > > Now the thing with the context manager, it can be as I stated: > > with sql.reader() as session: > > or we can even have that accept the “context”: > > with sql.reader(context) as session: > > The latter again avoids having to use thread locals. > > But can I get a feel for how comfortable we are with using thread > local storage to implement this feature? I had anticipated people > wouldn’t like it because it’s kind of a “global” object, even > though it will be hidden behind this facade (of course CONF is > global as is sys.modules, and I think it is fine). If I just > use a tlocal, this whole thing is pretty simple. Won't the approach conflict with eventlet consumers that for some reason do not patch thread module, or do not patch it early enough? I guess in that case we may end up with mixed contexts. /Ihar -BEGIN PGP SIGNATURE- Version: GnuPG/MacGPG2 v2.0.22 (Darwin) iQEcBAEBCgAGBQJUN7FeAAoJEC5aWaUY1u57sq0IANSM8gKCQZbgEY62uvyhZqzN 9gRDcFbTrH/yUNMv0tt3+e2vVEbn8VIatDWG4/OYghzuI1BerKzhP7JD5J+mV8yK sB0fc6ybHmz14T962LhFkAxWKybPW3sJO/0GIy606ty0OEV8QAgPwaYvjW596MUa BAp0IRAz4/MglT/M80OkT2jFdMV+a9SAFUKvnF/21KSGo/t4qcawA/+B0c3Ownle eYt+Auk3hcgJnZAvCOwy7bcAb1b12gonk4nIwMl/Mik8IKNR5fnl1IqTiISUO2Jw PV95/7muukSrt54lY+9u4SW9o/Zf/gaNCMmK3xfDmvMug6tk0SWzkzBZLva3wNc= =04Wx -END PGP SIGNATURE- ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [all] [oslo] Proposed database connectivity patterns
So so far, everyone seems really positive and psyched about the proposal. It looks like providing some options for how to use would be best, that is provide decorators and context managers. Now the thing with the context manager, it can be as I stated: with sql.reader() as session: or we can even have that accept the “context”: with sql.reader(context) as session: The latter again avoids having to use thread locals. But can I get a feel for how comfortable we are with using thread local storage to implement this feature? I had anticipated people wouldn’t like it because it’s kind of a “global” object, even though it will be hidden behind this facade (of course CONF is global as is sys.modules, and I think it is fine). If I just use a tlocal, this whole thing is pretty simple. I’m going to do another pass that attempts to unify these three syntaxes - I’m proposing calling the context manager “using_” so that it can be differentiated from the decorator (e.g. so each function doesn’t need to inspect arguments): @sql.reader def my_api_method(context, …): context.session def my_api_method(context, …): with sql.using_reader(context) as session: session , context.session def my_api_method(…): with sql.using_reader() as session: session all three will be fully interchangeable - meaning they will ultimately use thread local storage in any case.For now I think if sql.using_reader(context) or @sql.reader is called with different context identities in a single call stack, it should raise an exception - not that we can’t support that, but whether it means we should push new state onto the “stack” or not is ambiguous at the moment so we should refuse to guess. On Oct 8, 2014, at 5:07 PM, Mike Bayer wrote: > Hi all - > > I’ve drafted up my next brilliant idea for how to get Openstack projects to > use SQLAlchemy more effectively. The proposal here establishes significant > detail on what’s wrong with the current state of things, e.g. the way I see > EngineFacade, get_session() and get_engine() being used, and proposes a new > system that provides a true facade around a managed context. But of course, > it requires that you all change your code! (a little bit). Based on just a > few tiny conversations on IRC so far, seems like this might be a hard sell. > But please note, most projects are pretty much broken in how they use the > database - this proposal is just a first step to making it all non-broken, if > not as amazing and cool as some people wish it could be. Unbreaking the code > and removing boilerplate is the first step - with sane and declarative > patterns in place, we can then build in more bells and whistles. > > Hoping you’re all super curious now, here it is! Jump in: > https://review.openstack.org/#/c/125181/ > > - mike > > > > > > > > ___ > OpenStack-dev mailing list > OpenStack-dev@lists.openstack.org > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [all] [oslo] Proposed database connectivity patterns
Hi Mike, Great stuff! Fixing the mess with transactions and their scope is probably one of the most important tasks for us, IMO. I look forward for this to be implemented in oslo.db and consuming projects! Thanks, Roman On Thu, Oct 9, 2014 at 12:07 AM, Mike Bayer wrote: > Hi all - > > I’ve drafted up my next brilliant idea for how to get Openstack projects to > use SQLAlchemy more effectively. The proposal here establishes significant > detail on what’s wrong with the current state of things, e.g. the way I see > EngineFacade, get_session() and get_engine() being used, and proposes a new > system that provides a true facade around a managed context. But of course, > it requires that you all change your code! (a little bit). Based on just a > few tiny conversations on IRC so far, seems like this might be a hard sell. > But please note, most projects are pretty much broken in how they use the > database - this proposal is just a first step to making it all non-broken, if > not as amazing and cool as some people wish it could be. Unbreaking the code > and removing boilerplate is the first step - with sane and declarative > patterns in place, we can then build in more bells and whistles. > > Hoping you’re all super curious now, here it is! Jump in: > https://review.openstack.org/#/c/125181/ > > - mike > > > > > > > > ___ > OpenStack-dev mailing list > OpenStack-dev@lists.openstack.org > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
[openstack-dev] [all] [oslo] Proposed database connectivity patterns
Hi all - I’ve drafted up my next brilliant idea for how to get Openstack projects to use SQLAlchemy more effectively. The proposal here establishes significant detail on what’s wrong with the current state of things, e.g. the way I see EngineFacade, get_session() and get_engine() being used, and proposes a new system that provides a true facade around a managed context. But of course, it requires that you all change your code! (a little bit). Based on just a few tiny conversations on IRC so far, seems like this might be a hard sell. But please note, most projects are pretty much broken in how they use the database - this proposal is just a first step to making it all non-broken, if not as amazing and cool as some people wish it could be. Unbreaking the code and removing boilerplate is the first step - with sane and declarative patterns in place, we can then build in more bells and whistles. Hoping you’re all super curious now, here it is! Jump in: https://review.openstack.org/#/c/125181/ - mike ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
