| *** goldyfruit_ has joined #openstack-glance | 02:35 | |
| *** goldyfruit_ has quit IRC | 02:36 | |
| *** goldyfruit_ has joined #openstack-glance | 02:36 | |
| *** rcernin_ has joined #openstack-glance | 03:13 | |
| *** rcernin has quit IRC | 03:13 | |
| *** rcernin_ has quit IRC | 03:56 | |
| *** rcernin has joined #openstack-glance | 04:00 | |
| *** udesale has joined #openstack-glance | 04:01 | |
| *** goldyfruit_ has quit IRC | 04:24 | |
| *** goldyfruit_ has joined #openstack-glance | 04:24 | |
| *** spsurya has joined #openstack-glance | 04:32 | |
| *** bhagyashris has joined #openstack-glance | 04:43 | |
| *** goldyfruit_ has quit IRC | 04:48 | |
| *** tkajinam has quit IRC | 04:52 | |
| *** gyee has quit IRC | 05:00 | |
| *** goldyfruit_ has joined #openstack-glance | 05:13 | |
| *** tkajinam has joined #openstack-glance | 05:23 | |
| *** abhishekk has joined #openstack-glance | 05:32 | |
| *** bhagyashris has quit IRC | 05:33 | |
| *** bhagyashris has joined #openstack-glance | 05:33 | |
| *** evrardjp has quit IRC | 05:33 | |
| *** evrardjp has joined #openstack-glance | 05:33 | |
| *** ratailor has joined #openstack-glance | 05:35 | |
| *** tkajinam has quit IRC | 05:58 | |
| *** pcaruana has joined #openstack-glance | 06:00 | |
| *** pcaruana has quit IRC | 06:14 | |
| *** ratailor_ has joined #openstack-glance | 06:22 | |
| openstackgerrit | Abhishek Kekane proposed openstack/glance-specs master: Copying existing image in multiple stores https://review.opendev.org/694724 | 06:23 |
|---|---|---|
| *** ratailor has quit IRC | 06:25 | |
| *** tkajinam has joined #openstack-glance | 06:41 | |
| *** tkajinam_ has joined #openstack-glance | 06:52 | |
| *** rcernin has quit IRC | 06:54 | |
| *** tkajinam has quit IRC | 06:55 | |
| *** jawad_axd has joined #openstack-glance | 07:18 | |
| *** bhagyashris has quit IRC | 07:41 | |
| openstackgerrit | Erno Kuvaja proposed openstack/glance master: Add possibility to delete image from single store https://review.opendev.org/698049 | 07:47 |
| jokke_ | abhishekk: what do you think about my sledgehammer approach ^^? | 07:48 |
| openstackgerrit | Erno Kuvaja proposed openstack/glance master: Add possibility to delete image from single store https://review.opendev.org/698049 | 08:04 |
| *** brinzhang has joined #openstack-glance | 08:09 | |
| abhishekk | jokke_, in meeting, will look at it shortly | 08:15 |
| jokke_ | kk | 08:15 |
| *** udesale has quit IRC | 08:16 | |
| *** tesseract has joined #openstack-glance | 08:17 | |
| *** tosky has joined #openstack-glance | 08:23 | |
| *** pcaruana has joined #openstack-glance | 08:29 | |
| abhishekk | jokke_, at first I didn't understand the test case written by Brian | 08:30 |
| *** FlorianFa has quit IRC | 08:30 | |
| abhishekk | and if you look at the failure logs then you can find that even 1st location is not deleted and assert is failing while comparing the locations | 08:31 |
| abhishekk | I am having confusion in function racy_pop at https://review.opendev.org/#/c/698049/12/glance/tests/unit/v2/test_images_resource.py@5260 | 08:33 |
| abhishekk | IMO that test is incorrect or I am missing some pointers here :( | 08:34 |
| jokke_ | abhishekk: I was quite confused initially as well | 08:35 |
| jokke_ | but the location pop actually was injecting the location back on _every_ failure | 08:36 |
| abhishekk | is it? | 08:36 |
| jokke_ | yeah, ifyou look the last rev, it's actually passing that Brians test and addressing it | 08:37 |
| jokke_ | Only change I did that affects something is in the locations.py | 08:38 |
| abhishekk | jokke_, on the latest patch of yours test is still failing | 08:38 |
| jokke_ | hmm-m, wtf if passed locally | 08:39 |
| abhishekk | not sure what is the problem | 08:41 |
| jokke_ | I have feeling that has again something to do about the different db drivers then. and the fact that Brian's test is poking it directly | 08:42 |
| abhishekk | could be | 08:46 |
| *** priteau has joined #openstack-glance | 08:47 | |
| *** tridde has quit IRC | 08:53 | |
| *** trident has joined #openstack-glance | 08:55 | |
| *** jawad_axd has quit IRC | 08:57 | |
| *** tkajinam_ has quit IRC | 08:58 | |
| *** brinzhang_ has joined #openstack-glance | 09:01 | |
| *** brinzhang_ has quit IRC | 09:03 | |
| *** brinzhang_ has joined #openstack-glance | 09:03 | |
| *** jawad_axd has joined #openstack-glance | 09:03 | |
| *** jawad_axd has quit IRC | 09:03 | |
| *** brinzhang has quit IRC | 09:04 | |
| openstackgerrit | Grégoire Unbekandt proposed openstack/glance-specs master: Spec to import image in multi stores https://review.opendev.org/669201 | 09:05 |
| *** bhagyashris has joined #openstack-glance | 09:05 | |
| jokke_ | hmm-m wtf, I have passed py37 test run in my shell buffer, now it fails again :| | 09:06 |
| jokke_ | clearly missing something | 09:06 |
| *** udesale has joined #openstack-glance | 09:07 | |
| *** brinzhang has joined #openstack-glance | 09:08 | |
| *** brinzhang_ has quit IRC | 09:11 | |
| jokke_ | abhishekk: there is definitely something wrong with Brian's mock as well. Even returning the loc instead of raising not found fails the test. | 09:18 |
| abhishekk | jokke_, yes, I am still not able to understand the mock function :( | 09:19 |
| jokke_ | yeah, it almost mde sense until I realized that it's mocking the same function in location.py I did the fix on :P | 09:22 |
| *** brinzhang_ has joined #openstack-glance | 09:22 | |
| jokke_ | then I started testing what if it returns correctly and it didn't still work | 09:22 |
| jokke_ | so there is something wrong with it. Need to ask Brian once he comes online | 09:22 |
| openstackgerrit | Abhishek Kekane proposed openstack/glance master: PoC - Copy existing image in multiple stores https://review.opendev.org/696457 | 09:23 |
| *** brinzhang has quit IRC | 09:25 | |
| abhishekk | jokke_, yes | 09:26 |
| abhishekk | jokke_, between I have tested copying patch that on failure it is deleting data from those stores only where it has copied the data | 09:32 |
| *** abhishekk has quit IRC | 09:33 | |
| *** abhishekk has joined #openstack-glance | 09:55 | |
| jokke_ | abhishekk: that's good. I was expecting so based on how the taskflow stack works but I thought it was important thing to highlight and make sure we cover our arses | 09:58 |
| abhishekk | jokke_, :D | 09:58 |
| jokke_ | and that our image import is not doing anything stupid on the revert | 09:58 |
| abhishekk | yes, I will write functional test to cover it as well | 09:58 |
| *** brinzhang has joined #openstack-glance | 10:02 | |
| *** brinzhang_ has quit IRC | 10:05 | |
| *** udesale has quit IRC | 10:08 | |
| *** udesale has joined #openstack-glance | 10:09 | |
| *** ratailor__ has joined #openstack-glance | 10:22 | |
| *** udesale has quit IRC | 10:24 | |
| *** ratailor_ has quit IRC | 10:25 | |
| *** abhishekk has quit IRC | 10:31 | |
| *** priteau has quit IRC | 10:37 | |
| *** bhagyashris has quit IRC | 10:55 | |
| *** udesale has joined #openstack-glance | 10:56 | |
| *** brinzhang_ has joined #openstack-glance | 10:58 | |
| *** brinzhang has quit IRC | 11:01 | |
| *** bhagyashris has joined #openstack-glance | 11:08 | |
| *** udesale has quit IRC | 11:13 | |
| *** brinzhang has joined #openstack-glance | 11:14 | |
| *** brinzhang_ has quit IRC | 11:17 | |
| *** brinzhang_ has joined #openstack-glance | 11:22 | |
| *** brinzhang has quit IRC | 11:26 | |
| *** lpetrut has joined #openstack-glance | 12:33 | |
| *** jawad_axd has joined #openstack-glance | 12:58 | |
| *** szaher has joined #openstack-glance | 13:00 | |
| *** udesale has joined #openstack-glance | 13:04 | |
| *** brinzhang has joined #openstack-glance | 13:09 | |
| *** brinzhang_ has quit IRC | 13:13 | |
| *** goldyfruit_ has quit IRC | 13:20 | |
| *** ratailor__ has quit IRC | 13:31 | |
| *** priteau has joined #openstack-glance | 13:46 | |
| *** bhagyashris has quit IRC | 13:54 | |
| *** brinzhang_ has joined #openstack-glance | 14:03 | |
| *** brinzhang has quit IRC | 14:06 | |
| rosmaita | jokke_: hello | 14:14 |
| *** pcaruana has quit IRC | 14:20 | |
| *** brinzhang has joined #openstack-glance | 14:24 | |
| *** brinzhang_ has quit IRC | 14:28 | |
| *** pcaruana has joined #openstack-glance | 14:32 | |
| *** brinzhang_ has joined #openstack-glance | 14:36 | |
| *** brinzhang_ has quit IRC | 14:38 | |
| *** brinzhang_ has joined #openstack-glance | 14:38 | |
| *** brinzhang has quit IRC | 14:40 | |
| *** brinzhang_ has quit IRC | 14:40 | |
| *** brinzhang_ has joined #openstack-glance | 14:41 | |
| *** brinzhang_ has quit IRC | 14:42 | |
| *** brinzhang_ has joined #openstack-glance | 14:43 | |
| *** jawad_axd has quit IRC | 14:49 | |
| *** jawad_axd has joined #openstack-glance | 14:50 | |
| *** jawad_ax_ has joined #openstack-glance | 14:53 | |
| *** jawad_axd has quit IRC | 14:55 | |
| *** jawad_ax_ has quit IRC | 14:58 | |
| *** lpetrut has quit IRC | 15:31 | |
| *** udesale has quit IRC | 15:48 | |
| jokke_ | rosmaita: hey | 15:57 |
| jokke_ | rosmaita: did you read backlog or would you like TL;DR? | 15:58 |
| rosmaita | i haven't had a chance to look at your patch yet today, see you had some questions | 15:58 |
| *** stephenfin has left #openstack-glance | 16:03 | |
| *** jawad_axd has joined #openstack-glance | 16:04 | |
| jokke_ | so, I realized after I actually found where we are causing the race condition, that it was the same function you mocked. But the bigger issue is that your test doesn't test the race condition and fails even if the mocked function is changed to return correctly instead of raising the exception | 16:04 |
| rosmaita | well, you can change the mock to mock image_proxy.store_utils.delete_image_location_from_backend() and do the db update and raise NotFound from there | 16:08 |
| jokke_ | not bad idea. | 16:10 |
| rosmaita | the race condition is thread one updates the database to remove the location while thread two gets a NotFound when trying to delete from the backend, and thread two was ultimately calling image.save() and putting the location back | 16:10 |
| rosmaita | the mock simulates thread one by doing the db update directly behind thread two's back | 16:11 |
| rosmaita | but i wonder why we were putting the location back in glance/location.py to begin with | 16:11 |
| rosmaita | (just saw your change in that file) | 16:11 |
| jokke_ | yeah, but there was something dodgy about it as the test fails even if you just comment out the raise and add 'return loc' | 16:12 |
| rosmaita | we probably need to review the circumstances under which glance_store returns NotFoiuhd | 16:12 |
| jokke_ | which is wht the original returns on normal situation | 16:12 |
| jokke_ | rosmaita: I think the store returns not found correctly, but I'm not convinced that the location pop 'expect Exception: with save_and_reraise: <put the location right back to the db' is the right thing to do, like ever if the data is in the location | 16:14 |
| rosmaita | i guess the idea is that if you call pop and got an exception, pop should not modify the list | 16:15 |
| *** gyee has joined #openstack-glance | 16:16 | |
| rosmaita | because you will get the NotFound happening in the calling function, maybe the idea is you decide what to do there | 16:16 |
| jokke_ | you got point there. So maybe we just should not pass on the NotFound from the location.pop() but actually give the location object and just ignore the fact that the data (we're not returning anyways and is not expected to be found in store after) is not in the store | 16:17 |
| jokke_ | we could catch that NotFound from store and literally just ignore it as we've got to the point where we wanted to be anyways | 16:19 |
| rosmaita | yeah, i think leave the locations code as it is, and handle it in your new code | 16:19 |
| rosmaita | though i'm not sure we should ignore it, i think maybe a conflict or something | 16:19 |
| jokke_ | Not convinced on that approach either. the problem is that if we don't fix the location reverting the action every time, there is literally no way to get rid of location after say backend corruption. | 16:21 |
| rosmaita | i don't think the location is actually reverted until you call image.save() in your new code | 16:22 |
| jokke_ | As if the data is not in the backend where it should be, you can't remove it as our code treats it as race condition and returns you conflict | 16:22 |
| jokke_ | So the revert won't on real race condition. but it won't be actually removed from DB either before the save is called | 16:23 |
| jokke_ | So the actual case of store being broken and needing cleanup won't ever work otherwise than going and poking the db manually | 16:24 |
| rosmaita | well, i don't know that we want to delete the location automatically --- maybe need to wait for admin | 16:24 |
| jokke_ | Admin can't do it either. I'm not talking automatically deleting anything. I'm saying if user asks to delete location and the data is not there, why in eart should we revert the delete on database | 16:25 |
| rosmaita | i don't know ... what would happen currently? | 16:25 |
| rosmaita | with just one store | 16:25 |
| rosmaita | we need to preserve that behavior | 16:25 |
| jokke_ | there is currently no way to delete last location, we block that already way before | 16:26 |
| jokke_ | even as admin and locations api exposed it will tell you can't remove last location, delete the image instead | 16:26 |
| jokke_ | and that's something we already check way before we even call the location.pop | 16:27 |
| jokke_ | so that behaviour won't change | 16:27 |
| rosmaita | we need to discuss more later, i am in a meeting now and not concentrating completely | 16:28 |
| jokke_ | yeah, np. I'm heading out. We can continue this tomorrow, or I can put it for the Thu Agenda | 16:29 |
| rosmaita | ok, have a good evening, let's talk tomorrow\ | 16:30 |
| *** pcaruana has quit IRC | 17:05 | |
| *** tosky has quit IRC | 17:23 | |
| *** pcaruana has joined #openstack-glance | 17:28 | |
| *** evrardjp has quit IRC | 17:33 | |
| *** evrardjp has joined #openstack-glance | 17:33 | |
| *** priteau has quit IRC | 17:45 | |
| *** jmlowe has quit IRC | 17:46 | |
| *** jawad_axd has quit IRC | 17:53 | |
| *** jmlowe has joined #openstack-glance | 18:03 | |
| *** brinzhang has joined #openstack-glance | 18:03 | |
| *** jawad_axd has joined #openstack-glance | 18:04 | |
| *** brinzhang_ has quit IRC | 18:06 | |
| *** jmlowe has quit IRC | 18:23 | |
| *** jmlowe has joined #openstack-glance | 18:26 | |
| *** pvradu has joined #openstack-glance | 18:36 | |
| *** pvradu has quit IRC | 18:40 | |
| *** jawad_axd has quit IRC | 18:44 | |
| *** jmlowe has quit IRC | 18:45 | |
| *** trident has quit IRC | 18:48 | |
| *** trident has joined #openstack-glance | 18:49 | |
| *** jmlowe has joined #openstack-glance | 18:57 | |
| *** jmlowe has quit IRC | 18:58 | |
| *** spsurya has quit IRC | 19:25 | |
| *** jmlowe has joined #openstack-glance | 19:47 | |
| *** brinzhang_ has joined #openstack-glance | 19:57 | |
| *** jmlowe has quit IRC | 20:00 | |
| *** brinzhang has quit IRC | 20:00 | |
| *** jawad_axd has joined #openstack-glance | 20:03 | |
| *** jmlowe has joined #openstack-glance | 20:04 | |
| *** jawad_axd has quit IRC | 20:09 | |
| *** brinzhang has joined #openstack-glance | 20:40 | |
| *** pcaruana has quit IRC | 20:41 | |
| *** brinzhang_ has quit IRC | 20:44 | |
| *** brinzhang_ has joined #openstack-glance | 20:46 | |
| *** brinzhang has quit IRC | 20:49 | |
| *** brinzhang has joined #openstack-glance | 20:54 | |
| *** brinzhang has quit IRC | 20:56 | |
| *** brinzhang has joined #openstack-glance | 20:57 | |
| *** brinzhang_ has quit IRC | 20:58 | |
| *** pvradu has joined #openstack-glance | 21:13 | |
| *** rcernin has joined #openstack-glance | 21:37 | |
| *** rcernin has quit IRC | 21:37 | |
| *** rcernin has joined #openstack-glance | 21:38 | |
| *** pvradu has quit IRC | 21:47 | |
| *** rosmaita has quit IRC | 22:27 | |
| *** tkajinam has joined #openstack-glance | 23:08 | |
| *** tesseract has quit IRC | 23:13 | |
| *** goldyfruit_ has joined #openstack-glance | 23:17 | |
| *** brinzhang_ has joined #openstack-glance | 23:31 | |
| *** brinzhang has quit IRC | 23:35 | |
| *** pvradu has joined #openstack-glance | 23:44 | |
| *** brinzhang has joined #openstack-glance | 23:49 | |
| *** brinzhang_ has quit IRC | 23:53 | |
| *** brinzhang_ has joined #openstack-glance | 23:54 | |
| *** brinzhang has quit IRC | 23:55 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!