14:11:49 <jokke_> #startmeeting glance
14:11:50 <openstack> Meeting started Thu Sep 25 14:11:49 2014 UTC and is due to finish in 60 minutes.  The chair is jokke_. Information about MeetBot at http://wiki.debian.org/MeetBot.
14:11:51 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
14:11:53 <openstack> The meeting name has been set to 'glance'
14:12:14 <jokke_> #topic Concurrency issue
14:12:20 <ativelkov> This is mine
14:12:27 <jokke_> shoot :)
14:12:31 <ativelkov> So, there is a problem in v2
14:12:43 <ativelkov> in image-update routine
14:13:04 <ativelkov> the API fetches the image, forms a domain object, passes it through all the layers and then to DB repository
14:13:25 <ativelkov> the repository serializes the updated object into a dict of keys and then issues an update query
14:13:44 <ativelkov> this leads to the update of ALL attributes and extra properties
14:13:51 <ativelkov> not only the modified ones
14:14:07 <ativelkov> which is very likely to cause race conditions in case of concurrent update of different attributes
14:14:33 <rosmaita> also, it makes it really difficult to track property updates
14:14:39 <ativelkov> yes
14:14:46 <ativelkov> and also hits the performance
14:14:53 <rosmaita> if you need to know about a particular property, like for auditing or billing reasons
14:14:57 <ativelkov> A quick (and a bit dirty) fix is to introduce "optimistic lock" using versioning
14:15:24 <ativelkov> i.e. update the object only if timestamp of the fetched object matches the timestamp of the object in DB
14:15:37 <ativelkov> This is the PS which does it: https://review.openstack.org/#/c/122814/
14:15:55 <ativelkov> however, it just solves the race, but does not help to track property values
14:16:06 <ativelkov> and updated_at is a datetime field in DB
14:16:29 <ativelkov> which often has precision of seconds
14:16:54 <zhiyan> ativelkov: i think the approach which using update_at is nice idea, imo. but iirc, few code of update logic on image entity didn't update update_at field
14:17:11 <ativelkov> So, in some (quite rare, TBH) circumstances the races still may occur
14:17:36 <ativelkov> zhiyan: where? _image_update in sqlalchemy spi.py does it always
14:17:50 <jokke_> ativelkov: sounds like that does not solve the issue when you're trying to update multiple things about same time on different requests
14:18:03 <ativelkov> jokke_: right
14:18:39 <ativelkov> we could introduce an extra field with better precision (microsecond-timestamps or even uuids) to solve this
14:19:13 <ativelkov> hovever, a better approach would be to actually track the changes and update only the necessary properties
14:19:40 <ativelkov> (which could be combined with timestamps, btw - just to add extra proptection in case of concurrent updates of the same properties)
14:19:58 <ativelkov> I've started doing this: https://review.openstack.org/#/c/123722/
14:20:15 <ativelkov> It breaks some tests though and I am working to fix them right now
14:20:24 <zhiyan> ativelkov: does this kind of race issue only affect image but others, like task?
14:21:03 <ativelkov> zhiyan: Need to check it, I have this in my backlog but didn't get there yet. It depends on how the update logic is implemented
14:21:37 <rosmaita> zhiyan: tasks don't have an update call exposed in API
14:21:47 <ativelkov> #action ativelkov to investigate if race condition affects other types od DB repositories (tasks etc)
14:22:33 <zhiyan> nice
14:22:35 <jokke_> ativelkov: I really like the later solution. Specifically for the exact reason this is issue on the first place ... or combination with more acurate timestamp
14:22:47 <lakshmiS> I guess most of the update api's save all the fields right now.
14:22:59 <ativelkov> So, my suggestion is to apply timestamp-based fix (with current datetime-based precision) + the property-tracking solution (when it passes the checks)
14:23:20 <zhiyan> rosmaita: i see, i does now, but i think this issue from db operator level.
14:23:21 <ativelkov> then we may think about increasing accuracy of the timestamp
14:23:51 <zhiyan> ativelkov: do you think we could using a versioning based approach?
14:23:51 <ativelkov> updated_at is a common field for all openstack projects, and it is always a DATETIME
14:24:39 <ativelkov> zhiyan: yes, it is always good to have optimistic concurrency checks, but is they are made based on timestamps, then the timestamps have to have good precision
14:24:47 <zhiyan> ativelkov: since i'm not sure the time precision of db datetime field is always fit our requirement, iiuc.
14:24:55 <jokke_> zhiyan: is there reason to use artificial version versus natural version (aka timestamp)?
14:25:13 <jokke_> ah
14:25:15 <boris-42> zhiyan ping
14:25:22 <ativelkov> we may add extra field (say, TS) with microsecond precision
14:25:33 <jokke_> that datetime field explains
14:26:18 <zhiyan> ativelkov: i'm a little worried due to not sure if precision of particular column type is related with particular database
14:26:33 <ativelkov> zhiyan: it is related indeed. Even with the version of it
14:26:55 <ativelkov> mysql added second fractions since 5.6.4
14:26:56 <zhiyan> but, imo, an int based (just for example) versioning field will work well for any db
14:27:10 <jokke_> ativelkov: do we actually need that time based optimistic locking if we/you do the tracking change there?
14:27:15 <zhiyan> 'second' level is too low, btw
14:27:30 <mfedosin> btw, mysql 5.6 supports millis precision, but 5.5 doesn't
14:27:40 <zhiyan> mfedosin: thanks. so..
14:27:53 <ativelkov> zhiyan: right. However this will require us to add a new field which has the same semantics as the existing updated_at
14:28:14 <ativelkov> jokke_: it may still be a good idea to make sure that there are no concurrent updates on the same property
14:28:41 <zhiyan> imo, versioning field seems better, frankly
14:28:44 <ativelkov> in case of locking one of the competing request will get Conflict response
14:29:03 <zhiyan> db version/capability un-related
14:29:07 <nikhil_k> guys, we have just half an hour to go and many items to cover from agenda
14:29:12 <nikhil_k> fyi
14:29:18 <ativelkov> should we move this to mailing list?
14:29:28 <zhiyan> ativelkov: sounds good
14:29:31 <ativelkov> I may summarise all the stuff for further discussion
14:29:39 <jokke_> ativelkov: my concern is that we do something like that and decide we're locking, but that will be only on coarse time accuracy ... kind of feels that we would be better off without it that trusting something we know we can't trust
14:29:53 <jokke_> zhiyan: I agree
14:30:16 <jokke_> ok
14:30:21 <ativelkov> ok, let's move on
14:30:25 <ativelkov> I'll write to ML
14:30:32 <jokke_> #topic Documentation patches
14:30:46 <ativelkov> #action ativelkov to write to ML about concurrency issue
14:30:48 <zhiyan> btw, boris-42 has a idea for rally demo
14:31:04 <lakshmiS> This patches are continuing from last week meeting. We just need one more +2 on all of them
14:31:51 <lakshmiS> zhiyan has given his +2 on it. we need one more volunteer :)
14:32:02 <jokke_> lakshmiS: thanks, is there something specific you want to flag out of them?
14:32:03 <zhiyan> lakshmiS: thanks for working on those
14:32:13 <gokrokve> zhiyan: Do you know what is in scope of this demo
14:32:21 <zhiyan> lakshmiS: do you think we need to put them into juno release?
14:32:35 <zhiyan> boris-42: ^ pls tell us
14:32:40 <boris-42> zhiyan so
14:32:43 <jokke_> #action Core reviews needed for metadef patches listed in https://etherpad.openstack.org/p/glance-team-meeting-agenda
14:32:49 <lakshmiS> The first set of patches are related to metadata properties which should be easy for anyone to review
14:33:06 <boris-42> zhiyan guys if you would like I am ready almost always
14:33:16 <boris-42> it's just demo for about 40 minutes
14:33:23 <zhiyan> lakshmiS: i'm not sure if we really need this stuff for juno, due to currently imo we'd better focus on bug fix?
14:33:33 <lakshmiS> zhiyan: yes we need those juno since the definitions are related to juno release
14:33:41 <boris-42> zhiyan so could you help to pick dates
14:34:00 <lakshmiS> zhiyan: There are also bug fixes in the next set
14:34:15 <zhiyan> boris-42: sorry, just hold on pls.
14:34:53 <jokke_> lakshmiS: are these the corrections that was left to be done to get the big patch in before FF?
14:34:57 <zhiyan> lakshmiS: do we have a etherpad which could list all patches those metadef needs in juno?
14:34:59 <lakshmiS> Bug fixes based on previous review comments: https://review.openstack.org/#/c/120957/  https://review.openstack.org/#/c/121836/  https://review.openstack.org/#/c/120334/ https://review.openstack.org/#/c/120355/
14:35:18 <lakshmiS> Its added in the meeting agenda
14:35:19 <lakshmiS> https://etherpad.openstack.org/p/glance-team-meeting-agenda
14:35:20 <zhiyan> cool. lakshmiS that's the full list right?
14:35:38 <lakshmiS> zhiyan: Meeting agenda has the full list
14:35:44 <zhiyan> nice
14:36:24 <jokke_> #topic community image spec
14:36:25 <zhiyan> lakshmiS: frankly i'm a little worried if it's a right time to merge so much patches in rc stage..
14:36:40 <jokke_> oh sorry ... I was too hasty
14:37:23 <zhiyan> nikhil_k: jokke_ do you think we could finish them in rc2?
14:37:46 <lakshmiS> zhiyan: I understand. Please see whichever can. Thats why we put the data related patches first in the list since those are the least to affect any functionality
14:37:53 <jokke_> I haven't looked into them too closely
14:38:21 <jokke_> lakshmiS: are those patches all priority and by your view at their last form?
14:38:24 <nikhil_k> if we can have an etherpad with the state of the patches and which ones need +2s
14:38:37 <nikhil_k> do not mean the agenda etherpad
14:38:57 <zhiyan> nikhil_k: yep, that was my idea in above msg
14:39:11 <lakshmiS> jokke_: nikhil_k: I will add them on another etherpad and send the link before meeting ends now
14:39:13 <nikhil_k> cool
14:39:21 <nikhil_k> sounds good, thanks lakshmiS !
14:39:40 <jokke_> #action lakshmiS will make etherpad with patch priority and current state for the metadef patches pending
14:39:53 <wayne___> The metadef bugs listed in the etherpad are the ones that need one more +2 and are in their final form
14:40:10 <jokke_> thanks wayne___
14:40:19 <wayne___> Unless the second core is not in agreement of course
14:40:26 <jokke_> ;)
14:40:34 <wayne___> :)
14:40:48 <jokke_> ok then back to topic ... community image spec
14:41:05 <kragniz> so there's a spec up here: https://review.openstack.org/#/c/124050/1/specs/kilo/community-level-v2-image-sharing.rst
14:41:19 <wayne___> There are two other metadef patch sets that have not gotten any reviews at all yet
14:41:27 <wayne___> and are not in the etherpad list
14:41:46 <kragniz> we've had some discussion about community images simply being a special case of shared images
14:41:58 <kragniz> and the spec reflects that
14:42:01 <lakshmiS> wayne__: Add it to this etherpad : https://etherpad.openstack.org/p/glance-rc1-patches
14:42:29 <kragniz> it would be cool if people could have a glance over the spec and point out any problems with it
14:42:36 <wayne___> :lakshmiS ok
14:42:58 <kragniz> the main question I have is how glanceclient will work with this
14:43:00 * ativelkov is going to check the spec, thanks
14:43:18 <jokke_> kragniz: do you want to run quick summary what you were planning?
14:43:31 <kragniz> in the spec I wrote $ glance image-update --visibility community <IMAGE>
14:43:39 <kragniz> would make an image a community image
14:43:47 <kragniz> any thoughts on that?
14:43:55 <jokke_> kragniz: so people have an idea of the approach. I don't think we have time for everyone read it through and comment within the slot we have for the meeting
14:44:01 <ativelkov> LGTM
14:44:12 <kragniz> jokke_: okay
14:45:00 <zhiyan> kragniz: then membership doesn't need to accept but will made automatically right?
14:45:19 <jokke_> #link https://review.openstack.org/#/c/124050
14:45:34 <kragniz> zhiyan: membership would work in the same way that shared images currently work
14:46:00 <kragniz> zhiyan: the difference being the owner 'shares' it with everyone
14:46:23 <zhiyan> will check the spec, but iirc, base on our original talking, the membership will be made automatically without 'accept'.
14:47:09 <rosmaita> kragniz: i thought no membership was required for community images
14:47:28 <rosmaita> just for if the consumer wants it to show up in the image-list
14:47:44 <ativelkov> on mini-summit we've agreed that the users will have to accept the membership of the community image
14:47:56 <jokke_> yes we did
14:47:57 <rosmaita> ativelkov: only for display, not for usage
14:48:15 <zhiyan> ativelkov: ok
14:48:17 <ativelkov> yes, sure - same as with regular membership now
14:48:19 <kragniz> rosmaita: yes, no membership is required just to use the image
14:48:27 <rosmaita> for community?
14:48:33 <jokke_> kragniz: would you think it being easy to extend this approach so that users could auto accept community images from certain providers
14:48:36 <rosmaita> then why bother having it?
14:49:14 <kragniz> rosmaita: the provider of the image doesn't have to add each user who wants to use the image
14:49:28 <jokke_> rosmaita: iiuc the point is to lower the burden for community image providers maintaining the member lists
14:49:41 <jokke_> ^^ :)
14:49:41 <zhiyan> ok, seems we need more discussion on this, i think we could do that in spec after re-view and thinking?
14:49:47 <kragniz> jokke_: yes, I think so
14:49:53 <kragniz> zhiyan: sounds good
14:49:55 <jokke_> zhiyan: sounds good
14:50:04 <kragniz> zhiyan: I just wanted to bring it up here
14:50:18 <jokke_> thanks kragniz
14:50:26 <zhiyan> kragniz: i see, thanks raise it up!
14:50:26 <jokke_> #topic open discussion
14:50:30 <jcook> /win 2
14:50:33 <zhiyan> boris-42: ^^
14:50:42 <jokke_> #link https://etherpad.openstack.org/p/glance-rc1-patches#
14:50:52 <jokke_> #link https://etherpad.openstack.org/p/glance-rc1-patches
14:51:20 <zhiyan> boris-42: pls tell us what's you idea for rally demo?
14:52:23 <jokke_> if not I'd like to raise the logging change topic again ... there was some discussion on the mailing list
14:52:37 <abhishekk> https://bugs.launchpad.net/glance/+bug/1372888
14:52:38 <uvirtbot> Launchpad bug 1372888 in glance "g-api raise 500 error if filesystem_store_datadirs and filesystem_store_datadir both specified" [Medium,Confirmed]
14:52:53 <zhiyan> boris-42: i could help pick a time for the demo, will check with other members who interested in this.
14:53:08 <jokke_> how people want us to approach that. Should we drop all user related to debug?
14:53:37 <abhishekk> is 410 resonse ok for "g-api raise 500 error if filesystem_store_datadirs and filesystem_store_datadir both specified"
14:54:00 <nikhil_k> seems like we have 2 conversations going on here..
14:54:22 <abhishekk> nikhil_k:sorry, i will step back
14:55:23 <jokke_> abhishekk: if that currently breaks the server I think we should not let it even start with that configuration#
14:56:05 <zhiyan> abhishekk: seems it's a worth point for us, i prefer to address in juno release
14:56:09 <zhiyan> will take a look
14:56:32 <abhishekk> jokke_, zhiyan:thank you
14:56:57 <jokke_> regarding the logging I'd like to get the reviews for https://review.openstack.org/#/c/116626/ and https://review.openstack.org/#/c/117204/ so we could land them in J
14:57:24 <jokke_> in my point of view they're ready to land and the first one I have some support for that point ;)
14:58:16 <zhiyan> jokke_: all user related stuff going to debug level?
14:59:03 <jokke_> zhiyan: that seems to be what Sean and Jay wan'ts ... personally I disagree strongly but no other opinions were raised on the discussions#
14:59:29 <zhiyan> ok
14:59:53 <jokke_> ok, we're out of time. Thanks everyone!
14:59:55 <zhiyan> ok, seems we have a lot patches want to fit into juno in rc stage
14:59:59 <zhiyan> heh
15:00:02 <jokke_> #endmeeting