You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Min Chen <mi...@citrix.com> on 2012/11/21 19:54:54 UTC
Review Request: List API performance optimization part I.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8172/
-----------------------------------------------------------
Review request for cloudstack and Prachi Damle.
Description
-------
This is part 1 of list API refactoring.
Commands covered: listVmsCmd, listRoutersCmd
Response covered: UserVmResponse, DomainRouterResponse.
DB views created: user_vm_view, domain_router_view.
This addresses bug https://issues.apache.org/jira/browse/CLOUDSTACK-527.
Diffs
-----
api/src/com/cloud/api/ResponseGenerator.java 996a5fc
api/src/com/cloud/api/ResponseObject.java 2d08fb9
api/src/com/cloud/api/commands/ListRoutersCmd.java 8bf9ba8
api/src/com/cloud/api/commands/ListVMsCmd.java 2f6f988
api/src/com/cloud/api/response/BaseResponse.java e343a10
api/src/com/cloud/api/response/DomainRouterResponse.java d710aad
api/src/com/cloud/api/response/IControlledViewEntityResponse.java PRE-CREATION
api/src/com/cloud/api/response/NicResponse.java 69d5c31
api/src/com/cloud/api/response/UserVmResponse.java f74c072
api/src/com/cloud/api/view/vo/DomainRouterJoinVO.java PRE-CREATION
api/src/com/cloud/api/view/vo/IControlledViewEntity.java PRE-CREATION
api/src/com/cloud/api/view/vo/UserVmJoinVO.java PRE-CREATION
api/src/com/cloud/server/ManagementService.java 7532cae
api/src/com/cloud/vm/UserVmService.java 02682b0
server/src/com/cloud/api/ApiDBUtils.java 418fe362
server/src/com/cloud/api/ApiResponseHelper.java caabd71
server/src/com/cloud/api/ApiServer.java a5c9ea5
server/src/com/cloud/api/response/ApiResponseSerializer.java dc15763
server/src/com/cloud/configuration/DefaultComponentLibrary.java 4edd402
server/src/com/cloud/server/ManagementServerImpl.java 117be57
server/src/com/cloud/user/AccountManager.java 90a34ad
server/src/com/cloud/user/AccountManagerImpl.java 0def008
server/src/com/cloud/vm/UserVmManager.java 4ce9bfe
server/src/com/cloud/vm/UserVmManagerImpl.java a2a548f
server/src/com/cloud/vm/dao/DomainRouterJoinDao.java PRE-CREATION
server/src/com/cloud/vm/dao/DomainRouterJoinDaoImpl.java PRE-CREATION
server/src/com/cloud/vm/dao/UserVmJoinDao.java PRE-CREATION
server/src/com/cloud/vm/dao/UserVmJoinDaoImpl.java PRE-CREATION
server/test/com/cloud/api/APITest.java PRE-CREATION
server/test/com/cloud/api/ListPerfTest.java PRE-CREATION
server/test/com/cloud/api/LoginResponse.java PRE-CREATION
server/test/com/cloud/keystore/KeystoreTest.java e0e2126
server/test/com/cloud/user/MockAccountManagerImpl.java 08234fd
server/test/com/cloud/vm/MockUserVmManagerImpl.java 35ee139
setup/db/create-schema.sql acbbc41
utils/src/com/cloud/utils/db/GenericDaoBase.java 8d5cb96
Diff: https://reviews.apache.org/r/8172/diff/
Testing
-------
Create a performance unit test class to test the performance time.
Thanks,
Min Chen
Re: Review Request: List API performance optimization part I.
Posted by Min Chen <mi...@citrix.com>.
That is a good idea for such API refactoring efforts mentioned in
https://cwiki.apache.org/confluence/display/CLOUDSTACK/CloudStack+API+refac
toring, since these work will touch every API, not just list API since it
involves refactoring all Response objects to remove IdentityMapper and
thus other create/operational apis will be impacted as well. I don't know
if we have already a suite of test cases to test API regressions. Sudha,
any ideas from QA side?
Thanks
-min
On 11/21/12 5:29 PM, "David Nalley" <da...@gnsa.us> wrote:
>>> On Nov. 22, 2012, 12:05 a.m., Chiradeep Vittal wrote:
>>> > server/src/com/cloud/vm/UserVmManagerImpl.java, line 3006
>>> >
>>><https://reviews.apache.org/r/8172/diff/1/?file=222634#file222634line300
>>>6>
>>> >
>>> > this is a major piece of surgery on this method. How are you
>>>sure that the before and after results are identical?
>>
>> I have done a prototype before this checkin and verified that it
>>returns the same result as before. UserVmJoinVO is view object
>>containing all the fields before in UserVm interface, no information is
>>lost.
>
>
>Perhaps we should have tests that establish existing present
>functionality, and then have those in place before this surgery is
>performed which could then assure us that the functionality is
>identical.
>
>--David
Re: Review Request: List API performance optimization part I.
Posted by David Nalley <da...@gnsa.us>.
>> On Nov. 22, 2012, 12:05 a.m., Chiradeep Vittal wrote:
>> > server/src/com/cloud/vm/UserVmManagerImpl.java, line 3006
>> > <https://reviews.apache.org/r/8172/diff/1/?file=222634#file222634line3006>
>> >
>> > this is a major piece of surgery on this method. How are you sure that the before and after results are identical?
>
> I have done a prototype before this checkin and verified that it returns the same result as before. UserVmJoinVO is view object containing all the fields before in UserVm interface, no information is lost.
Perhaps we should have tests that establish existing present
functionality, and then have those in place before this surgery is
performed which could then assure us that the functionality is
identical.
--David
Re: Review Request: List API performance optimization part I.
Posted by Min Chen <mi...@citrix.com>.
> On Nov. 22, 2012, 12:05 a.m., Chiradeep Vittal wrote:
> > utils/src/com/cloud/utils/db/GenericDaoBase.java, line 1811
> > <https://reviews.apache.org/r/8172/diff/1/?file=222646#file222646line1811>
> >
> > this change seems unrelated? perhaps it needs a new bug?
SQL statement used here to get total count for list API (select count(*) ....) will not work if there is group by clause.
> On Nov. 22, 2012, 12:05 a.m., Chiradeep Vittal wrote:
> > server/src/com/cloud/vm/UserVmManagerImpl.java, line 3006
> > <https://reviews.apache.org/r/8172/diff/1/?file=222634#file222634line3006>
> >
> > this is a major piece of surgery on this method. How are you sure that the before and after results are identical?
I have done a prototype before this checkin and verified that it returns the same result as before. UserVmJoinVO is view object containing all the fields before in UserVm interface, no information is lost.
> On Nov. 22, 2012, 12:05 a.m., Chiradeep Vittal wrote:
> > api/src/com/cloud/api/view/vo/IControlledViewEntity.java, line 27
> > <https://reviews.apache.org/r/8172/diff/1/?file=222621#file222621line27>
> >
> > It isn't the current practice to prefix interfaces with "I".
I can change that to ControlledViewEntity.
- Min
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8172/#review13683
-----------------------------------------------------------
On Nov. 21, 2012, 6:54 p.m., Min Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8172/
> -----------------------------------------------------------
>
> (Updated Nov. 21, 2012, 6:54 p.m.)
>
>
> Review request for cloudstack and Prachi Damle.
>
>
> Description
> -------
>
> This is part 1 of list API refactoring.
> Commands covered: listVmsCmd, listRoutersCmd
> Response covered: UserVmResponse, DomainRouterResponse.
> DB views created: user_vm_view, domain_router_view.
>
>
> This addresses bug https://issues.apache.org/jira/browse/CLOUDSTACK-527.
>
>
> Diffs
> -----
>
> api/src/com/cloud/api/ResponseGenerator.java 996a5fc
> api/src/com/cloud/api/ResponseObject.java 2d08fb9
> api/src/com/cloud/api/commands/ListRoutersCmd.java 8bf9ba8
> api/src/com/cloud/api/commands/ListVMsCmd.java 2f6f988
> api/src/com/cloud/api/response/BaseResponse.java e343a10
> api/src/com/cloud/api/response/DomainRouterResponse.java d710aad
> api/src/com/cloud/api/response/IControlledViewEntityResponse.java PRE-CREATION
> api/src/com/cloud/api/response/NicResponse.java 69d5c31
> api/src/com/cloud/api/response/UserVmResponse.java f74c072
> api/src/com/cloud/api/view/vo/DomainRouterJoinVO.java PRE-CREATION
> api/src/com/cloud/api/view/vo/IControlledViewEntity.java PRE-CREATION
> api/src/com/cloud/api/view/vo/UserVmJoinVO.java PRE-CREATION
> api/src/com/cloud/server/ManagementService.java 7532cae
> api/src/com/cloud/vm/UserVmService.java 02682b0
> server/src/com/cloud/api/ApiDBUtils.java 418fe362
> server/src/com/cloud/api/ApiResponseHelper.java caabd71
> server/src/com/cloud/api/ApiServer.java a5c9ea5
> server/src/com/cloud/api/response/ApiResponseSerializer.java dc15763
> server/src/com/cloud/configuration/DefaultComponentLibrary.java 4edd402
> server/src/com/cloud/server/ManagementServerImpl.java 117be57
> server/src/com/cloud/user/AccountManager.java 90a34ad
> server/src/com/cloud/user/AccountManagerImpl.java 0def008
> server/src/com/cloud/vm/UserVmManager.java 4ce9bfe
> server/src/com/cloud/vm/UserVmManagerImpl.java a2a548f
> server/src/com/cloud/vm/dao/DomainRouterJoinDao.java PRE-CREATION
> server/src/com/cloud/vm/dao/DomainRouterJoinDaoImpl.java PRE-CREATION
> server/src/com/cloud/vm/dao/UserVmJoinDao.java PRE-CREATION
> server/src/com/cloud/vm/dao/UserVmJoinDaoImpl.java PRE-CREATION
> server/test/com/cloud/api/APITest.java PRE-CREATION
> server/test/com/cloud/api/ListPerfTest.java PRE-CREATION
> server/test/com/cloud/api/LoginResponse.java PRE-CREATION
> server/test/com/cloud/keystore/KeystoreTest.java e0e2126
> server/test/com/cloud/user/MockAccountManagerImpl.java 08234fd
> server/test/com/cloud/vm/MockUserVmManagerImpl.java 35ee139
> setup/db/create-schema.sql acbbc41
> utils/src/com/cloud/utils/db/GenericDaoBase.java 8d5cb96
>
> Diff: https://reviews.apache.org/r/8172/diff/
>
>
> Testing
> -------
>
> Create a performance unit test class to test the performance time.
>
>
> Thanks,
>
> Min Chen
>
>
Re: Review Request: List API performance optimization part I.
Posted by Chiradeep Vittal <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8172/#review13683
-----------------------------------------------------------
api/src/com/cloud/api/view/vo/IControlledViewEntity.java
<https://reviews.apache.org/r/8172/#comment29323>
It isn't the current practice to prefix interfaces with "I".
server/src/com/cloud/vm/UserVmManagerImpl.java
<https://reviews.apache.org/r/8172/#comment29325>
this is a major piece of surgery on this method. How are you sure that the before and after results are identical?
utils/src/com/cloud/utils/db/GenericDaoBase.java
<https://reviews.apache.org/r/8172/#comment29324>
this change seems unrelated? perhaps it needs a new bug?
- Chiradeep Vittal
On Nov. 21, 2012, 6:54 p.m., Min Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8172/
> -----------------------------------------------------------
>
> (Updated Nov. 21, 2012, 6:54 p.m.)
>
>
> Review request for cloudstack and Prachi Damle.
>
>
> Description
> -------
>
> This is part 1 of list API refactoring.
> Commands covered: listVmsCmd, listRoutersCmd
> Response covered: UserVmResponse, DomainRouterResponse.
> DB views created: user_vm_view, domain_router_view.
>
>
> This addresses bug https://issues.apache.org/jira/browse/CLOUDSTACK-527.
>
>
> Diffs
> -----
>
> api/src/com/cloud/api/ResponseGenerator.java 996a5fc
> api/src/com/cloud/api/ResponseObject.java 2d08fb9
> api/src/com/cloud/api/commands/ListRoutersCmd.java 8bf9ba8
> api/src/com/cloud/api/commands/ListVMsCmd.java 2f6f988
> api/src/com/cloud/api/response/BaseResponse.java e343a10
> api/src/com/cloud/api/response/DomainRouterResponse.java d710aad
> api/src/com/cloud/api/response/IControlledViewEntityResponse.java PRE-CREATION
> api/src/com/cloud/api/response/NicResponse.java 69d5c31
> api/src/com/cloud/api/response/UserVmResponse.java f74c072
> api/src/com/cloud/api/view/vo/DomainRouterJoinVO.java PRE-CREATION
> api/src/com/cloud/api/view/vo/IControlledViewEntity.java PRE-CREATION
> api/src/com/cloud/api/view/vo/UserVmJoinVO.java PRE-CREATION
> api/src/com/cloud/server/ManagementService.java 7532cae
> api/src/com/cloud/vm/UserVmService.java 02682b0
> server/src/com/cloud/api/ApiDBUtils.java 418fe362
> server/src/com/cloud/api/ApiResponseHelper.java caabd71
> server/src/com/cloud/api/ApiServer.java a5c9ea5
> server/src/com/cloud/api/response/ApiResponseSerializer.java dc15763
> server/src/com/cloud/configuration/DefaultComponentLibrary.java 4edd402
> server/src/com/cloud/server/ManagementServerImpl.java 117be57
> server/src/com/cloud/user/AccountManager.java 90a34ad
> server/src/com/cloud/user/AccountManagerImpl.java 0def008
> server/src/com/cloud/vm/UserVmManager.java 4ce9bfe
> server/src/com/cloud/vm/UserVmManagerImpl.java a2a548f
> server/src/com/cloud/vm/dao/DomainRouterJoinDao.java PRE-CREATION
> server/src/com/cloud/vm/dao/DomainRouterJoinDaoImpl.java PRE-CREATION
> server/src/com/cloud/vm/dao/UserVmJoinDao.java PRE-CREATION
> server/src/com/cloud/vm/dao/UserVmJoinDaoImpl.java PRE-CREATION
> server/test/com/cloud/api/APITest.java PRE-CREATION
> server/test/com/cloud/api/ListPerfTest.java PRE-CREATION
> server/test/com/cloud/api/LoginResponse.java PRE-CREATION
> server/test/com/cloud/keystore/KeystoreTest.java e0e2126
> server/test/com/cloud/user/MockAccountManagerImpl.java 08234fd
> server/test/com/cloud/vm/MockUserVmManagerImpl.java 35ee139
> setup/db/create-schema.sql acbbc41
> utils/src/com/cloud/utils/db/GenericDaoBase.java 8d5cb96
>
> Diff: https://reviews.apache.org/r/8172/diff/
>
>
> Testing
> -------
>
> Create a performance unit test class to test the performance time.
>
>
> Thanks,
>
> Min Chen
>
>
Re: Review Request: List API performance optimization part I.
Posted by Min Chen <mi...@citrix.com>.
> On Nov. 22, 2012, 12:33 a.m., Prachi Damle wrote:
> > Looks good to me apart from changing interface name to match current format - Also all our VO's refer zoneId as datacenterId - can we keep it uniform in the views as well.
> > Are we changing the ListVMsCmd and ListRoutersCmd to not use @IdentityMapper? or will that be a separate change?
>
> Min Chen wrote:
> Yes, this also changes ListVmsCmd and ListRoutersCmd to not use @IdentityMapper, since that is the main bottleneck for performance. This is reflected in UserVmResponse and DomainRouterResponse class change. I can change interface naming convention and zoneId to datacenterId.
Regarding your comments of zoneId and datacenterId, my understanding is a bit different. They are not the same, based on our Admin Documentation, "A zone typically corresponds to a single datacenter, although it is permissible to have multiple zones in a datacenter". So it is possible to have multiple zones in one data center. In that sense, I think that zoneId is more accurate than datacenterId when we have more than one zones in one datacenter. However, I did notice that in our internal db schema, we use "datacenter" to represent zone concept. I guess that to avoid a broader change, I will have to change those fields (zoneId, zoneUuid, zoneName) to (dataCenterId, dataCenterUuid, dataCenterName).
- Min
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8172/#review13685
-----------------------------------------------------------
On Nov. 21, 2012, 6:54 p.m., Min Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8172/
> -----------------------------------------------------------
>
> (Updated Nov. 21, 2012, 6:54 p.m.)
>
>
> Review request for cloudstack and Prachi Damle.
>
>
> Description
> -------
>
> This is part 1 of list API refactoring.
> Commands covered: listVmsCmd, listRoutersCmd
> Response covered: UserVmResponse, DomainRouterResponse.
> DB views created: user_vm_view, domain_router_view.
>
>
> This addresses bug https://issues.apache.org/jira/browse/CLOUDSTACK-527.
>
>
> Diffs
> -----
>
> api/src/com/cloud/api/ResponseGenerator.java 996a5fc
> api/src/com/cloud/api/ResponseObject.java 2d08fb9
> api/src/com/cloud/api/commands/ListRoutersCmd.java 8bf9ba8
> api/src/com/cloud/api/commands/ListVMsCmd.java 2f6f988
> api/src/com/cloud/api/response/BaseResponse.java e343a10
> api/src/com/cloud/api/response/DomainRouterResponse.java d710aad
> api/src/com/cloud/api/response/IControlledViewEntityResponse.java PRE-CREATION
> api/src/com/cloud/api/response/NicResponse.java 69d5c31
> api/src/com/cloud/api/response/UserVmResponse.java f74c072
> api/src/com/cloud/api/view/vo/DomainRouterJoinVO.java PRE-CREATION
> api/src/com/cloud/api/view/vo/IControlledViewEntity.java PRE-CREATION
> api/src/com/cloud/api/view/vo/UserVmJoinVO.java PRE-CREATION
> api/src/com/cloud/server/ManagementService.java 7532cae
> api/src/com/cloud/vm/UserVmService.java 02682b0
> server/src/com/cloud/api/ApiDBUtils.java 418fe362
> server/src/com/cloud/api/ApiResponseHelper.java caabd71
> server/src/com/cloud/api/ApiServer.java a5c9ea5
> server/src/com/cloud/api/response/ApiResponseSerializer.java dc15763
> server/src/com/cloud/configuration/DefaultComponentLibrary.java 4edd402
> server/src/com/cloud/server/ManagementServerImpl.java 117be57
> server/src/com/cloud/user/AccountManager.java 90a34ad
> server/src/com/cloud/user/AccountManagerImpl.java 0def008
> server/src/com/cloud/vm/UserVmManager.java 4ce9bfe
> server/src/com/cloud/vm/UserVmManagerImpl.java a2a548f
> server/src/com/cloud/vm/dao/DomainRouterJoinDao.java PRE-CREATION
> server/src/com/cloud/vm/dao/DomainRouterJoinDaoImpl.java PRE-CREATION
> server/src/com/cloud/vm/dao/UserVmJoinDao.java PRE-CREATION
> server/src/com/cloud/vm/dao/UserVmJoinDaoImpl.java PRE-CREATION
> server/test/com/cloud/api/APITest.java PRE-CREATION
> server/test/com/cloud/api/ListPerfTest.java PRE-CREATION
> server/test/com/cloud/api/LoginResponse.java PRE-CREATION
> server/test/com/cloud/keystore/KeystoreTest.java e0e2126
> server/test/com/cloud/user/MockAccountManagerImpl.java 08234fd
> server/test/com/cloud/vm/MockUserVmManagerImpl.java 35ee139
> setup/db/create-schema.sql acbbc41
> utils/src/com/cloud/utils/db/GenericDaoBase.java 8d5cb96
>
> Diff: https://reviews.apache.org/r/8172/diff/
>
>
> Testing
> -------
>
> Create a performance unit test class to test the performance time.
>
>
> Thanks,
>
> Min Chen
>
>
Re: Review Request: List API performance optimization part I.
Posted by Min Chen <mi...@citrix.com>.
> On Nov. 22, 2012, 12:33 a.m., Prachi Damle wrote:
> > Looks good to me apart from changing interface name to match current format - Also all our VO's refer zoneId as datacenterId - can we keep it uniform in the views as well.
> > Are we changing the ListVMsCmd and ListRoutersCmd to not use @IdentityMapper? or will that be a separate change?
Yes, this also changes ListVmsCmd and ListRoutersCmd to not use @IdentityMapper, since that is the main bottleneck for performance. This is reflected in UserVmResponse and DomainRouterResponse class change. I can change interface naming convention and zoneId to datacenterId.
- Min
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8172/#review13685
-----------------------------------------------------------
On Nov. 21, 2012, 6:54 p.m., Min Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8172/
> -----------------------------------------------------------
>
> (Updated Nov. 21, 2012, 6:54 p.m.)
>
>
> Review request for cloudstack and Prachi Damle.
>
>
> Description
> -------
>
> This is part 1 of list API refactoring.
> Commands covered: listVmsCmd, listRoutersCmd
> Response covered: UserVmResponse, DomainRouterResponse.
> DB views created: user_vm_view, domain_router_view.
>
>
> This addresses bug https://issues.apache.org/jira/browse/CLOUDSTACK-527.
>
>
> Diffs
> -----
>
> api/src/com/cloud/api/ResponseGenerator.java 996a5fc
> api/src/com/cloud/api/ResponseObject.java 2d08fb9
> api/src/com/cloud/api/commands/ListRoutersCmd.java 8bf9ba8
> api/src/com/cloud/api/commands/ListVMsCmd.java 2f6f988
> api/src/com/cloud/api/response/BaseResponse.java e343a10
> api/src/com/cloud/api/response/DomainRouterResponse.java d710aad
> api/src/com/cloud/api/response/IControlledViewEntityResponse.java PRE-CREATION
> api/src/com/cloud/api/response/NicResponse.java 69d5c31
> api/src/com/cloud/api/response/UserVmResponse.java f74c072
> api/src/com/cloud/api/view/vo/DomainRouterJoinVO.java PRE-CREATION
> api/src/com/cloud/api/view/vo/IControlledViewEntity.java PRE-CREATION
> api/src/com/cloud/api/view/vo/UserVmJoinVO.java PRE-CREATION
> api/src/com/cloud/server/ManagementService.java 7532cae
> api/src/com/cloud/vm/UserVmService.java 02682b0
> server/src/com/cloud/api/ApiDBUtils.java 418fe362
> server/src/com/cloud/api/ApiResponseHelper.java caabd71
> server/src/com/cloud/api/ApiServer.java a5c9ea5
> server/src/com/cloud/api/response/ApiResponseSerializer.java dc15763
> server/src/com/cloud/configuration/DefaultComponentLibrary.java 4edd402
> server/src/com/cloud/server/ManagementServerImpl.java 117be57
> server/src/com/cloud/user/AccountManager.java 90a34ad
> server/src/com/cloud/user/AccountManagerImpl.java 0def008
> server/src/com/cloud/vm/UserVmManager.java 4ce9bfe
> server/src/com/cloud/vm/UserVmManagerImpl.java a2a548f
> server/src/com/cloud/vm/dao/DomainRouterJoinDao.java PRE-CREATION
> server/src/com/cloud/vm/dao/DomainRouterJoinDaoImpl.java PRE-CREATION
> server/src/com/cloud/vm/dao/UserVmJoinDao.java PRE-CREATION
> server/src/com/cloud/vm/dao/UserVmJoinDaoImpl.java PRE-CREATION
> server/test/com/cloud/api/APITest.java PRE-CREATION
> server/test/com/cloud/api/ListPerfTest.java PRE-CREATION
> server/test/com/cloud/api/LoginResponse.java PRE-CREATION
> server/test/com/cloud/keystore/KeystoreTest.java e0e2126
> server/test/com/cloud/user/MockAccountManagerImpl.java 08234fd
> server/test/com/cloud/vm/MockUserVmManagerImpl.java 35ee139
> setup/db/create-schema.sql acbbc41
> utils/src/com/cloud/utils/db/GenericDaoBase.java 8d5cb96
>
> Diff: https://reviews.apache.org/r/8172/diff/
>
>
> Testing
> -------
>
> Create a performance unit test class to test the performance time.
>
>
> Thanks,
>
> Min Chen
>
>
Re: Review Request: List API performance optimization part I.
Posted by Prachi Damle <pr...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8172/#review13685
-----------------------------------------------------------
Looks good to me apart from changing interface name to match current format - Also all our VO's refer zoneId as datacenterId - can we keep it uniform in the views as well.
Are we changing the ListVMsCmd and ListRoutersCmd to not use @IdentityMapper? or will that be a separate change?
- Prachi Damle
On Nov. 21, 2012, 6:54 p.m., Min Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8172/
> -----------------------------------------------------------
>
> (Updated Nov. 21, 2012, 6:54 p.m.)
>
>
> Review request for cloudstack and Prachi Damle.
>
>
> Description
> -------
>
> This is part 1 of list API refactoring.
> Commands covered: listVmsCmd, listRoutersCmd
> Response covered: UserVmResponse, DomainRouterResponse.
> DB views created: user_vm_view, domain_router_view.
>
>
> This addresses bug https://issues.apache.org/jira/browse/CLOUDSTACK-527.
>
>
> Diffs
> -----
>
> api/src/com/cloud/api/ResponseGenerator.java 996a5fc
> api/src/com/cloud/api/ResponseObject.java 2d08fb9
> api/src/com/cloud/api/commands/ListRoutersCmd.java 8bf9ba8
> api/src/com/cloud/api/commands/ListVMsCmd.java 2f6f988
> api/src/com/cloud/api/response/BaseResponse.java e343a10
> api/src/com/cloud/api/response/DomainRouterResponse.java d710aad
> api/src/com/cloud/api/response/IControlledViewEntityResponse.java PRE-CREATION
> api/src/com/cloud/api/response/NicResponse.java 69d5c31
> api/src/com/cloud/api/response/UserVmResponse.java f74c072
> api/src/com/cloud/api/view/vo/DomainRouterJoinVO.java PRE-CREATION
> api/src/com/cloud/api/view/vo/IControlledViewEntity.java PRE-CREATION
> api/src/com/cloud/api/view/vo/UserVmJoinVO.java PRE-CREATION
> api/src/com/cloud/server/ManagementService.java 7532cae
> api/src/com/cloud/vm/UserVmService.java 02682b0
> server/src/com/cloud/api/ApiDBUtils.java 418fe362
> server/src/com/cloud/api/ApiResponseHelper.java caabd71
> server/src/com/cloud/api/ApiServer.java a5c9ea5
> server/src/com/cloud/api/response/ApiResponseSerializer.java dc15763
> server/src/com/cloud/configuration/DefaultComponentLibrary.java 4edd402
> server/src/com/cloud/server/ManagementServerImpl.java 117be57
> server/src/com/cloud/user/AccountManager.java 90a34ad
> server/src/com/cloud/user/AccountManagerImpl.java 0def008
> server/src/com/cloud/vm/UserVmManager.java 4ce9bfe
> server/src/com/cloud/vm/UserVmManagerImpl.java a2a548f
> server/src/com/cloud/vm/dao/DomainRouterJoinDao.java PRE-CREATION
> server/src/com/cloud/vm/dao/DomainRouterJoinDaoImpl.java PRE-CREATION
> server/src/com/cloud/vm/dao/UserVmJoinDao.java PRE-CREATION
> server/src/com/cloud/vm/dao/UserVmJoinDaoImpl.java PRE-CREATION
> server/test/com/cloud/api/APITest.java PRE-CREATION
> server/test/com/cloud/api/ListPerfTest.java PRE-CREATION
> server/test/com/cloud/api/LoginResponse.java PRE-CREATION
> server/test/com/cloud/keystore/KeystoreTest.java e0e2126
> server/test/com/cloud/user/MockAccountManagerImpl.java 08234fd
> server/test/com/cloud/vm/MockUserVmManagerImpl.java 35ee139
> setup/db/create-schema.sql acbbc41
> utils/src/com/cloud/utils/db/GenericDaoBase.java 8d5cb96
>
> Diff: https://reviews.apache.org/r/8172/diff/
>
>
> Testing
> -------
>
> Create a performance unit test class to test the performance time.
>
>
> Thanks,
>
> Min Chen
>
>
Re: Review Request: List API performance optimization part I.
Posted by Rohit Yadav <ro...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8172/#review14081
-----------------------------------------------------------
Ship it!
Please git pull latest api refactoring, the patch was big about 5-6k lines and huge merge conflicts, took me some time to merge them.
Please check the code, in case I missed something.
The build system is broken now, we need to fix it.
Committed in api_refactoring;
commit b0ce8fd4ff512c7a7da0a71671c4477492e854e7
Author: Min Chen <mi...@citrix.com>
Date: Wed Dec 5 16:18:38 2012 -0800
api: Optimize and improve api, db call perfomance
This is part 1 of list API refactoring. Commands covered:
listVmsCmd, listRoutersCmd Response covered:
UserVmResponse, DomainRouterResponse. DB views created:
user_vm_view, domain_router_view.
Signed-off-by: Rohit Yadav <bh...@apache.org>
- Rohit Yadav
On Nov. 29, 2012, 6:57 p.m., Min Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8172/
> -----------------------------------------------------------
>
> (Updated Nov. 29, 2012, 6:57 p.m.)
>
>
> Review request for cloudstack and Prachi Damle.
>
>
> Description
> -------
>
> This is part 1 of list API refactoring.
> Commands covered: listVmsCmd, listRoutersCmd
> Response covered: UserVmResponse, DomainRouterResponse.
> DB views created: user_vm_view, domain_router_view.
>
>
> This addresses bug https://issues.apache.org/jira/browse/CLOUDSTACK-527.
>
>
> Diffs
> -----
>
> api/src/com/cloud/api/ResponseGenerator.java 4e8fbd8
> api/src/com/cloud/api/ResponseObject.java 2d08fb9
> api/src/com/cloud/api/commands/ListRoutersCmd.java 8bf9ba8
> api/src/com/cloud/api/commands/ListVMsCmd.java 2f6f988
> api/src/com/cloud/api/response/BaseResponse.java e343a10
> api/src/com/cloud/api/response/ControlledViewEntityResponse.java PRE-CREATION
> api/src/com/cloud/api/response/DomainRouterResponse.java d710aad
> api/src/com/cloud/api/response/NicResponse.java 69d5c31
> api/src/com/cloud/api/response/UserVmResponse.java f74c072
> api/src/com/cloud/api/view/vo/ControlledViewEntity.java PRE-CREATION
> api/src/com/cloud/api/view/vo/DomainRouterJoinVO.java PRE-CREATION
> api/src/com/cloud/api/view/vo/UserVmJoinVO.java PRE-CREATION
> api/src/com/cloud/server/ManagementService.java 7532cae
> api/src/com/cloud/vm/UserVmService.java 98d02db
> api/test/src/com/cloud/api/commands/test/ListRoutersCmdTest.java PRE-CREATION
> api/test/src/com/cloud/api/commands/test/ListVmsCmdTest.java PRE-CREATION
> server/src/com/cloud/api/ApiDBUtils.java 3b5f634
> server/src/com/cloud/api/ApiResponseHelper.java ebe8415
> server/src/com/cloud/api/ApiServer.java a5c9ea5
> server/src/com/cloud/api/response/ApiResponseSerializer.java 4be5dfa
> server/src/com/cloud/configuration/DefaultComponentLibrary.java ef61044
> server/src/com/cloud/server/ManagementServerImpl.java 117be57
> server/src/com/cloud/user/AccountManager.java 90a34ad
> server/src/com/cloud/user/AccountManagerImpl.java f595478
> server/src/com/cloud/vm/UserVmManager.java 4ce9bfe
> server/src/com/cloud/vm/UserVmManagerImpl.java 687f521
> server/src/com/cloud/vm/dao/DomainRouterJoinDao.java PRE-CREATION
> server/src/com/cloud/vm/dao/DomainRouterJoinDaoImpl.java PRE-CREATION
> server/src/com/cloud/vm/dao/UserVmJoinDao.java PRE-CREATION
> server/src/com/cloud/vm/dao/UserVmJoinDaoImpl.java PRE-CREATION
> server/test/com/cloud/keystore/KeystoreTest.java e0e2126
> server/test/com/cloud/user/MockAccountManagerImpl.java 08234fd
> server/test/com/cloud/vm/MockUserVmManagerImpl.java 35ee139
> setup/db/create-schema.sql fff084e
> utils/src/com/cloud/utils/db/GenericDaoBase.java 8d5cb96
>
> Diff: https://reviews.apache.org/r/8172/diff/
>
>
> Testing
> -------
>
> Create a performance unit test class to test the performance time.
>
>
> Thanks,
>
> Min Chen
>
>
Re: Review Request: List API performance optimization part I.
Posted by Min Chen <mi...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8172/
-----------------------------------------------------------
(Updated Nov. 29, 2012, 6:57 p.m.)
Review request for cloudstack and Prachi Damle.
Changes
-------
Added unit testcases for those two modified list apis. Based on response to my question on dev list, this change should come with unit testcase with no dependent on db, and functional testcases that will rely on a real MS and DB running. Functional testcases for these two APIs are already existing, so no new functional tests are needed for this patch.
Description
-------
This is part 1 of list API refactoring.
Commands covered: listVmsCmd, listRoutersCmd
Response covered: UserVmResponse, DomainRouterResponse.
DB views created: user_vm_view, domain_router_view.
This addresses bug https://issues.apache.org/jira/browse/CLOUDSTACK-527.
Diffs (updated)
-----
api/src/com/cloud/api/ResponseGenerator.java 4e8fbd8
api/src/com/cloud/api/ResponseObject.java 2d08fb9
api/src/com/cloud/api/commands/ListRoutersCmd.java 8bf9ba8
api/src/com/cloud/api/commands/ListVMsCmd.java 2f6f988
api/src/com/cloud/api/response/BaseResponse.java e343a10
api/src/com/cloud/api/response/ControlledViewEntityResponse.java PRE-CREATION
api/src/com/cloud/api/response/DomainRouterResponse.java d710aad
api/src/com/cloud/api/response/NicResponse.java 69d5c31
api/src/com/cloud/api/response/UserVmResponse.java f74c072
api/src/com/cloud/api/view/vo/ControlledViewEntity.java PRE-CREATION
api/src/com/cloud/api/view/vo/DomainRouterJoinVO.java PRE-CREATION
api/src/com/cloud/api/view/vo/UserVmJoinVO.java PRE-CREATION
api/src/com/cloud/server/ManagementService.java 7532cae
api/src/com/cloud/vm/UserVmService.java 98d02db
api/test/src/com/cloud/api/commands/test/ListRoutersCmdTest.java PRE-CREATION
api/test/src/com/cloud/api/commands/test/ListVmsCmdTest.java PRE-CREATION
server/src/com/cloud/api/ApiDBUtils.java 3b5f634
server/src/com/cloud/api/ApiResponseHelper.java ebe8415
server/src/com/cloud/api/ApiServer.java a5c9ea5
server/src/com/cloud/api/response/ApiResponseSerializer.java 4be5dfa
server/src/com/cloud/configuration/DefaultComponentLibrary.java ef61044
server/src/com/cloud/server/ManagementServerImpl.java 117be57
server/src/com/cloud/user/AccountManager.java 90a34ad
server/src/com/cloud/user/AccountManagerImpl.java f595478
server/src/com/cloud/vm/UserVmManager.java 4ce9bfe
server/src/com/cloud/vm/UserVmManagerImpl.java 687f521
server/src/com/cloud/vm/dao/DomainRouterJoinDao.java PRE-CREATION
server/src/com/cloud/vm/dao/DomainRouterJoinDaoImpl.java PRE-CREATION
server/src/com/cloud/vm/dao/UserVmJoinDao.java PRE-CREATION
server/src/com/cloud/vm/dao/UserVmJoinDaoImpl.java PRE-CREATION
server/test/com/cloud/keystore/KeystoreTest.java e0e2126
server/test/com/cloud/user/MockAccountManagerImpl.java 08234fd
server/test/com/cloud/vm/MockUserVmManagerImpl.java 35ee139
setup/db/create-schema.sql fff084e
utils/src/com/cloud/utils/db/GenericDaoBase.java 8d5cb96
Diff: https://reviews.apache.org/r/8172/diff/
Testing
-------
Create a performance unit test class to test the performance time.
Thanks,
Min Chen
Re: Review Request: List API performance optimization part I.
Posted by Prachi Damle <pr...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8172/#review13785
-----------------------------------------------------------
Changes are fine. But we should add a unit test to test that the results returned by the List calls before and after the changes are exactly same. For this the unit test can work on a sample db data.
- Prachi Damle
On Nov. 26, 2012, 7:53 p.m., Min Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8172/
> -----------------------------------------------------------
>
> (Updated Nov. 26, 2012, 7:53 p.m.)
>
>
> Review request for cloudstack and Prachi Damle.
>
>
> Description
> -------
>
> This is part 1 of list API refactoring.
> Commands covered: listVmsCmd, listRoutersCmd
> Response covered: UserVmResponse, DomainRouterResponse.
> DB views created: user_vm_view, domain_router_view.
>
>
> This addresses bug https://issues.apache.org/jira/browse/CLOUDSTACK-527.
>
>
> Diffs
> -----
>
> api/src/com/cloud/api/ResponseGenerator.java 996a5fc
> api/src/com/cloud/api/ResponseObject.java 2d08fb9
> api/src/com/cloud/api/commands/ListRoutersCmd.java 8bf9ba8
> api/src/com/cloud/api/commands/ListVMsCmd.java 2f6f988
> api/src/com/cloud/api/response/BaseResponse.java e343a10
> api/src/com/cloud/api/response/DomainRouterResponse.java d710aad
> api/src/com/cloud/api/response/IControlledViewEntityResponse.java PRE-CREATION
> api/src/com/cloud/api/response/NicResponse.java 69d5c31
> api/src/com/cloud/api/response/UserVmResponse.java f74c072
> api/src/com/cloud/api/view/vo/ControlledViewEntity.java PRE-CREATION
> api/src/com/cloud/api/view/vo/DomainRouterJoinVO.java PRE-CREATION
> api/src/com/cloud/api/view/vo/UserVmJoinVO.java PRE-CREATION
> api/src/com/cloud/server/ManagementService.java 7532cae
> api/src/com/cloud/vm/UserVmService.java 02682b0
> server/src/com/cloud/api/ApiDBUtils.java 418fe362
> server/src/com/cloud/api/ApiResponseHelper.java caabd71
> server/src/com/cloud/api/ApiServer.java a5c9ea5
> server/src/com/cloud/api/response/ApiResponseSerializer.java dc15763
> server/src/com/cloud/configuration/DefaultComponentLibrary.java 4edd402
> server/src/com/cloud/server/ManagementServerImpl.java 117be57
> server/src/com/cloud/user/AccountManager.java 90a34ad
> server/src/com/cloud/user/AccountManagerImpl.java 0def008
> server/src/com/cloud/vm/UserVmManager.java 4ce9bfe
> server/src/com/cloud/vm/UserVmManagerImpl.java a2a548f
> server/src/com/cloud/vm/dao/DomainRouterJoinDao.java PRE-CREATION
> server/src/com/cloud/vm/dao/DomainRouterJoinDaoImpl.java PRE-CREATION
> server/src/com/cloud/vm/dao/UserVmJoinDao.java PRE-CREATION
> server/src/com/cloud/vm/dao/UserVmJoinDaoImpl.java PRE-CREATION
> server/test/com/cloud/api/APITest.java PRE-CREATION
> server/test/com/cloud/api/ListPerfTest.java PRE-CREATION
> server/test/com/cloud/api/LoginResponse.java PRE-CREATION
> server/test/com/cloud/keystore/KeystoreTest.java e0e2126
> server/test/com/cloud/user/MockAccountManagerImpl.java 08234fd
> server/test/com/cloud/vm/MockUserVmManagerImpl.java 35ee139
> setup/db/create-schema.sql acbbc41
> utils/src/com/cloud/utils/db/GenericDaoBase.java 8d5cb96
>
> Diff: https://reviews.apache.org/r/8172/diff/
>
>
> Testing
> -------
>
> Create a performance unit test class to test the performance time.
>
>
> Thanks,
>
> Min Chen
>
>
Re: Review Request: List API performance optimization part I.
Posted by Min Chen <mi...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8172/
-----------------------------------------------------------
(Updated Nov. 26, 2012, 7:53 p.m.)
Review request for cloudstack and Prachi Damle.
Changes
-------
Addressed two comments in the new patch:
1) Rename IControlledViewEntity interface name to ControlledViewEntity
2) Rename all zone related field names to dataCenter prefixed names based on CloudStack convention.
Description
-------
This is part 1 of list API refactoring.
Commands covered: listVmsCmd, listRoutersCmd
Response covered: UserVmResponse, DomainRouterResponse.
DB views created: user_vm_view, domain_router_view.
This addresses bug https://issues.apache.org/jira/browse/CLOUDSTACK-527.
Diffs (updated)
-----
api/src/com/cloud/api/ResponseGenerator.java 996a5fc
api/src/com/cloud/api/ResponseObject.java 2d08fb9
api/src/com/cloud/api/commands/ListRoutersCmd.java 8bf9ba8
api/src/com/cloud/api/commands/ListVMsCmd.java 2f6f988
api/src/com/cloud/api/response/BaseResponse.java e343a10
api/src/com/cloud/api/response/DomainRouterResponse.java d710aad
api/src/com/cloud/api/response/IControlledViewEntityResponse.java PRE-CREATION
api/src/com/cloud/api/response/NicResponse.java 69d5c31
api/src/com/cloud/api/response/UserVmResponse.java f74c072
api/src/com/cloud/api/view/vo/ControlledViewEntity.java PRE-CREATION
api/src/com/cloud/api/view/vo/DomainRouterJoinVO.java PRE-CREATION
api/src/com/cloud/api/view/vo/UserVmJoinVO.java PRE-CREATION
api/src/com/cloud/server/ManagementService.java 7532cae
api/src/com/cloud/vm/UserVmService.java 02682b0
server/src/com/cloud/api/ApiDBUtils.java 418fe362
server/src/com/cloud/api/ApiResponseHelper.java caabd71
server/src/com/cloud/api/ApiServer.java a5c9ea5
server/src/com/cloud/api/response/ApiResponseSerializer.java dc15763
server/src/com/cloud/configuration/DefaultComponentLibrary.java 4edd402
server/src/com/cloud/server/ManagementServerImpl.java 117be57
server/src/com/cloud/user/AccountManager.java 90a34ad
server/src/com/cloud/user/AccountManagerImpl.java 0def008
server/src/com/cloud/vm/UserVmManager.java 4ce9bfe
server/src/com/cloud/vm/UserVmManagerImpl.java a2a548f
server/src/com/cloud/vm/dao/DomainRouterJoinDao.java PRE-CREATION
server/src/com/cloud/vm/dao/DomainRouterJoinDaoImpl.java PRE-CREATION
server/src/com/cloud/vm/dao/UserVmJoinDao.java PRE-CREATION
server/src/com/cloud/vm/dao/UserVmJoinDaoImpl.java PRE-CREATION
server/test/com/cloud/api/APITest.java PRE-CREATION
server/test/com/cloud/api/ListPerfTest.java PRE-CREATION
server/test/com/cloud/api/LoginResponse.java PRE-CREATION
server/test/com/cloud/keystore/KeystoreTest.java e0e2126
server/test/com/cloud/user/MockAccountManagerImpl.java 08234fd
server/test/com/cloud/vm/MockUserVmManagerImpl.java 35ee139
setup/db/create-schema.sql acbbc41
utils/src/com/cloud/utils/db/GenericDaoBase.java 8d5cb96
Diff: https://reviews.apache.org/r/8172/diff/
Testing
-------
Create a performance unit test class to test the performance time.
Thanks,
Min Chen