> On July 28, 2013, 3:59 a.m., Min Chen wrote:
> > server/src/com/cloud/template/HypervisorTemplateAdapter.java, line 249
> > <https://reviews.apache.org/r/12968/diff/2/?file=329846#file329846line249>
> >
> >     This is not right. For cross-zone template, in 
> > HypervisorTemplateAdapter.create(TemplateProfile) method, you will see that 
> > there is a for-loop to download the template to each image store for each 
> > zone. For each image store, this createTemplateAsyncCallback will be 
> > invoked. So in this callback, you should just generate usage event for this 
> > zone.

The reason I do it is because, this part of code will be executed only once 
even when template is cross zone.
If I do not generate all the events here, only the first template that got 
created will publish the usage event.
Usage records do not have the information about store id, so we cannot be sure 
if the usage_record for that store id is already created or not.

It is too much of unnecessary hack to bypass the root issue, I am discarding 
the whole patch and will wait for the root cause (2 async call backs for a 
single register-template) to get fix.
Then it won't be required to do this.


> On July 28, 2013, 3:59 a.m., Min Chen wrote:
> > server/src/com/cloud/template/HypervisorTemplateAdapter.java, line 229
> > <https://reviews.apache.org/r/12968/diff/2/?file=329846#file329846line229>
> >
> >     How can you guarantee that listSrcTmpltStore.get(0) is the entry you 
> > desire? You should find that exact (templateId, storeId) entry by passing 
> > both templateId and storeId, which should be able to be obtained from 
> > template.getDataStore().getId().

This is based on 2 assumptions:
1)Size/physical size of cross-zone template on all the stores will be equal
2)Size/physical sizes are populated only when 100% download is complete.
So even if there are multiple results for the same, all results will have same 
sizes.
For the first time when the template is downloaded to a store, there would be a 
single result having the correct sizes.
For later downloads of template this variable would not be even required 
because I am trying to publish all events at the first shot.


> On July 28, 2013, 3:59 a.m., Min Chen wrote:
> > server/src/com/cloud/template/HypervisorTemplateAdapter.java, line 234
> > <https://reviews.apache.org/r/12968/diff/2/?file=329846#file329846line234>
> >
> >     Why cannot you provide a method in UsageEventDao to search by template 
> > id and event type? This way, you don't need to find a bigger set and filter 
> > here by looping.

Not sure how much we can save here, even though we can add a filter for the 
event-type, a single crosszone template can have multiple usage-records for 
same event-type.
But yes, it will make the search results smaller.


- Saksham


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12968/#review24049
-----------------------------------------------------------


On July 27, 2013, 6:58 p.m., Saksham Srivastava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12968/
> -----------------------------------------------------------
> 
> (Updated July 27, 2013, 6:58 p.m.)
> 
> 
> Review request for cloudstack and Min Chen.
> 
> 
> Bugs: 3686
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Registering a template does not generate a usage event.
> The process should generate a usage event when the template is 100% 
> downloaded.
> Added a new usage event that has virtual_size also as a parameter.
> 
> 
> Diffs
> -----
> 
>   engine/schema/src/com/cloud/event/dao/UsageEventDao.java 01979e1 
>   engine/schema/src/com/cloud/event/dao/UsageEventDaoImpl.java cda02ef 
>   server/src/com/cloud/template/HypervisorTemplateAdapter.java 4b3cade 
> 
> Diff: https://reviews.apache.org/r/12968/diff/
> 
> 
> Testing
> -------
> 
> Now usage_event table is getting updated with the new usage_event.
> 
>  select * from usage_event where id = 23;
> +----+-----------------+------------+---------------------+---------+-------------+---------------+-------------+-------------+----------+---------------+-----------+--------------+
> | id | type            | account_id | created             | zone_id | 
> resource_id | resource_name | offering_id | template_id | size     | 
> resource_type | processed | virtual_size |
> +----+-----------------+------------+---------------------+---------+-------------+---------------+-------------+-------------+----------+---------------+-----------+--------------+
> | 23 | TEMPLATE.CREATE |          2 | 2013-07-26 12:04:29 |       0 |         
> 215 | r8            |        NULL |        NULL | 16777728 | NULL          |  
>        0 |     16777216 |
> +----+-----------------+------------+---------------------+---------+-------------+---------------+-------------+-------------+----------+---------------+-----------+--------------+
> 
> 
> Build passed.
> Patch applies cleanly.
> 
> 
> Thanks,
> 
> Saksham Srivastava
> 
>

Reply via email to