On Mon, Jan 27, 2014 at 03:02:22PM +0800, Fei Long Wang wrote: > I just verified this scenario. If image uploading is cancelled by Ctrl+C > in v2, the image will be stuck at "saving" status because of > DisconnectionError. Obviously, it doesn't make sense to leave the image in > saving status, though the image can be activated by re-uploading image > file. So if we're going to remove 'killed' status from v2 intentionally, at > least the image status should be rollbacked to 'queued' status. Please let > me know if I missed anything.
I think that's what Mark said - instead of being stranded in a 'saving' state we should roll back to the 'queued' state. Filed a bug: https://bugs.launchpad.net/glance/+bug/1273087 I'll be happy to take it up. -- Koo > > 2014-01-27 14:38:50.611 18964 ERROR glance.api.v2.image_data > [a330c019-a951-431e-be9c-fac04e9ebf68 669ebf06ffa24f0db7e9bce04ccc83a1 > 82d65617aa3244dba88a316234c5fd4e] Failed to upload image data due to > internal error > 2014-01-27 14:38:50.611 18964 TRACE glance.api.v2.image_data Traceback > (most recent call last): > 2014-01-27 14:38:50.611 18964 TRACE glance.api.v2.image_data File > "/opt/stack/glance/glance/api/v2/image_data.py", line 52, in upload > 2014-01-27 14:38:50.611 18964 TRACE glance.api.v2.image_data try: > 2014-01-27 14:38:50.611 18964 TRACE glance.api.v2.image_data File > "/opt/stack/glance/glance/domain/proxy.py", line 127, in set_data > 2014-01-27 14:38:50.611 18964 TRACE glance.api.v2.image_data > self.base.set_data(data, size) > 2014-01-27 14:38:50.611 18964 TRACE glance.api.v2.image_data File > "/opt/stack/glance/glance/domain/proxy.py", line 127, in set_data > 2014-01-27 14:38:50.611 18964 TRACE glance.api.v2.image_data > self.base.set_data(data, size) > 2014-01-27 14:38:50.611 18964 TRACE glance.api.v2.image_data File > "/opt/stack/glance/glance/notifier.py", line 220, in set_data > 2014-01-27 14:38:50.611 18964 TRACE glance.api.v2.image_data > self.image.set_data(data, size) > 2014-01-27 14:38:50.611 18964 TRACE glance.api.v2.image_data File > "/opt/stack/glance/glance/api/policy.py", line 237, in set_data > 2014-01-27 14:38:50.611 18964 TRACE glance.api.v2.image_data return > self.image.set_data(*args, **kwargs) > 2014-01-27 14:38:50.611 18964 TRACE glance.api.v2.image_data File > "/opt/stack/glance/glance/quota/__init__.py", line 274, in set_data > 2014-01-27 14:38:50.611 18964 TRACE glance.api.v2.image_data > self.image.set_data(data, size=size) > 2014-01-27 14:38:50.611 18964 TRACE glance.api.v2.image_data File > "/opt/stack/glance/glance/store/__init__.py", line 668, in set_data > 2014-01-27 14:38:50.611 18964 TRACE glance.api.v2.image_data > self.image.image_id, utils.CooperativeReader(data), size) > 2014-01-27 14:38:50.611 18964 TRACE glance.api.v2.image_data File > "/opt/stack/glance/glance/store/__init__.py", line 358, in add_to_backend > 2014-01-27 14:38:50.611 18964 TRACE glance.api.v2.image_data return > store_add_to_backend(image_id, data, size, store) > 2014-01-27 14:38:50.611 18964 TRACE glance.api.v2.image_data File > "/opt/stack/glance/glance/store/__init__.py", line 336, in > store_add_to_backend > 2014-01-27 14:38:50.611 18964 TRACE glance.api.v2.image_data (location, > size, checksum, metadata) = store.add(image_id, data, size) > 2014-01-27 14:38:50.611 18964 TRACE glance.api.v2.image_data File > "/opt/stack/glance/glance/store/filesystem.py", line 280, in add > 2014-01-27 14:38:50.611 18964 TRACE glance.api.v2.image_data raise > exceptions.get(e.errno, e) > 2014-01-27 14:38:50.611 18964 TRACE glance.api.v2.image_data > DisconnectionError: The client disconnected while sending the POST/PUT body > (640303104 more bytes were expected) > 2014-01-27 14:38:50.611 18964 TRACE glance.api.v2.image_data > > Thanks & Best regards, > Fei Long Wang (?????????) > --------------------------------------------------------------------- > Tech Lead of Nitrogen (SME team) > Cloud Solutions and OpenStack Development > Tel: 8610-82450513 | T/L: 905-0513 > Email: flw...@cn.ibm.com > China Systems & Technology Laboratory in Beijing > --------------------------------------------------------------------- > > > > > From: Mark Washenberger <mark.washenber...@markwash.net> > To: "OpenStack Development Mailing List (not for usage questions)" > <openstack-dev@lists.openstack.org>, > Date: 01/27/2014 09:39 AM > Subject: Re: [openstack-dev] [Glance] Is the 'killed' state ever set in > v2? > > > > > > > On Sun, Jan 26, 2014 at 4:37 PM, David Koo <kpublicm...@gmail.com> wrote: > > > Perhaps there is still a bug where an image is getting stuck in > 'saving' or > > some other state when a PUT fails? > > ?? ?? Yes, that's precisely the problem. > > We should definitely fix that, thanks for pointing it out! > > > ?? ?? Of course, one could argue that that if an upload fails the user > should be able to continue trying until the upload succeeds! But in that > case the image status should probably be reset to "queued" rather than > stay at "saving". > > That's exactly my argument so I'd like to see it go back to 'queued'. > Nothing except the status has substantially changed during an upload that > fails due to either the client or the underlying store, so it is easy to > just revert the status and leave the image in a state where the user can > reattempt the upload. > > > ?? ?? But this makes me a little uneasy because our > consistency/concurrency handling seems a little weak at the moment (am I > right?). If we were to have a more complicated state machine then we > would need much stronger consistency guarantees when there are > simultaneous uploads in progress (where some fail and some succeed)! > > +1 to less complicated state machines :-) > > This is part of what the current work on the import task is designed to > accomplish. When you use import, an image effectively has only two states, > 'active' and nonexistent. > > > > ?? ?? Is there any work on this (concurrency/consistency) front? I > remember seeing some patches related to caching of simultaneous > downloads of an image file where issues related to concurrent update of > image metadata were addressed but IIRC it was -1ed because it reduced > concurrency. > > I might be confused now or confused when I did that review, because I > thought it was reducing download concurrency rather than upload > concurrency. Are you talking about??https://review.openstack.org/#/c/46479/ > ? > > > ?? ?? So do we bring back the 'killed' state or should we shoot for a more > complicated/powerful state machine? > > I think we can get by with trying to simplify the state that is involved > and fixing any bugs with our state management. Is there a specific problem > you're seeing with the > > > -- > Koo > > > On Sun, Jan 26, 2014 at 06:36:36AM -0800, Mark Washenberger wrote: > > It does not seem very ReSTful--or very usable, for that matter--for a > > resource to be permanently modified when you a PUT fails. So I don't > think > > we need the 'killed' status. It was purposefully left out of v2 images, > > which is not just a reskin of v1. > > > > Perhaps there is still a bug where an image is getting stuck in > 'saving' or > > some other state when a PUT fails? > > > > > > On Sun, Jan 26, 2014 at 5:10 AM, David Koo <kpublicm...@gmail.com> > wrote: > > > > > > > > Hi Fei, > > > > > > ?? ?? Thanks for the confirmation. > > > > > > > I think you're right. The 'killed' status should be set in method > > > upload() > > > > if there is an upload failure, see > > > > > > > > https://github.com/openstack/glance/blob/master/glance/common/utils.py#L244 > > > > > > > I think you meant: > > > > > > > > > > > https://github.com/openstack/glance/blob/master/glance/api/v1/upload_utils.py#L244 > > > > > > > (the safe_kill() call) right? > > > > > > -- > > > Koo > > > > > > > > > > ------------------ Original ------------------ > > > > From: ??"David Koo"<kpublicm...@gmail.com>; > > > > Date: ??Jan 26, 2014 > > > > To: ??"OpenStack Development Mailing > > > > List"<openstack-dev@lists.openstack.org>; > > > > Subject: ??[openstack-dev] [Glance] Is the 'killed' state ever set > in v2? > > > > > > > > Hi All, > > > > > > > > While trying to work on a bug I was trying to simulate some image > > > > download failures and found that apparently the 'killed' state is > never > > > > set using v2 APIs. > > > > > > > > If I understand correctly, a file upload goes to > > > > api.v2.image_data.ImageDataController.upload and goes all the way > to > > > > store.ImageProxy.set_data which proceeds to write to the backend > store. > > > > > > > > If the backend store raises an exception it is simply propagated > all the > > > > way up. The notifier re-encodes the exceptions (which is the bug I > was > > > > looking at) but doesn't do anything about the image status. > > > > > > > > Nowhere does the image status seem to get set to 'killed'. > > > > > > > > Before I log a bug I just wanted to confirm with everybody whether > or > > > > not I've missed out on something. > > > > > > > > Thanks. > > > > > > > > -- > > > > Koo _______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev