| mhen | croelandt: thank you. I reported the bug here: https://bugs.launchpad.net/python-openstackclient/+bug/2111777 | 06:45 |
|---|---|---|
| opendevreview | Abhishek Kekane proposed openstack/glance master: [WIP] Cancel hashing op during image deletion https://review.opendev.org/c/openstack/glance/+/950853 | 10:37 |
| croelandt | abhishekk: so RBD still cannot delete the image after the hash computation has been cancelled? | 14:11 |
| abhishekk | croelandt: I think we need to remove the workaround we have added in location_import HashCalculationTask, also we need to find out a way to abort the task execution or add some sleep time in delete method | 14:17 |
| dansmith | I think the implementation in the patch is not doing what we think | 14:19 |
| dansmith | I'm working on comments now | 14:19 |
| dansmith | I had a beautiful and efficient implementation all written out for you, and then I remembered glance has process workers :/ | 14:20 |
| croelandt | abhishekk: I wonder why exiting the hash calculation is not enough | 14:21 |
| croelandt | does that not end the task? | 14:21 |
| abhishekk | I have added logs locally to check the hash calculation terminated or not | 14:21 |
| dansmith | croelandt: it has to end it within 5s or the originator will try anyway | 14:21 |
| abhishekk | It does end that task and then goes to next task in the flow | 14:21 |
| croelandt | and is there another task in that flow? | 14:22 |
| abhishekk | I will wait for comments before removing these 3 lines | 14:23 |
| abhishekk | https://github.com/openstack/glance/blob/master/glance/async_/flows/location_import.py#L106 | 14:23 |
| dansmith | I think using memcache here is not reasonable | 14:23 |
| abhishekk | yes there is more task in the flow | 14:23 |
| dansmith | it will mean every single person has to deploy with memcache to get proper glance behavior | 14:23 |
| abhishekk | db call then? | 14:23 |
| dansmith | it's also not necessary, and the proxying in this patch is wrong which is one of the reasons why memcache seems necessary | 14:23 |
| dansmith | just let me finish comments :) | 14:24 |
| abhishekk | ack | 14:24 |
| abhishekk | dansmith: going through your comments, I thought of using file, but found that we have memcached_server parameter configured in keystone_authtoken so instead went for using it | 14:53 |
| abhishekk | ALSO what should be the DATA_DIR, should it be hardcoded path or something else? | 14:53 |
| dansmith | it would require memcache to be deployed all the time in order for glance to work properly, so that's not ideal | 14:53 |
| dansmith | and also, the synchronization is only within a single worker (if you do the proxy properly) so no need to depend on memcache so two processes on the same node can communicate | 14:54 |
| dansmith | abhishekk: we already have some directory where glance writes data files right? | 14:54 |
| abhishekk | os_glance_staging_dir in case import is enabled or os_glance_tasks_dir? | 14:55 |
| dansmith | sure | 14:56 |
| abhishekk | ack | 14:57 |
| abhishekk | just few questions related to your comments on registry.py | 15:06 |
| abhishekk | The register_operation should be called from hash calculation task, right? | 15:07 |
| abhishekk | and is_canceled we should check file contains the operation_id written in it? | 15:11 |
| abhishekk | may be signal_cancel is removing the file which means we don't continue hash calculation | 15:13 |
| dansmith | abhishekk: sorry, you didn't mention my nick so I didn't see this.. just replied | 15:47 |
| abhishekk | no worries | 15:47 |
| dansmith | you need a way to signal it's running (file exists), signal to cancel (write), signal that it's stopped (file deleted) | 15:47 |
| abhishekk | ack, three signals ;) | 15:51 |
| croelandt | abhishekk: took a while to review https://review.opendev.org/c/openstack/glance/+/948455 but it's amazing how much you refactored! I just have a question about the bump of a timeout, but it looks great otherwise | 15:55 |
| abhishekk | ack, replied | 15:58 |
| croelandt | ok let's approve that | 16:04 |
| opendevreview | Merged openstack/glance stable/2024.2: Fix vTPM metadefs. https://review.opendev.org/c/openstack/glance/+/948516 | 16:46 |
| opendevreview | Merged openstack/glance master: Functional tests migration of TestImageMultipleBackend https://review.opendev.org/c/openstack/glance/+/948455 | 17:09 |
| croelandt | rosmaita: you'll be pleased to know abhishekk removed your "glance is dead sexy" comment from 2018 | 17:13 |
| croelandt | we're no longer sexy :-( | 17:14 |
| abhishekk | :-( | 17:14 |
| abhishekk | dansmith: writing unit tests for the code you provided, but I am failing to mock external_lock method as it is used for module level method | 19:07 |
| abhishekk | https://paste.opendev.org/show/bqKtyrwaY6XE7xIDGk8z/ | 19:07 |
| abhishekk | Unless I move the import line inside setup, I am not able to find any suitable option | 19:08 |
| dansmith | abhishekk: just do mock.object(glance_module, 'LOCK') | 19:35 |
| abhishekk | this at module level? | 19:35 |
| dansmith | no I mean in your test, but I guess the problem is because it's used as a decorator during import time as well | 19:36 |
| dansmith | just change it to "with LOCK" in the bodies of those functions | 19:36 |
| dansmith | or better yet, put that stuff in a class instead of module-level like you had it, use self._lock and then you can control it better in testing | 19:36 |
| dansmith | make it a singleton in the usual pattern | 19:36 |
| abhishekk | ack | 19:37 |
| dansmith | this is a common thing for making tests work, I am sure you can figure out a way | 19:37 |
| abhishekk | working on it | 19:37 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Add task cancellation tracker utility https://review.opendev.org/c/openstack/glance/+/951031 | 20:24 |
Generated by irclog2html.py 4.0.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!