| *** threestrands has joined #openstack-glance | 00:38 | |
| *** Liang__ has joined #openstack-glance | 00:58 | |
| *** mvkr has joined #openstack-glance | 01:15 | |
| *** threestrands has quit IRC | 01:43 | |
| *** awalende has joined #openstack-glance | 02:45 | |
| *** awalende has quit IRC | 02:49 | |
| *** threestrands has joined #openstack-glance | 03:10 | |
| *** threestrands has quit IRC | 03:11 | |
| *** threestrands has joined #openstack-glance | 03:11 | |
| *** davee__ has joined #openstack-glance | 04:03 | |
| *** davee_ has quit IRC | 04:05 | |
| *** davee___ has quit IRC | 04:05 | |
| *** davee_ has joined #openstack-glance | 04:06 | |
| *** bhagyashris has joined #openstack-glance | 04:11 | |
| *** bhagyashris has quit IRC | 04:42 | |
| *** bhagyashris has joined #openstack-glance | 05:04 | |
| *** awalende has joined #openstack-glance | 05:18 | |
| *** awalende has quit IRC | 05:22 | |
| *** evrardjp has quit IRC | 05:33 | |
| *** evrardjp has joined #openstack-glance | 05:34 | |
| *** threestrands has quit IRC | 05:38 | |
| *** ratailor has joined #openstack-glance | 05:58 | |
| *** udesale has joined #openstack-glance | 06:09 | |
| *** udesale is now known as udesale_ | 06:28 | |
| *** udesale_ is now known as udesale | 06:29 | |
| *** Luzi has joined #openstack-glance | 06:35 | |
| *** ratailor_ has joined #openstack-glance | 06:41 | |
| *** ratailor has quit IRC | 06:42 | |
| *** ratailor__ has joined #openstack-glance | 06:46 | |
| *** ratailor_ has quit IRC | 06:49 | |
| *** awalende has joined #openstack-glance | 07:01 | |
| *** jawad-axd has joined #openstack-glance | 07:02 | |
| *** awalende_ has joined #openstack-glance | 07:03 | |
| *** awalende has quit IRC | 07:05 | |
| *** jawad-axd has quit IRC | 07:06 | |
| *** jawad-axd has joined #openstack-glance | 07:07 | |
| *** jawad-ax_ has joined #openstack-glance | 07:10 | |
| *** jawad-axd has quit IRC | 07:12 | |
| *** rcernin has quit IRC | 07:13 | |
| *** brinzhang has joined #openstack-glance | 07:24 | |
| *** brinzhang_ has joined #openstack-glance | 07:36 | |
| *** brinzhang has quit IRC | 07:39 | |
| *** brinzhang has joined #openstack-glance | 07:48 | |
| *** brinzhang_ has quit IRC | 07:51 | |
| *** bhagyashris has quit IRC | 07:58 | |
| *** trident has joined #openstack-glance | 08:02 | |
| *** brinzhang_ has joined #openstack-glance | 08:10 | |
| *** brinzhang has quit IRC | 08:13 | |
| *** tesseract has joined #openstack-glance | 08:21 | |
| *** tosky has joined #openstack-glance | 08:22 | |
| *** bhagyashris has joined #openstack-glance | 09:05 | |
| *** brinzhang_ has quit IRC | 09:32 | |
| *** Liang__ is now known as Liangfang | 09:33 | |
| *** Liangfang is now known as LiangFang | 09:33 | |
| *** brinzhang_ has joined #openstack-glance | 09:33 | |
| *** brinzhang has joined #openstack-glance | 09:41 | |
| *** brinzhang_ has quit IRC | 09:45 | |
| *** pcaruana has joined #openstack-glance | 09:49 | |
| *** brinzhang_ has joined #openstack-glance | 09:51 | |
| *** brinzhang has quit IRC | 09:55 | |
| *** pcaruana has quit IRC | 09:56 | |
| *** ratailor has joined #openstack-glance | 09:59 | |
| *** LiangFang has quit IRC | 09:59 | |
| *** ratailor__ has quit IRC | 10:01 | |
| *** brinzhang has joined #openstack-glance | 10:02 | |
| *** brinzhang_ has quit IRC | 10:06 | |
| *** jawad-ax_ has quit IRC | 10:07 | |
| *** ratailor_ has joined #openstack-glance | 10:17 | |
| *** ratailor has quit IRC | 10:19 | |
| *** brinzhang_ has joined #openstack-glance | 10:21 | |
| *** brinzhang has quit IRC | 10:24 | |
| *** brinzhang has joined #openstack-glance | 10:30 | |
| *** brinzhang_ has quit IRC | 10:33 | |
| *** brinzhang_ has joined #openstack-glance | 10:40 | |
| *** brinzhang has quit IRC | 10:43 | |
| *** jawad-axd has joined #openstack-glance | 10:49 | |
| *** jawad-axd has quit IRC | 10:53 | |
| *** rcernin has joined #openstack-glance | 11:11 | |
| *** jawad-axd has joined #openstack-glance | 11:14 | |
| *** brinzhang has joined #openstack-glance | 11:30 | |
| *** brinzhang_ has quit IRC | 11:34 | |
| *** brinzhang_ has joined #openstack-glance | 11:39 | |
| *** brinzhang has quit IRC | 11:42 | |
| *** bhagyashris has quit IRC | 11:50 | |
| *** brinzhang_ has quit IRC | 11:52 | |
| *** brinzhang_ has joined #openstack-glance | 11:52 | |
| *** brinzhang has joined #openstack-glance | 11:54 | |
| *** brinzhang_ has quit IRC | 11:58 | |
| *** bhagyashris has joined #openstack-glance | 12:02 | |
| *** brinzhang has quit IRC | 12:06 | |
| *** brinzhang has joined #openstack-glance | 12:07 | |
| *** zzzeek has quit IRC | 12:16 | |
| *** zzzeek has joined #openstack-glance | 12:17 | |
| *** udesale has quit IRC | 12:23 | |
| *** abhishekk has quit IRC | 12:23 | |
| *** abhishekk has joined #openstack-glance | 12:24 | |
| *** brinzhang_ has joined #openstack-glance | 12:38 | |
| *** brinzhang_ has quit IRC | 12:40 | |
| *** brinzhang_ has joined #openstack-glance | 12:41 | |
| *** brinzhang has quit IRC | 12:42 | |
| *** brinzhang_ has quit IRC | 12:42 | |
| *** brinzhang_ has joined #openstack-glance | 12:43 | |
| *** brinzhang_ has quit IRC | 12:44 | |
| *** brinzhang has joined #openstack-glance | 12:45 | |
| *** brinzhang has quit IRC | 12:46 | |
| *** brinzhang has joined #openstack-glance | 12:47 | |
| *** brinzhang_ has joined #openstack-glance | 13:20 | |
| *** brinzhang_ has quit IRC | 13:21 | |
| *** brinzhang_ has joined #openstack-glance | 13:21 | |
| *** brinzhang_ has quit IRC | 13:22 | |
| *** brinzhang_ has joined #openstack-glance | 13:23 | |
| *** brinzhang has quit IRC | 13:23 | |
| *** brinzhang_ has quit IRC | 13:24 | |
| *** brinzhang_ has joined #openstack-glance | 13:24 | |
| *** ratailor_ has quit IRC | 13:35 | |
| *** brinzhang has joined #openstack-glance | 13:37 | |
| *** brinzhang_ has quit IRC | 13:40 | |
| *** brinzhang_ has joined #openstack-glance | 13:42 | |
| *** brinzhang has quit IRC | 13:45 | |
| *** rcernin has quit IRC | 13:53 | |
| *** brinzhang has joined #openstack-glance | 14:01 | |
| *** brinzhang has quit IRC | 14:03 | |
| *** brinzhang has joined #openstack-glance | 14:03 | |
| *** brinzhang_ has quit IRC | 14:04 | |
| *** Luzi has quit IRC | 14:12 | |
| *** zaneb has joined #openstack-glance | 14:22 | |
| *** brinzhang_ has joined #openstack-glance | 14:31 | |
| *** brinzhang has quit IRC | 14:34 | |
| *** pcaruana has joined #openstack-glance | 14:34 | |
| *** brinzhang has joined #openstack-glance | 14:37 | |
| *** brinzhang_ has quit IRC | 14:40 | |
| *** bhagyashris has quit IRC | 14:51 | |
| *** lpetrut has joined #openstack-glance | 15:12 | |
| *** awalende_ has quit IRC | 15:22 | |
| *** awalende has joined #openstack-glance | 15:23 | |
| *** awalende_ has joined #openstack-glance | 15:27 | |
| *** jawad-axd has quit IRC | 15:27 | |
| *** awalende has quit IRC | 15:27 | |
| *** awalende_ has quit IRC | 15:31 | |
| *** pcaruana has quit IRC | 16:12 | |
| *** gyee has joined #openstack-glance | 16:15 | |
| *** jmlowe has quit IRC | 16:51 | |
| *** jmlowe has joined #openstack-glance | 16:52 | |
| *** pcaruana has joined #openstack-glance | 17:13 | |
| *** nicolasbock has joined #openstack-glance | 17:20 | |
| *** jawad-axd has joined #openstack-glance | 17:33 | |
| *** evrardjp has quit IRC | 17:33 | |
| *** evrardjp has joined #openstack-glance | 17:34 | |
| *** lpetrut has quit IRC | 17:54 | |
| *** jawad-axd has quit IRC | 18:04 | |
| *** lpetrut has joined #openstack-glance | 18:17 | |
| *** lpetrut has quit IRC | 18:28 | |
| *** jawad-axd has joined #openstack-glance | 18:45 | |
| *** tesseract has quit IRC | 18:53 | |
| *** jawad-axd has quit IRC | 19:00 | |
| *** awalende has joined #openstack-glance | 19:23 | |
| *** awalende has quit IRC | 19:28 | |
| jokke_ | rosmaita: around? | 21:42 |
|---|---|---|
| rosmaita | hello | 21:42 |
| jokke_ | rosmaita: hey, thanks for the review. I was writing response for it and started circulating. I realized I still don't understand what/why you are trying to achieve. | 21:44 |
| jokke_ | I had one mistake there, so there is not even HTTPInvalid, the invalid is translated few lines down to a BadRequest (which indeed probably isn't the best response for that) | 21:45 |
| rosmaita | ok, we should all take a look at the response codes again on that | 21:46 |
| jokke_ | Which is irrelevant as long as we're not in agreement of how we adress the horrible behaviour underneath it. | 21:47 |
| rosmaita | well, i think there are 2 things we want to accomplish | 21:47 |
| rosmaita | (1) avoid the race condition on our side that would re-insert the location that is being deleted | 21:48 |
| rosmaita | (which your patch does) | 21:48 |
| rosmaita | and | 21:48 |
| rosmaita | (2) have the API behave well | 21:48 |
| rosmaita | which i think your patch is not doing, because a user could get a 204 for two separate delete requests | 21:49 |
| rosmaita | catching the glance_store.NotFound in location.pop() just feels icky to me, but I guess i could go along with it if everyone else agrees it's ok | 21:50 |
| rosmaita | as long as we can address (2) also | 21:50 |
| jokke_ | I think what you were proposing is not the greatest API behaviour either where we claim the asset doesn't exists but yet subsequent request might clame it does again | 21:51 |
| rosmaita | yeah, but i think that's a bigger issue -- we're going to have that if someone does DELETE v2/stores/sid/iid and GET v2/images/iid almost at the same time | 21:53 |
| jokke_ | we could address that by passing the image save function to the location code and call it (saving the database state before trying to delete the resource from store) and if store fails we could put it back there like the location failur does now. This would just be horrible mess and really difficult logic to follow | 21:53 |
| rosmaita | yeah, so i think we should not do that | 21:54 |
| jokke_ | and all that just because of our horrible onion proxy system that is causing us to not to keep our db up to date | 21:55 |
| rosmaita | right, so i don't think we can solve that problem for all affected calls ATM ... but i do think it is weird bahavior for the API to return 204 for multiple calls of the same DELETE request | 21:57 |
| rosmaita | that's all i'm saying | 21:57 |
| jokke_ | One thing we could do is to raise store.exception.NotFound from the location (without re-adding the location back to the list) and return 410 Gone from the API call (after saving the deletion from the metadata) | 22:00 |
| *** vesper has joined #openstack-glance | 22:00 | |
| jokke_ | I just think it feels even weirder to return 4xx and yet modify the resource | 22:01 |
| *** vesper11 has quit IRC | 22:01 | |
| rosmaita | well, we wouldn't be modifying the resource in the thread that's returning the 4xx | 22:01 |
| rosmaita | that would be handled by the "other" thread | 22:02 |
| jokke_ | we would | 22:02 |
| jokke_ | no, we would be removing still the location metadata | 22:02 |
| rosmaita | no, the location.pop() puts it back, but as long as we don't call image.save(), we're not modifying anything | 22:03 |
| jokke_ | so one issuing DELETE to the stores endpoint where the data is not in the backend would get rid of the location record but receive 410 instead of 204 as the resource state was not in normal state | 22:03 |
| jokke_ | but that doesn't change the fact that we cannot clean up the stores ever if the data is missing. We _must_ fix the location code for that | 22:04 |
| jokke_ | and all the GET calls would be still returning the store in the image stores property | 22:04 |
| jokke_ | what I'm proposing is that we do save the image, but for your peace of mind we could return 410 instead of 204 in the case the data is not there | 22:05 |
| jokke_ | and 410 would be perfectly reasonable response for that, looking the description in w3.org | 22:06 |
| rosmaita | yes, i agree about that | 22:06 |
| rosmaita | although | 22:06 |
| jokke_ | 10.4.11 410 Gone | 22:06 |
| jokke_ | The requested resource is no longer available at the server and no forwarding address is known. This condition is expected to be considered permanent. | 22:07 |
| rosmaita | my concern is that we are absolutely positive that for all drivers, a glance_store.NotFound will only happen when the data doesn't exist -- couldn't be an ephemeral response during cluster rebalancing or something? | 22:07 |
| rosmaita | i mean "are we absolutely positive" | 22:07 |
| jokke_ | and I don't think there is anything in the HTTP spec that states there must be no changes in the resource when 4xx i returned | 22:07 |
| rosmaita | cdent was my go-to guy for that kind of question, but i believe he has burned out on openstack and is doing something else now | 22:09 |
| rosmaita | (if you can imagine that) | 22:09 |
| jokke_ | naah, not possible ;( | 22:09 |
| jokke_ | It might be worth of checking (and fixing) but there is one corner case that comes to my mind now when you mentioned (and if we do that, it's really really shitty from us) | 22:11 |
| jokke_ | Do we return NotFound from store if we can't reach the store endpoint or do we return something sane? | 22:12 |
| jokke_ | say if the swift or ceph is offline | 22:12 |
| rosmaita | i think 404 is ok, but def not 410 | 22:13 |
| rosmaita | or we can return some kind of 500 | 22:13 |
| rosmaita | since it's a server-side problem | 22:13 |
| jokke_ | No I mean currently | 22:13 |
| jokke_ | not what we would do if we were writing this now, and I mean glance_store.exception.NotFound not HTTPNotFound | 22:14 |
| rosmaita | gotcha | 22:14 |
| rosmaita | i do not know, but hope it is a 5xx | 22:14 |
| jokke_ | glance_store does not return http exceptions at all | 22:14 |
| jokke_ | we just have overloaded some of the names | 22:14 |
| jokke_ | but my point is I hope we raise something sane to distinquish it rather thn the same error we would do when the data is just missing | 22:15 |
| jokke_ | back in 5min | 22:15 |
| rosmaita | right, and that's why i am worried about whether all drivers return g_s.NotFound under the same circumstances | 22:15 |
| rosmaita | ok, i will be here for 45 more min | 22:16 |
| rosmaita | jokke_: from looking at glance_store.exceptions, we should get something like this when the store is not available: | 22:22 |
| rosmaita | class RemoteServiceUnavailable(GlanceStoreException): | 22:22 |
| rosmaita | message = _("Remote server where the image is present is unavailable.") | 22:22 |
| rosmaita | though if i am grepping correctly, it's only raised in one place: _drivers/http.py:220 | 22:25 |
| jokke_ | ok, so that's probably something we should look into addressing then :( | 22:30 |
| jokke_ | the code calling it can obviously handle that exception so we just need to make sure we return that or something sane rather than exception.NotFound when the store is not reachable | 22:31 |
| rosmaita | right | 22:31 |
| jokke_ | I knew I was missing something from your thoughts that just didn't translate from the review :D | 22:32 |
| jokke_ | thanks for taking the time with me to figure this out | 22:32 |
| rosmaita | yeah, sorry about that | 22:34 |
| rosmaita | looks like your code would translate a glance_store.RemoteServiceUnavailable into a HTTPInternalServerError, so that's cool | 22:34 |
| jokke_ | So if I make sure our stores don't return exception.NotFound from backends we can't reach would that 410 Gone approach sound reasonable for you? | 22:35 |
| *** brinzhang_ has joined #openstack-glance | 22:35 | |
| jokke_ | that way we would be eliminating the race condition and able to recover from failed deletes or store corruption without huge amount of refactoring or admin intervention | 22:37 |
| rosmaita | give me a min to scroll back to that | 22:37 |
| *** brinzhang has quit IRC | 22:39 | |
| jokke_ | Let me re-phrase that for you: | 22:40 |
| rosmaita | yes, please | 22:40 |
| rosmaita | :) | 22:40 |
| jokke_ | If store returns NotFound (that being second delete call while the first one is still doing the actual delete or longer term situation where we don't have the data in backend) we keep the location code as it's propoed in current patch | 22:41 |
| jokke_ | but instead of returning 204 we save the image and return 410 Gone | 22:42 |
| jokke_ | and I'd fix the BadRequest from the response of the actual location not being in the image metadata | 22:43 |
| jokke_ | that would address us ever returning more than once 204 from the DELETE calls and us having conflicting responses at latest after the second DELETE | 22:44 |
| jokke_ | and allow user to clear the locations by issuing the DELETE call if the data is not available due corruption, earlier crash during delete etc. | 22:45 |
| rosmaita | but if we keep the location code same as proposed on the current patch, where it eats the NotFound, we can't tell in images.py that we should raise a 410 ? | 22:48 |
| *** tkajinam has joined #openstack-glance | 22:49 | |
| jokke_ | oh sorry, yes instead of returning the location, we will reraise the NotFound, just not inject the location back to the list | 22:49 |
| jokke_ | so we keep the special case in location.py for the NotFound | 22:49 |
| jokke_ | and we revert on any other exception coming from store | 22:50 |
| rosmaita | gotcha | 22:50 |
| jokke_ | and if we get NotFound from location.pop() we save the image and return 410 Gone | 22:51 |
| jokke_ | That will make that we ever get only one 204 and only once 410 | 22:52 |
| jokke_ | and after that it behaves like the location does not exist | 22:52 |
| jokke_ | even if the delete call is still going that will eventually return the 204 | 22:53 |
| rosmaita | ok, and it also addresses your concern that we need a way to delete-from-store when the image is showing that data exists in a store, but the data has actually been deleted from the backend | 22:53 |
| rosmaita | i think that sounds ok | 22:53 |
| jokke_ | exactly | 22:53 |
| rosmaita | i believe i can live with that | 22:54 |
| rosmaita | yeah, it sounds good | 22:54 |
| jokke_ | so timeline this would be possible: 1) DELETE 2) DELETE 3) 410 Gone 4) GET (returns without the store in the list) 5) 204 from first delete | 22:54 |
| jokke_ | or the same thing without 1) and 5) | 22:55 |
| jokke_ | in that data missing case | 22:55 |
| jokke_ | but 1) DELETE 2) DELETE 3) DELETE where 1) takes long time would return 4) 410 Gone 5) 404 NotFound 6) 204 | 22:57 |
| *** rcernin has joined #openstack-glance | 22:57 | |
| jokke_ | great, I'll get rev up tomorrow and we kept within that 45min what you had :D | 22:58 |
| rosmaita | you da man! | 22:59 |
| rosmaita | thanks for working this out | 22:59 |
| jokke_ | thank you for pointing the underlying problems and bearing with me :D | 22:59 |
| jokke_ | now, dinner time! TTYL | 23:00 |
| *** tosky has quit IRC | 23:06 | |
| *** awalende has joined #openstack-glance | 23:19 | |
| *** awalende has quit IRC | 23:23 | |
| *** Liang__ has joined #openstack-glance | 23:25 | |
| *** Liang__ has quit IRC | 23:59 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!