Saturday, 2015-02-21

*** openstack has joined #openstack-barbican16:40
openstackgerritLouis Taylor proposed openstack/barbican: Use dictionary comprehensions and literals  https://review.openstack.org/15806916:48
*** atiwari1 has joined #openstack-barbican16:54
*** atiwari has quit IRC16:56
*** woodster_ has quit IRC17:20
*** woodster_ has joined #openstack-barbican17:24
*** jaosorior has quit IRC17:41
openstackgerritJohn Vrbanac proposed openstack/barbican: Fixing unable to retrieve req id bug  https://review.openstack.org/15803417:43
*** kgriffs|afk is now known as kgriffs17:50
*** kgriffs is now known as kgriffs|afk18:40
*** zz_dimtruck is now known as dimtruck19:48
*** dave-mccowan has quit IRC20:05
*** chlong has quit IRC20:23
*** jaosorior has joined #openstack-barbican20:46
jaosoriorjvrbanac: pong. Greetings from the airport20:48
jvrbanacjaosorior, greetings20:49
jvrbanacjaosorior, I added the context you wanted to the CR20:49
jaosoriorAnd I workflowed that cr about a minute ago :P20:50
jvrbanacjaosorior, oh lol thx!20:50
jaosoriorHehe20:50
jaosoriorHow's stuff there?20:51
jvrbanacjaosorior, pretty good. I'm trying to clean up some garbage here in Barbican. Make some of this code a little easier to understand20:54
jaosoriorNiiiiice20:55
jaosoriorWhat are you cleaning up?20:55
jvrbanacjaosorior, right now, the app and database initialization20:55
jaosoriorÖhö, let's see20:56
jvrbanacjaosorior, Paul and myself spent a decent amount of time on Friday playing around with the secret store plugin manager and realized that there are a couple race conditions around that stuff. I'll have a couple CR's up in a bit to resolve some of that stuff20:58
jaosoriorAaah is it the stuff related the locks that he mentioned?20:58
jaosoriorDuuuuude, nice,  let me know when you have that stuff up. Got about an hour and a half before I board20:59
jvrbanacjaosorior, kind of. We removed the lock and all hell broke loose and brought up the race condition20:59
jvrbanacjaosorior, I've gotta run a few errands here, so I probably won't get the CR's up until tonight or tomorrow21:00
jvrbanacjaosorior, btw, how was the remaining time in Austin?21:03
jaosoriorReally good!21:04
jaosoriorThough I might have exceeded myself on the eating part21:04
jvrbanacjaosorior, I thought that was part of the idea? ;)21:05
jaosoriorjvrbanac: hahaha anyway, it was brilliant21:07
jvrbanacjaosorior, well, I'm glad you had a good time! Did you go exploring around Austin or did you hang out downtown?21:09
openstackgerritMerged openstack/barbican: Fixing unable to retrieve req id bug  https://review.openstack.org/15803421:10
jaosoriorDid some exploring and took a couple of tours. It was nice hanging out with the family21:11
jvrbanacawesome!21:11
woodster_jaosorior: have a safe flight back21:22
jaosoriorwoodster_: Thanks Mr.!21:22
woodster_jvrbanac: I'm curious what race conditions you guys ran into. Did performance improve at all?21:23
jvrbanacwoodster_, performance didn't increase, but the app startup will be more stable21:24
jvrbanacwoodster_, atleast it didn't seem to increase21:24
jaosoriorstrange21:24
jvrbanacwoodster_, for one thing the secret store plugin manager was getting re-initialized all the time. Also, when we removed the lock, it showed that we were starting the db connection on first request and the config wasn't loading fast enough to initialize the db connect for the request causing the requests to fail.21:28
woodster_jvrbanac: I'm glad you guys are gut checking things then. The other plug in managers might have similar issues.21:32
jvrbanacwoodster_, I'll have a CR up in a bit that removes a couple layers of abstraction around db initialization and making sure we initialize our connection right before we start the app21:33
jvrbanacwoodster_, at some point, I would like for us to revisit our configuration setup. From the way I see it, we only need the per-module config options for plugins. Everything within core should be able to be specified in a config module.21:36
jvrbanacwoodster_, having multiple points of config option registration within the core system is horrible to maintain21:37
reaperhulkyeah, it makes it so we have to lazy init everything >:(21:38
*** atiwari2 has joined #openstack-barbican21:40
woodster_jvrbanac: do you mean the config parser init in app.py, or the per module config registrations?21:40
jvrbanacwoodster_, reaperhulk, the per-module config registration junk. I would suggest we centralize the core config options and then see what we can do about better handling plugin configuration21:42
reaperhulkjvrbanac: do we do per module config outside of the plugins too?21:43
reaperhulkIf so, definitely should consolidate anything in barbican-core21:43
*** atiwari1 has quit IRC21:43
jaosoriorjvrbanac: I suggest you add that topic to the weekly. Would be good to bring it up there21:43
jvrbanacreaperhulk, yeah.21:43
jvrbanacjaosorior, yep21:43
woodster_jvrbanac: well that is what other projects where doing in the past, and it is nice locating those settings near the logic that uses them. Not opposed to a central approach though21:44
jvrbanacwoodster_, I'll be blunt... the other projects are wrong :P21:45
jaosoriorjvrbanac: If this will ease the deployers' and admin's life, then it's probably a good idea21:46
jvrbanacjaosorior, agreed. It also makes our lives easier as we don't have to chase config values around the app and worry about when they'll be available21:47
woodster_jvrbanac: just to be clear, are you seeing a race condition with the config option settings, on the CONF object? If so that could be an Oslo config bug21:47
reaperhulkit's not a bug21:50
reaperhulkit's a bad design decision21:50
woodster_jvrbanac: my recollection is that the app.py config.parse_args() call freezes the config options bases on the Barbican-API.conf file settings21:50
reaperhulkYou are correct woodster_21:50
jvrbanacwoodster_, it's not a problem with Olso config. Because we define config options all over the place within Barbican, we can't load the configuration until all options from all loaded modules have been registered. This is only because we register those on a per module basis21:50
woodster_Sorry using21:50
woodster_My phone and distracted:)21:51
*** kgriffs|afk is now known as kgriffs21:51
woodster_jvrbanac: I recall this was to support a CLI-based help process, so showing all the options you are really using in the app. Attempts to add after parse_args() will fail. At any rate give central a try. Will be a lot of files though.21:54
jvrbanacwoodster_, it shouldn't matter where options are registered21:58
reaperhulkjvrbanac: I think woodster_'s statement is that the default option registry may not matter? It will load whatever is in the config when you call parse_args()?21:59
woodster_reaperhulk: that's correct, but it's been awhile since I dug into Oslo foo21:59
reaperhulkif so, then moving the config options to be central isn't very useful since the ultimate issue will always be when we invoke parse_args22:00
reaperhulk(Ideally we'd invoke that before module import so we could have module level variables with no lazy load required)22:00
jvrbanacreaperhulk, woodster_ yeah. hmm... I'll have to play with it a bit and see comes out of that22:00
woodster_The config stuff did seem over engineered to me, just sayin'22:01
jaosoriorwoodster_: Got time to review this one? https://review.openstack.org/#/c/157068/4/barbican/tests/api/test_resources.py22:08
jaosoriorI mean  https://review.openstack.org/#/c/15706822:08
openstackgerritJohn Vrbanac proposed openstack/barbican: Attempting to clean up some of the db session code  https://review.openstack.org/15808522:11
woodster_ jaosorior Ozz could you also add a functional test for the new-way call too?22:22
jaosoriorI thought I had22:23
jaosoriorwoodster_: I did22:23
jaosoriorhttps://review.openstack.org/#/c/157068/4/functionaltests/api/v1/behaviors/secret_behaviors.py22:23
jaosoriorthis changes the secret behaviours22:23
jaosoriorand  I did what we discussed22:23
jaosoriorthat I would just add a couple of tests using the old way22:23
jaosoriorto show that it's still compatible22:24
jaosoriorbut the new way is now the default22:24
jaosoriorin the tests22:24
woodster_jaosorior: oh so existing tests use the new way, got it22:25
openstackgerritMerged openstack/barbican: Use dictionary comprehensions and literals  https://review.openstack.org/15806922:26
kragniz\o/ barbican patch #4!22:33
openstackgerritJohn Vrbanac proposed openstack/barbican: Attempting to clean up some of the db session code  https://review.openstack.org/15808522:36
jvrbanacwoodster_, ping22:45
woodster_kragniz: nice! Keep 'em coming please22:45
woodster_jvrbanac: I added comments to your CR22:46
jvrbanacwoodster_, yeah I just saw that.22:46
woodster_jvrbanac: is the main issue that the db stuff is lazy configured? If so I think your app.py method is fine sans the last two calls. The other original lines should work as they were though22:48
jvrbanacwoodster_, yeah the db being lazy configured is killing the first few calls the service takes.22:50
jvrbanacwoodster_, the point about the workers is good. I didn't realize we were using the same function for the transaction hook as for the worker db init22:52
jvrbanacwoodster_, btw, the sessions are still scoped on a per-thread/process basis22:59
*** chlong has joined #openstack-barbican22:59
jvrbanacwoodster_, if you look at get_session, it's still creating a new scoped session22:59
woodster_ jvrbanac also if the sequence isn't right objects linger in the session across transactions . That's why I'd try the cold start idea first, and then change those lifecycle methods23:00
woodster_ jvrbanac I'll have to refresh my memory on the db steps23:07
jvrbanacwoodster_, I guess I'm not understanding how this is working. The way I saw it, We needed to setup the engine and session maker in the beginning, after that, just calling get_session() created us a scoped session for our thread and we were good to go from there. We really weren't using start at all except to lazy init the engine and session in the beginning23:09
*** kgriffs is now known as kgriffs|afk23:10
woodster_jvrbanac: yeah I think you are right...looking at entire module now....23:17
jvrbanacwoodster_, I think if we were to be very strict with the transaction concept, we would need to rewrite all the repos to follow something like that. However, I'm not sure that would solve the initial request failures as it would still lazy init the engine and sessions23:21
woodster_jvrbanac: I think your code is fine, though line 109 is not really needed as repos call that as needed. Line 152 releases the session resources so the next get-session call will start a new transaction. So really just change doctext and remove 10923:28
jvrbanacwoodster_, ok. I also, do need to make sure the worker initializes correctly as well.23:29
woodster_jvrbanac: you can just call your setup call from the bin scripts I think, one for worker one for keystone listener23:31
jvrbanacwoodster_, Hmm... yeah I see it. I suppose we could also do it in the TaskServer constructor23:35
jvrbanacwoodster_, would someone run the TaskServer outside of barbican-worker.py?23:36
woodster_jvrbanac: those bin scripts boot things up (there will be multiple worker-like processes like that eventually) so if you can find a common code to put the init in that would be best23:41
jvrbanacwoodster_, ok. will do23:42
woodster_jvrbanac: nice thanks23:46

Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!