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.c...@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.prokharc...@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 >> >> 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. >> >> So removing async job params from the list response is not a good idea >>as >> customers' software might rely on its presence. >> >> On 4/25/13 10:32 AM, "Min Chen" <min.c...@citrix.com> wrote: >> >> >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" <t...@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" <t...@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 >> >> >> > >> > >> >
