Mailing-List: contact dev-help@cloudstack.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cloudstack.apache.org
Received-SPF: pass (nike.apache.org: domain of min.chen@citrix.com designates
 66.165.176.89 as permitted sender)
From: Min Chen <min.chen@citrix.com>
To: "dev@cloudstack.apache.org" <dev@cloudstack.apache.org>
Date: Thu, 25 Apr 2013 14:54:03 -0700
Subject: Re: [ACS41] Blocker and critical issues stopping me from spinning
 out a new RC for 4.1
Thread-Topic: [ACS41] Blocker and critical issues stopping me from spinning
 out a new RC for 4.1
Thread-Index: Ac5B/2cXWPbC4fC+SBCjlnuvn11txA==
Message-ID: <CD9EF190.23480%min.chen@citrix.com>
In-Reply-To: 
 <61AE1E2837A06D4A8E98B796183842D4013E9BD262A4@SJCPMAILBOX01.citrite.net>
Accept-Language: en-US
Content-Language: en-US
user-agent: Microsoft-MacOutlook/14.3.2.130206
acceptlanguage: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0

I did further analysis on the issue (CLOUDSTACK-2126)  and checked the
code back in 3.0.x and 4.0 branch, and found the following:
1. Showing pending job information for ListXXX api has been broken since
3.0.6. Based on ML discussion, this feature is requested by customers, so
we should fix this in 4.1. For this, I created this bug
(https://issues.apache.org/jira/browse/CLOUDSTACK-2196) for this issue.
2. The main issue reported by Prasanna in
https://issues.apache.org/jira/browse/CLOUDSTACK-2126 is more or less
caused by CLOUDSTACK-2196. Due to empty instance_type column in async_job
table, UserVmResponse will always have an incorrect job status for the
newly created DB view. If CLOUDSTACK-2196 is fixed, we will have the
following expected behavior: async job information will not show up in the
completed VM and will only show up in the pending VM.
Based on these new investigation, I think that we need to fix these two
bugs in 4.1.0, and assigned to myself to address them. Should be able to
submit a patch for them by the end of today.

Thanks
-min


On 4/25/13 10:52 AM, "Will Chan" <will.chan@citrix.com> wrote:

>Alena is right.  They were introduced eons ago for customers to figure
>out which objects still had pending jobs on them without having the need
>to know the async job ID in the first place.  I'm not sure who's using
>that now but if you remove it, it may cause existing customers that do
>rely on this.
>
>Will
>
>> -----Original Message-----
>> From: Alena Prokharchyk [mailto:Alena.Prokharchyk@citrix.com]
>> Sent: Thursday, April 25, 2013 10:48 AM
>> To: dev@cloudstack.apache.org
>> Subject: Re: [ACS41] Blocker and critical issues stopping me from
>>spinning
>> out a new RC for 4.1
>>=20
>> JobId/JobStatus were introduced to the list* responses on purpose. Some
>> of the CS customers wanted to see what async jobs that are currently
>>being
>> executed on the object, just by looking at the result of list* command.
>>=20
>> So removing async job params from the list response is not a good idea
>>as
>> customers' software might rely on its presence.
>>=20
>> On 4/25/13 10:32 AM, "Min Chen" <min.chen@citrix.com> wrote:
>>=20
>> >Totally agree.
>> >
>> >Chip and others, Prasanna and I exchanged ideas about CLOUDSTACK--
>> 2126,
>> >and we both agree that we should only return async job Id and job
>> >status in queryAsyncJob and listAsyncJob commands, not in any other
>> >APIs so that we can have consistent response return in various
>> >scenarios.  If we fix this way, the behavior for listXXX api is
>> >changed. In 4.0, CloudStack has used ApiServer.buildAsyncListResponse()
>> >routine to fill in async job Id and status for all listXXX Apis.  With
>> >the proposed fix, we will not show async job information in listXXX
>> >response as well. Not sure if anybody see any side effect on this
>> >slight change from 4.0? Anybody know the reason why we had that special
>> code in 4.0 to return async job for listXXX api?
>> >Since the proposed change will affect all newly created db view in 4.1,
>> >we suggest that this can be fixed in 4.2 since current behavior is not
>> >breaking functionalities, just return more information. Any thoughts?
>> >
>> >Thanks
>> >-min
>> >
>> >
>> >
>> >
>> >
>> >
>> >On 4/25/13 10:05 AM, "Prasanna Santhanam" <tsp@apache.org> wrote:
>> >
>> >>That's odd - listXxx commands don't do async at all and shouldn't be
>> >>generating jobstatus,jobid etc. So I'm not sure why that was added in
>> >>(prior?) 4.0. I'd like to take that out too if there's no real reason
>> >>behind it.
>> >>
>> >>--
>> >>Prasanna.,
>> >>
>> >>On Thu, Apr 25, 2013 at 09:57:02AM -0700, Min Chen wrote:
>> >>> In 4.0, CS has special code to return job status for a VM returned
>> >>>from  listVMsCmd. During API performance refactoring, I have a
>> >>>created a DB view  user_vm_view that joins async_job table just for
>> >>>that purpose and used  that view to uniformly generate
>> >>>UserVmResponse. So the same code will be  applied to generate
>> >>>UserVmResponse whenever it is used. In this case,  deployVMCmd itself
>> >>>will also return a UserVmResponse, thus the same code  applied, and
>> >>>so that is what you see. If we all agree that job status  should not
>> >>>appear in UserVmResponse, then I can change the view to remove  job
>> >>>from async_job. But I would argue that we should not return jobStatus
>> >>>in ListVmsCmd as well, this will also be a change from 4.0 release.
>> >>>
>> >>> Thanks
>> >>> -min
>> >>>
>> >>> On 4/25/13 9:48 AM, "Prasanna Santhanam" <tsp@apache.org>
>> wrote:
>> >>>
>> >>> >On Thu, Apr 25, 2013 at 09:30:08AM -0700, Min Chen wrote:
>> >>> >> Prasanna, I updated CLOUDSTACK-2126 with my comment. That is
>> the
>> >>> >>intended  change done in list API performance improvement work,
>> >>> >>and I don't
>> >>>see
>> >>> >>any
>> >>> >> issues by having the consistent UserVmResponse for both
>> >>> >>deployVMCmd
>> >>>and
>> >>> >> listVMsCmd. Every BaseResponse class has jobId and jobStatus as
>> >>> >>serialized  fields, I don't see why marvin has issues in
>> >>> >>deserialization in this case.
>> >>> >> Did I miss anything?
>> >>> >>
>> >>> >
>> >>> >I'm not sure why internal representation should be a reason to
>> >>> >surface it upwards. But that's not the part I'm concerned with: If
>> >>> >you look at the response carefully - queryAsyncJobResultResponse
>> >>> >contains two jobstatus attributes. One for the query job and one as
>> >>> >part of the virtualmachine (within the virtualmachine block). The
>> >>> >concern is with the latter.
>> >>> >
>> >>> >That block pasted for brevity:
>> >>> >
>> >>> >virtualmachine : {
>> >>> >	"id": "649663f7-3c8d-4e0d-b693-4b1ea6085a84",
>> >>> >	"name": "649663f7-3c8d-4e0d-b693-4b1ea6085a84",
>> >>> >	"account": "QX7KKV",
>> >>> >	...
>> >>> >	..
>> >>> >	"zoneid": "6e301be1-8010-4b57-9638-c90761e40dc9",
>> >>> >    "jobstatus": 0 <?My Problem?>
>> >>> >}
>> >>> >
>> >>> >These attributes qualify a VM, but I'm not sure why jobstatus is in
>> >>> >there. That's an attribute of the job itself which is CloudStack's
>> >>> >concern, but not the VM's concern. When marvin looks to deserialize
>> >>> >back to a VM object, it looks at the inner block only. I can
>> >>> >workaround these within marvin, so feel free to reduce the priority
>> >>> >if you think the bug can be fixed later. Just that jobstatus
>> >>> >represented as a VM attribute doesn't seem right to me.
>> >>> >
>> >>> >Thanks,
>> >>> >
>> >>> >--
>> >>> >Prasanna.,
>> >>> >
>> >>> >------------------------
>> >>> >Powered by BigRock.com
>> >>> >
>> >>
>> >>
>> >>------------------------
>> >>Powered by BigRock.com
>> >>
>> >
>> >
>>=20
>