opendevreview | Ke Niu proposed openstack/glance master: Use py3 as the default runtime for tox https://review.opendev.org/c/openstack/glance/+/889832 | 02:50 |
---|---|---|
opendevreview | Ke Niu proposed openstack/glance_store master: Use py3 as the default runtime for tox https://review.opendev.org/c/openstack/glance_store/+/889833 | 02:57 |
whoami-rajat | abhishekk, do you mean this comment? https://review.opendev.org/c/openstack/glance/+/886749/9/glance/async_/flows/location_import.py#126 | 09:30 |
abhishekk | whoami-rajat, yep | 09:50 |
whoami-rajat | abhishekk, ack, looking | 09:50 |
abhishekk | ack | 09:50 |
whoami-rajat | abhishekk, shouldn't the state be set to importing in the _CalculateHash task? It seems strange that we are setting it to importing after the whole hash calculation step which is a long running task | 09:54 |
abhishekk | we defined state transition in spec, I think it is right as per the spec | 09:58 |
abhishekk | https://review.opendev.org/c/openstack/glance-specs/+/883491/8/specs/2023.2/approved/glance/new-location-info-apis.rst | 09:58 |
abhishekk | line 100 | 09:58 |
* abhishekk going afk | 10:02 | |
whoami-rajat | abhishekk, I'm not so sure, there are 2 cases 1) validation data is not provided 2) validation data is provided | 10:02 |
whoami-rajat | for 1) we set the image to active state and calculate the hash in background | 10:02 |
whoami-rajat | for 2) the initial state is 'queued', then we start calculating the hash and the state is 'importing' and finally when hash is calculated and set, the state is 'active' | 10:02 |
abhishekk | yes I think that importing needs to be set earlier, you can add comment on the patch | 10:05 |
abhishekk | I think to avoid conflict pranali has set it in verify task, but yeah, that need to be set in calculate hash which will also avoid the potential race | 10:06 |
abhishekk | So make it simpler similar to _SetImageToActiveTask need to add _SetImageToImportingTask or refactor it to _SetImageTransition and use it twice in the flow | 10:08 |
abhishekk | whoami-rajat, add your comment on the patch | 10:09 |
* abhishekk going away now | 10:10 | |
dansmith | whoami-rajat: doing it anywhere in the task doesn't solve the atomicity concern though | 13:35 |
whoami-rajat | dansmith, yes correct, but at least it lowers the window for a race, allowing hashing to be done in queued state provides a long window for other operations to occur on that image | 19:04 |
dansmith | a small bit, but not enough on a busy cloud. I added atomic operations for this purpose, so we should use them (*and* set to importing at the start so a client knows it's progressing) | 19:04 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!