> On July 26, 2013, 5:38 p.m., Min Chen wrote: > > server/src/com/cloud/template/HypervisorTemplateAdapter.java, line 230 > > <https://reviews.apache.org/r/12968/diff/1/?file=328266#file328266line230> > > > > I don't understand why you need to do a bunch of search here. This > > method is invoked when a template is successfully downloaded to a secondary > > storage, why cannot you just do a simple UsageEventUtils.publishUsageEvents > > here just like _resourceLimitMgr.incrementResourceCount? template should > > have all the information you need to publish a usage event. > > Saksham Srivastava wrote: > Min, > I had to add these checks because the method was getting called twice for > a single event of register template, due to which there were duplicate > entries for usage event. > Until the method is guaranteed to be called once, duplicate entries would > be made for a single usage record and also incrementResourceCount was getting > executed twice and thus resulted in a faulty resource count. > So for usage-event I am trying to search the usage records and if an > entry for template creation for the same template exists, refrain from > creating a usage entry again. > Once the root issue is fixed, the changes won't be needed but until then, > they are required.
Ok, I remember that problem, yes, for some reasons, this createTemplateAsyncCallBack is called twice at the end of template download, I haven't figured out the real reason yet. Thanks for clarification. But I still have a question here: from my own testing, when control is coming to this method, template is sure already downloaded (100%), why cannot you just do a simple duplicate usage record check to decide whether to publish usage event or not? I saw that in this code before checking usage record, you have done a bunch of check regarding cross zones and download percent, etc. Any catch here? Also, it is also better to put some comments here for the usage check workaround so that the other developer can understand. - Min ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12968/#review23955 ----------------------------------------------------------- On July 26, 2013, 12:42 p.m., Saksham Srivastava wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12968/ > ----------------------------------------------------------- > > (Updated July 26, 2013, 12:42 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 > >
