[taskflow] Incorrect state transition on failure of create volume API

Bug #1230189 reported by Rohit Karajgi on 2013-09-25
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Cinder
High
Joshua Harlow

Bug Description

Commit: 1874d913ecbf8f09809992c98496365327905211

State transition of the volume state on failure during VolumeCastTask is not done correctly.

Given the below sequence of task during Create Volume api:
1) (InjectTask) Create request data for rest of the tasks.
2) (ExtractVolumeRequestTask) Extract and validate the API input.
3) (QuotaReserveTask) Reserve the quota for this request.
4) (EntryCreateTask) Create database entry for the volume(s). On revert, sets the volume state to "deleted"
5) (QuotaCommitTask) Commit the reserved quota for this request.
6) (OnFailureChangeStatusTask) Changes state of volume(s) to "error" on revert, if the next task fails
7) (VolumeCastTask) Casts to Scheduler - A Failure occurs during this task before rpc to c-scheduler is made

If failure occurs during the task "VolumeCastTask", then the revert action of the previous task "OnFailureChangeStatusTask", changes state of volume(s) to "error". But while running the revert action of the "EntryCreateTask", the volume state is changed to "deleted". which is not correct.

The state of error should remain until the user requests the volume to be marked deleted in this case.

Tags: ntt Edit Tag help
Changed in cinder:
assignee: nobody → Abhijeet Malawade (abhijeet-malawade)
Changed in cinder:
importance: Undecided → High
milestone: none → havana-rc1
Rohan (kanaderohan) wrote :

Hi John, I was under the assumption that in case of failure the http response is returned to the user and then these failed tasks are reverted.

But since the http response is returned only after all the reversions are completed, IMO if the volume goes into "error" state, we should destroy the volume and return the appropriate http response (500 or 4xx status code) instead of not destroying the volume and letting the user take action later on.

What do you think about this?

John Griffith (john-griffith) wrote :

Rohan,

I sort of like that idea, it would definitely NOT be something for Havana at this point but I think it's something worth discussing in a broader audience.

John Griffith (john-griffith) wrote :

Just to clarify and avoid any confusion that I may have introduced in my previous comment :)

The behavior logged and pointed out in the original bug posting here is in fact a bug and incorrect behavior that needs fixed for Havana. Discussions about changing how we do things in the future we can bring up later. I'm not sure it's what we want to do, just something worth talking about.

Joshua Harlow (harlowja) wrote :

Posted https://review.openstack.org/#/c/49282/, feel free to take ownership of that review if u want.

Changed in cinder:
status: New → In Progress
Changed in cinder:
assignee: Abhijeet Malawade (abhijeet-malawade) → Joshua Harlow (harlowja)

Reviewed: https://review.openstack.org/49282
Committed: http://github.com/openstack/cinder/commit/3814658cec4a075ebef2b7efdcff4b82f7cce830
Submitter: Jenkins
Branch: master

commit 3814658cec4a075ebef2b7efdcff4b82f7cce830
Author: Joshua Harlow <email address hidden>
Date: Tue Oct 1 15:53:30 2013 -0700

    After commiting quota we should avoid certain reverts

    After we commit the quota successfully we do not want to
    set the database volume to destroyed or attempt further
    rollback of the quota itself to reflect what the code
    previously did.

    Closes-Bug: #1230189

    Change-Id: I115dc6736b8f2d0d7b2b6f29e9fd1904fd1c6eee

Changed in cinder:
status: In Progress → Fix Committed
Thierry Carrez (ttx) on 2013-10-04
Changed in cinder:
status: Fix Committed → Fix Released
Thierry Carrez (ttx) on 2013-10-17
Changed in cinder:
milestone: havana-rc1 → 2013.2
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers