17:59:40 #startmeeting trove-bp-review 17:59:41 Meeting started Mon Apr 21 17:59:40 2014 UTC and is due to finish in 60 minutes. The chair is SlickNik. Information about MeetBot at http://wiki.debian.org/MeetBot. 17:59:42 Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. 17:59:44 The meeting name has been set to 'trove_bp_review' 17:59:51 okay, that's better. 18:00:08 amrith: Let's give folks a couple of minutes to trickle in 18:01:50 #topic do permissions work right on this thing? 18:01:55 darn 18:01:58 #link https://wiki.openstack.org/wiki/Meetings/TroveMeeting 18:02:18 kevinconway: I think only the person who started the meeting can change the topic. 18:03:12 kevinconway: Yeah permission is working.. i just did a /kick kevinconway and you are still here 18:03:31 #topic: Unify common code in guest agent configurations 18:03:44 #topic Unify common code in guest agent configurations 18:03:53 maybe not? 18:03:56 I'll take that as my cue 18:04:05 so, SnowDust and I put this thing together 18:04:17 subsequently, it appears that hub_cap has made some changes 18:04:30 and those represent the direction we may want to go 18:04:33 hey so 18:04:40 afte some thought on this, and a ton of refactoring 18:04:40 so, for the present, I say table this blueprint 18:04:48 im not sure i like the approach im taking 18:04:51 lets chat about it for a sec amrith 18:04:56 ok, sure 18:05:04 so there _will_ be datastore specific items that the api/taksmgr need to know about 18:05:11 and frankly the guest may never care about 18:05:28 let me find an exampl eof the one i was lookin at 18:05:38 hmm its been a while i dont want to waste our time 18:05:49 so to the config loading snowdust has done 18:06:07 i think that it might be valid to have those as config options groups within the main trove config 18:06:10 cfg.mysql.blah 18:06:20 some of them dont matter, like the mount_point 18:06:23 that was an easy refactor 18:06:30 but some of them like the ports, start to look ugly in amap 18:06:48 but the part that snowdust didnt do was the autoloading 18:07:01 i think it hsould be based on the datasotres that you have "installed", aka the ones in the database 18:07:05 not based on yet another config 18:07:13 if you don tdeploy couch, dont load its options 18:07:30 does that make sense? should be perty easy to get that list from the db and autoload based on that 18:07:43 could these be in the capabilities BP? 18:07:50 hub_cap, that was a little different from what I was proposing in my bp 18:07:52 robertmyers: some of them yes 18:08:04 amrith: sure thats like a first step (what im tlkin about) 18:08:27 robertmyers: im not sure that, say, ports for a given datastore need to be in capabilities 18:08:35 root_on_create, i think yes 18:08:43 ok 18:09:01 and yes i know this is less of amirths and more of snowdusts work, and what ive done last wk 18:09:07 I agree. I think the ports don't make sense in capabilities. 18:09:10 but i think we can move forward w/ some of the autoloading 18:09:12 so hub_cap, looks like we have to align our thoughts on this, maybe we can get it ready for discussion next week? 18:09:14 and the configuration stuff 18:09:17 yea i think so amrith 18:09:23 its perty big 18:09:30 SlickNik: action is mine to follow up with hub_cap 18:09:31 might even be a summit talk 18:10:07 Okay, let's table this until next week. 18:10:08 hub_cap/SlickNik, I agree. the thrust of my idea is around eliminating duplicated code and there are many places where we are magically defining the same constants 18:10:13 thx 18:10:19 yup amrith ++ 18:10:44 amrith: Sounds good. 18:10:49 #topic Adding datastore and version to the backup API 18:11:01 robertmyers, all yours 18:11:03 lol that doenst work here ;) but u konw that 18:11:04 hehe 18:11:15 sure 18:11:34 basically we want to add the datastore to the backup api 18:11:39 Yeah, I know :). I was surprised that meetbot actually responded earlier. 18:12:03 I see amcrn had some comments? 18:12:39 #link https://wiki.openstack.org/wiki/Trove/backup-metadata 18:12:53 Yes, there's a couple of comments from him at the bottom of that page. 18:13:04 robertmyers: As part of https://blueprints.launchpad.net/trove/+spec/cross-region-backup-availability, i am adding datastore and version to the backup table 18:13:25 i am not changing the backup API nor the views to return them though 18:13:52 perhaps we can work on this in a way we dont step on each other 18:13:57 Well, the point is for someone to be able to tell what datastore was backed up 18:14:10 SO my BP is to change the view 18:15:14 i will make a change where the datastore is stored in the DB when a backup is started 18:15:14 esmute: I'm fine with killing this if you do that as well 18:15:23 robertmyers, is the version already in someplace and your bp is just to add it to the api? 18:15:43 amrith: no, this will add it to the DB 18:15:43 ir confused a bit 18:15:50 dont u already know the datastore basd on the backup? 18:15:57 hub_cap: no 18:15:58 ok.. ill take this since i sorta started it.. i'll be syncing with you to make sure it covers what you had in mind 18:16:00 sure change the view, but isint it tied to an instance, aka, u can generate it 18:16:13 i dont know if we need to "store it" right? 18:16:28 or is there no link to an instnace from a backup 18:16:37 well, if you delete the instance then the info is gone 18:16:42 is it? 18:16:46 hub_cap: it is tied to the instance. Even if the instance is deleted, the we can find the datastore by querying the soft deleted instance 18:16:47 or is it just DELETED=true 18:16:55 right esmute 18:17:00 id rather _not_ dup the storage 18:17:02 we should not use soft deletes 18:17:03 but the issue comes when a backup is copy from a different region (see my BP) 18:17:05 if ew can just pull it 18:17:12 robertmyers: ummmm i sure hope yer jokin ;) 18:17:18 hub_cap: no 18:17:24 delete it gone 18:17:34 lol ok robert-conway 18:17:44 esmute, has a use case where we must store the version 18:17:50 hey the user requested a delete honor it 18:18:00 :) 18:18:05 smh robertmyers ;) 18:18:05 considering the underlying datastore-version will be changing on the instance when migrations land, having the current datastore-version isn't useful in determining the datastore-version that the backup was spawned from. 18:18:32 amrith: Yes. when the backup is copied from a different region, there is no instances (active or deleted) that trove can query to find its datastore 18:18:34 i think thats valid amrith 18:18:45 nor its backuptype... but that is a different story 18:18:51 and add to that amcrn's clarification 18:19:07 seems like it is decided that we must store the datastore version; am I correct? 18:19:11 amcrn: at least we need to know the datastore name. 18:19:31 *know=store 18:19:31 esmute, ok make that datastore name and version 18:19:36 see my question/comment on the wiki; you need the datastore-version + more. 18:19:38 esmute: there won't be a backup in the new region either.. so how does a FKEY help you 18:19:56 amcrn: but if the instance changes version the old backup will still be the old version? 18:19:58 vipul: through the datastore version name 18:20:11 which you provide in the api esmute? 18:20:14 im hoping the datastore version name is unique :P 18:20:34 vipul: nop.. it will be stored in the swift metadata 18:21:01 robertmyers: the backup row in the table should always know the exact datastore-version from which is spawned, regardless of whether the instance has moved on. 18:21:17 which it was spawned* 18:21:18 amcrn: +1 18:21:25 amcrn: I agree 18:21:54 which is why I think we should add a row or two to the backup table 18:22:04 +1 robertmyers 18:22:19 robertmyers: dont you mean column? 18:22:23 robertmyers, "a row or two"? 18:22:26 if not the version, at least the datastore name.. 18:22:30 or three 18:22:37 columns, maybe 18:22:40 can someone address my point in the wiki about backup/restore strategy? 18:22:41 :-P 18:22:44 cp16net: +1 18:22:49 we should have a column table where we can insert rows 18:23:25 Yeah, I agree that we need to store this with the backup. Do we need to store the strategy as well? 18:23:30 amcrn: well, I wasn't planning on changing the restore with this BP 18:23:35 version uuid, ya? 18:23:38 thatll give u all u need 18:23:43 amcrn, if I understand your comment correctly, just name and version are insufficient; there is additional payload that is required. 18:23:45 this is just to store the data for future use 18:23:46 is that correct? 18:23:50 amrith: correct 18:23:55 FUTURE PROOF robertmyers 18:24:00 lol 18:24:27 hub_cap: version_uuid, probably yes. 18:24:32 robertmyers: +1. Restore wont be changed 18:24:40 yup SlickNik thats all u need 18:24:54 #agree 18:24:59 ok so yes to adding datastore to backup records? 18:25:15 i guess we're not going to address the fact that the strategy can change at any time, and you'll have backups blowing up. 18:25:16 #info just need to add the version_uuid to the backup table 18:25:36 amcrn: that seems like a new BP 18:25:54 let me tl;dr my point: store datastore-version-uuid + backup/restore strategy in 3 columns. 18:26:15 amcrn: okay 3 columns it is 18:26:24 eww i see yer point, but eww amcrn 18:26:30 make it four and you have a deal 18:26:47 amcrn, why do you need backup_strategy in the backup? shouldn't restore strategy suffice? 18:26:49 kevinconway: one row default="troll", sound good? 18:26:59 amcrn: so storing the dataversion-uuid and the type? 18:27:01 amcrn: wait what's the 3rd. I understand version_uuid, backup_strategy…and? 18:27:24 SlickNik: restore_strategy 18:27:27 hub_cap: we can put a hash index on it for super fast lookups 18:27:34 we already have the backup type 18:27:48 this is what i have so far 18:27:49 https://gist.github.com/anonymous/11151670 18:27:58 amcrn, why do you need backup_strategy in the backup? shouldn't restore strategy suffice? 18:28:12 robertmyers: Oh yeah, I thought we had something. 18:28:15 kevinconway: ;) 18:28:43 amrith: with a datastore with two competing backup_strategy's, you can't possibly know which one was used if the deployer has flipped between the two at any point in time 18:28:50 so the restore uses the backup type to look up the restore strategy 18:28:57 maybe the version should have a backup/restore strat 18:29:02 and we shouldnt allow a change inflight 18:29:09 aka once u set it in the version table, its done 18:29:18 for all of that version 18:29:21 do we have plans to offer multiple backup/restore strategies per dstore? 18:29:23 hub_cap: that's another way to do it 18:29:29 hub_cap: +1 i don't see changing a backup strategy is something you should do 18:29:35 well it might be 18:29:38 but not in the same version 18:29:42 hub_cap: That's too limiting- what if people want to backup a datastore in multiple ways? 18:29:49 kevinconway: I believe not. hence my question. 18:29:53 In case there's an improvement that comes around 18:29:57 grapex: who is people? 18:30:00 operators? 18:30:03 Yeah 18:30:07 today we use xtrabackup 18:30:15 maybe i want a mysqldump of my database to put it somewhere else and a xtrabackup all the other times 18:30:18 ok, i think this has been going on for a little too long now 18:30:19 what if super xtra backup gets released tomorrow and is incompatable 18:30:21 grapex: +1 18:30:22 Okay, so let's do this. We all agree on the version_uuid. 18:30:36 yes seems like the other thing is a spiral at present SlickNik 18:30:45 We can store the strategy in the swift metadata? So when the restore is being done, it reads this metadata and knows which strategy to use? 18:30:48 I can see a case where the strategy has been changed and you have an old backup with the prior strategy that you want to restore 18:30:53 amcrn, if a backup is with xtrabackup then there's only one way to restore it. similarly if it is a snapshot there's only one (different) way to restore it 18:30:57 And we need some more definition around the multi backup-restore strategy case. 18:30:59 juice: my point exactly 18:30:59 esmute: thats sane i think 18:31:03 esmute: that works too 18:31:11 so, I'd suspect that for a given backup, storing just the restore strategy should suffice. 18:31:21 the only requirement is to store the strategy along with the backup 18:31:31 whether that is in the database or the swift object does it matter? 18:31:34 so theoretically if a backup occurred with xtrabackup 2.1 .. it shoudl be restorable with xtrabackup 2.2 18:31:34 So let's approve this BP, and incrementally add the other case as part of another bp. 18:31:35 id also suspect that this really wont change a ton in general 18:31:36 so i think this convo has gone way outside the scope of the BP 18:31:41 are we saying that we can only use 2.1 restore? 18:31:47 perhaps in esmute's scenario where you have cross region backups/restore 18:31:50 juice: its not something we need to store, its redundant, and will eventually help clog our db 18:31:53 :) 18:32:02 the app only needs to know about it on restore 18:32:02 SlickNik: +1 18:32:10 I'm calling time on this one, for now. :) 18:32:14 kevinconway: ++ its a spiral 18:32:20 SlickNik: +1 18:32:23 we can approve the uuid only, the point was to dispell the rumor/idea that somehow it was sufficient 18:32:27 we need backup to store datasstore information... we can add the other stuff incrementally 18:32:28 for all use-cases 18:32:41 * hub_cap hands amcrn a wrench 18:33:01 you could always convert the version_uuid field to a json blob that contains all the other metadata later 18:33:03 esmute: storing it in the metadata seems like a logical approach 18:33:07 amcrn: I think we need to still discuss the other case. But later. :) 18:33:12 I want to hear about: 18:33:14 k 18:33:19 #topic Move the Trove Guest Agent to its own module 18:33:21 dont feed the kevinconway 18:33:36 #link https://wiki.openstack.org/wiki/Trove/MoveTroveGuest 18:33:42 lets do it 18:34:07 Given the impact to other bps/bugs, could it be fasttracked? 18:34:17 the idea here is a baby step before moving to a separate repo 18:34:35 I LIKE IT 18:34:46 way better than tryin to rip it out wholesale 18:34:47 robertmyers: will trove-common be part of the guest-agent...similar to openstack-common? 18:35:06 esmute: it will need some oslo stuff 18:35:22 but for now the plan was just to copy it in plave 18:35:25 place 18:35:49 once it is in a new repo it will have its own oslo and config 18:36:05 this sounds like a good idea 18:36:20 cool 18:37:00 robertmyers: this is a good approach +1 18:37:14 robertmyers: I'm totally on board with this. 18:37:20 +1 18:37:35 * cp16net pushes the easy button... 18:37:36 SlickNik: juice: cool 18:37:52 hahaha 18:37:54 Approved. All agree? 18:37:56 cp16net: :) 18:38:04 agreed: +1 18:38:16 guest agent finally gets its own nest 18:38:18 esp: i really did... 18:38:18 :-P 18:38:36 robertmyers: Any ideas for the timeline? Trying to address vgnbkr's comment about whether the change would be disruptive... 18:38:59 and whether we need to get this in asap. 18:39:04 Well, I can put the review up by the end of the week 18:39:17 what disruption do you expect? a copy/pasta into another dir shouldn't be too bad of a rebase 18:39:51 basically once this is approved (the reivew) it needs to be merged first 18:39:53 kevinconway: Eeeh, it will and it won't 18:40:21 I think we should prioritize moving it though 18:40:21 kevinconway: There's _always_ little things. 18:40:30 it will make reviewing guest agent pull requests much easier 18:40:34 and we will stop making stupid mistakes 18:40:37 robertmyers: done, go forth and do it. 18:40:46 SlickNik: ok 18:40:56 And thanks! :) 18:40:58 grapex: not so sure about your second point 18:41:11 #topic Database log files manipulations 18:41:22 denis_makagon? 18:41:24 this sounds sinister 18:41:41 kevinconway: The original title was "log file machinations" 18:41:42 going once :P 18:41:59 I'd +1 with that old title 18:42:17 SlickNik: question 18:42:28 the BP is set to "Needs Code Review", is that accurate? 18:42:38 or is the BP in need of review/approval? 18:42:49 amrith: I think that there was a gerrit patch submitted for it. 18:43:04 several as a matter of fact 18:43:06 Doug Shelley proposed a change to openstack/trove: Correct inconsistent state issues with config https://review.openstack.org/88591 18:43:09 amrith: and gerrit updates the bp status automatically based on the Commit Message. 18:43:18 ok, I get it 18:43:20 SlickNik: ? 18:43:27 LEt's move on. 18:43:29 did they impl that? cuz i used to do it manually 18:43:38 aka gerrit does bugs but it dindt used to do bps 18:44:35 hub_cap: The "code review" piece has worked for me, not the "Committed" piece. 18:44:45 k 18:44:50 But it's been flakey, not sure why. 18:45:01 Next one is denis' too. 18:45:09 So let's table that till he's around. 18:45:18 #topic Add backup and restore support for single instance couchbase 18:45:33 oh this is a terrible idea 18:45:38 kidddddding 18:45:41 SlickNik - i added this one. but i think forgot to put my name 18:45:47 hup_cap: :) 18:45:48 do we really neeed to discuss this? i mean, yes we need it, approved 18:45:51 SlickNik: hub_cap spoke. Lets move on :P 18:46:25 I'm good with this. 18:46:33 We need to do it. Approved. 18:46:35 before you approve it, does it make sense to ensure it is aligned with others that are already doing b&r 18:46:42 too late. 18:46:55 amrith: I think the review can handle that 18:47:10 robertmyers, code review? 18:47:14 amrith: yes 18:47:17 robertmyers: ++ 18:47:21 next! 18:47:27 its pertty straight forward :) 18:47:42 #topic Populate endpoint URLs from Keystone service catalog 18:47:52 esmute: your up. 18:47:58 you're* 18:48:02 well.. the point is simple.. To remove the openstack endpoints from the config and get these URL from keystone catalog 18:48:11 i have a concern with this one 18:48:23 esmute: +1 18:48:24 the BP says you are removing the config options 18:48:24 what is it kevinconway? 18:48:32 IIRC, matt lowery was trying to do something similar (and had a review for it at some point too, I think). 18:48:35 so this is backwards incompatible? 18:48:39 yea mat-lowery had done this 18:48:41 There's a patch set for this https://review.openstack.org/#/c/68015/ 18:48:45 :) 18:48:50 i think mat-lowery 's work was valid too 18:48:56 you can have the config there but it wont be read by trove 18:48:58 even w/ its order of how it retireved 18:49:04 *retrieved 18:49:10 ahh i didnt know about that patch mat-lowery 18:49:15 mat-lowery: correct me, but if there is a config it overrides the service catalot right? 18:49:29 correct...the default (in the patch) is to get from catalog 18:49:51 ok.. i can abandon my BP and use mat-lowery's patch 18:49:52 wait thats the opposite of what i said hehe 18:49:57 yes esmute 18:49:59 yea i think the default is get from conf 18:50:03 according to the patch 18:50:05 yes i thought so to vipul 18:50:13 i think it was originally the opposite but we whined 18:50:42 as long as we can still override it when needed I'm fine with it 18:50:43 yep, looks much better mat-lowery, sorry it's been waiting in the review state for so long :( 18:50:48 if *_url in conf, use it else use catalog 18:50:50 robertmyers: ditto 18:51:10 Here at Rax, especially in staging and other environments we don't always have the luxury of using the catalog. 18:51:27 yup 18:51:32 grapex: its more likely we can use the sears catalog than our service catalog in staging 18:51:38 hub_cap: LOL! 18:51:46 hub_cap: lol 18:51:57 mat-lowery: Looks good at first glance. Will review it later this afternoon. 18:52:07 ok thanks 18:52:22 so that's awesome that mat-lowery made the thing backwards compat 18:52:23 esmute: plz give some review to it as well, to make sure it meets your needs 18:52:29 but does core have an official stance on how we plan to deprecate config options on occasions like this? 18:52:34 will do hub_cap. Thanks 18:52:43 kevinconway: I don't see why we need to deprecate them 18:52:49 kevinconway: i think its an openstak thing, like a N+1 strategy 18:52:49 I think the service catalog is great, if it works 18:52:53 kevinconway: it does not deprecate them 18:52:55 but for this it wont deprecate 18:52:57 kevinconway: I think they're still supported. Just optional. 18:52:58 but there's plenty of times you may need to override it 18:53:05 SlickNik: that's deprecated 18:53:15 kevinconway: ? 18:53:16 eventually they would be removed, yes? 18:53:20 no thats optional 18:53:22 i hope not 18:53:24 like if you're trying to boot strap Trove in a vacuum, and maybe keystone or anything like it isn't set up 18:53:27 kevinconway: never 18:53:27 eventually removed is deprecated 18:53:27 deprecated doesn't mean gone 18:53:30 kevinconway: They're not going away. 18:53:31 optional is optional 18:53:39 ok so diamonds are forever? 18:53:44 some orgs will always use those optional configs 18:53:56 we will deprecate and remove non heat support for installs 18:53:58 kevinconway: Yes, just consider that we've gone with diamond mode 18:54:05 Time to move on I think. :) 18:54:14 #topic Enable specification of Cinder Volume Types 18:54:19 So this one is mine. 18:55:10 And it's an addition to the conf file to specify the volume_type to use when provisioning a cinder volume. 18:55:27 SlickNik : I get a message that your page is private for the wiki. 18:55:34 I'm still drafting this, so I just wanted to get feedback on what people thought. 18:55:37 I like this idea 18:55:55 it should probably become a capability though so we can pick them for each datastore type 18:55:57 vgnbkr: The wiki's still not there. So it's still drafting. 18:56:01 SlickNik: at some point we want to tie this to datastore version? 18:56:01 SlickNik: oh, nevermind - I didn't read the link, just clicked on it :-) 18:56:07 grapex: +1 18:56:26 grapex: Yes that's the reason it's still drafting. I'm thinking about whether we should add this to capabilities when we that's added. 18:56:55 yea we need ot fasttrack capabilities 18:57:04 since we have other things that "maybe one day will be a capability" 18:57:09 imsplitbit: Spare thee not the whip! 18:57:21 i think k-pom has a review up 18:57:25 so maybe imsplitbit needs to whip us ;) 18:57:29 Okay, I have the feedback I was looking for. 18:57:34 where there's a whip there's a way! 18:57:45 exactly kevinconway 18:57:51 hub_cap: hopefully not Peter Griffith style 18:57:54 kevinconway: lol 18:58:05 +1 on getting capabilities in soon. 18:58:15 I think that's all we have time for. 18:58:19 #endmeeting