hr_timesheet_sheet: Timesheets big performance issues

Bug #798732 reported by Guewen Baconnier @ Camptocamp
18
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Odoo Addons (MOVED TO GITHUB)
Fix Released
Low
OpenERP R&D Addons Team 3

Bug Description

Hello,

We are experiencing major problems of performance on timesheets.
OpenERP 6.0.2
Performance issues occur with the module hr_timesheet_sheet.

We have ~ lines per tables :
 - hr_timesheet_sheet : 7'500
 - hr_analytic_timesheet : 110'000
 - hr_attendance : 110'000

Opening the "Timesheets" menu takes a few minutes for the 80 first rows.
Then open the timesheet form view takes nearly 30 seconds.
Each action on a timesheet (Sign-in/out, fill a timesheet line, ..) takes nearly 30 seconds again.

It's clear that the main cause of the problem is : hr_timesheet.sheet.day
This view computes the indicators for the whole tables and only afterwards we get the rows that we need.

The second cause of the problem is the fields sheet_id in hr.attendance and hr.analytic.timesheet which are function fields.

The function fields total_attendance_day, total_timesheet_day, total_difference_day, total_attendance, total_timesheet, total_difference of hr_timesheet_sheet.sheet are based on this view, so that's why each action on the timesheet take 30 seconds, because the view is computed again and again.

Here is the actions that I found to bring an acceptable situation:
 - Removed the "By Day" page on the timesheet view, instead I added a link which open a new view (this will still be slow but it can be acceptable as a first solution as you decide to open it...).
 - I replaced the methods to compute the hr_timesheet_sheet.sheet function fields by full python instead of a cr.execute() on the view.
 - There is still a bottleneck, on a search of sheet_id in hr.attendance and hr.analytic.timesheet, it executes each time a sql query. I added store=True to the function field. The only issue I see with that is in the case of a modification of the employee_id of the timesheet, we can add a trigger on this modification, but I don't see how we can search lines to update. What are your thoughts ? (Is it useful to change the employee of a timesheet after its creation, can we block this ?)

This improves a lot the performances, but there is still a lot of room to improve them !

Maybe we have to replace this function fields / one2many_mod with one2many fields ?

Here is the link to my branch (I propose a merge.)
https://code.launchpad.net/~gbaconnier-c2c/openobject-addons/6.0-hr_timesheet_sheet-performance-improvements

Thanks
Guewen

Related branches

description: updated
Revision history for this message
Ferdinand (office-chricar) wrote :

Did you check for indices
https://bugs.launchpad.net/bugs/771941

Changed in openobject-addons:
status: New → Triaged
Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

Hi Guewen,

Thanks for the very interesting merge proposal and the detailed bug report.
I'm confirming the bug and assigning to the R&D team that is responsible for the HR application.

They should:
- first have a decent test environment, including a database with 100,000+ records in the hr_attendance and hr_analytic_timesheet tables (unless Camptocamp has a test database that we could use?). Otherwise it should be possible to write a small script that fills in the attendances until we reach these kind of numbers.
- then review your merge proposal to analyze the effects of the refactoring you did, like combining multiple functions fields in one, or using stored functions when it makes sense
- finally, double-check the rest of the performance to look for other improvements, like combined indices as suggested by Ferdinand

All of the above should be discussed with QDP before implementing.

Thank you for reporting!

Changed in openobject-addons:
assignee: nobody → OpenERP R&D Addons Team 3 (openerp-dev-addons3)
importance: Undecided → Low
status: Triaged → Confirmed
Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

Hi,

Thanks for your consideration.

I updated my merge proposal with triggers on the sheet_id function fields, it is now really better.

Sorry we don't have any database which is not a production database with such volume of data.

I just want to emphasize that for a company with such volume of data (it grow very fast with ~30 users which fill ~10 timesheet lines per day for instance) could just not use the timesheets in their current state.
30 seconds to fill a line of timesheet => you don't do it.

Thanks

Changed in openobject-addons:
status: Confirmed → In Progress
Revision history for this message
Bharat Devnani (Open ERP) (bde-openerp) wrote :

Hello Guewen Baconnier,

Thanks for your time and contribution, i have applied your patch but i have kept the init function of hr_timesheet_sheet_sheet_day class as it is. Now its working fine. The solution of this bug will be merged
in main addons soon. Following are the Revision ID and Number of the branch which solves the bug :

Revision ID : <email address hidden>
Revision Number : 5108

Regards,
Devnani Bharat R.

Changed in openobject-addons:
status: In Progress → Fix Committed
Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

Hello,

Do you see any reason not to modify the view in the hr_timesheet_sheet_sheet_day init ?

We now have sheet_id in both hr.attendance and hr.analytic.timesheet, it is a pity not to use them for the joins in this view instead of joining on dates and user id.

The performance difference is huge, and really I don't understand why you want to keep the actual view ?

Revision history for this message
Bharat Devnani (Open ERP) (bde-openerp) wrote :

Hello Guewen Baconnier,

The reason behind doing that is, if we apply your solution for hr_timesheet_sheet_sheet_day init method, following is the result :

Date From : 22/9/2011
Date To : 22/9/2011

Attendances
-----------------

Date Action Employee's Name
2011-09-22 10:00:00 sign_in Administrator
2011-09-22 13:00:00 sign_out Administrator
2011-09-22 14:00:00 sign_in Administrator
2011-09-22 19:00:00 sign_out Administrator

Period (In Summary Tab)
---------------------------------

Date Attendance Total Timesheet Difference
2011-09-22 24.0 8.0 16.0

It takes 24.0 Hours in Attendance(Consider's whole day) instead of Actual Working Hour i.e (8 hours). And by keeping the same view for in init method it considers Actual working hous, which is correct according to fuctionality. following is the result obtained if we use the existing view :

Date Attendance Total Timesheet Difference
2011-09-22 8.0 8.0 0.0

Hope this will help you.

Thanks & Regards,
Devnani Bharat R.

Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

Hello,

Thanks for your explanation, but that's not the result I have with my code.
The only thing I modified in the view is the join between the table to use the ids instead of the dates. It should not modify the sum of attendances.

Also, we use that in our production at camptocamp and the attendances sum is right.

openerp_prod_camptocamp=# select sheet_id, action, name from hr_attendance where sheet_id = 8077;
 sheet_id | action | name
----------+----------+---------------------
     8077 | sign_in | 2011-09-20 07:06:19
     8077 | sign_out | 2011-09-20 12:42:35
     8077 | sign_in | 2011-09-20 13:35:13
     8077 | sign_out | 2011-09-20 16:40:31
     8077 | sign_in | 2011-09-21 08:21:10
     8077 | sign_out | 2011-09-21 11:25:59
     8077 | sign_in | 2011-09-21 13:03:09
     8077 | sign_out | 2011-09-21 17:39:32
     8077 | sign_in | 2011-09-22 08:19:19
     8077 | sign_out | 2011-09-22 13:02:35
     8077 | sign_in | 2011-09-22 13:44:10

openerp_prod_camptocamp=# select * from hr_timesheet_sheet_sheet_day where sheet_id = 8077;
   id | name | sheet_id | total_timesheet | total_attendance | total_difference
---------+------------+----------+-----------------+------------------+------------------
  116977 | 2011-09-20 | 8077 | 8.7 | 8.68333333333333 | -0.02
  117159 | 2011-09-21 | 8077 | 7.7 | 7.66666666666667 | -0.03
 -117626 | 2011-09-22 | 8077 | 0 | 6.55 | 6.55

Do as you want, but as said before it would be a pity not to modify the joins because the view take 1 second to display with the join on ids against 20 seconds with the joins on dates and user_id. Anyway, when we can do joins on FK and PK it is always better and it is the purpose of my modification.

Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

Anyway thanks for merging this.

Changed in openobject-addons:
status: Fix Committed → Fix Released
Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote : Re: [Bug 798732] Re: hr_timesheet_sheet: Timesheets big performance issues
Download full text (3.1 KiB)

Hello Fabien,

You changed the status to Fix Released put I cannot see the changes on the
branch, it doesn't seems to be merged. Can you check please ?

Thank you!
Guewen
Le 11 nov. 2011 15:35, "Fabien (Open ERP)" <email address hidden> a écrit :

> ** Changed in: openobject-addons
> Status: Fix Committed => Fix Released
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/798732
>
> Title:
> hr_timesheet_sheet: Timesheets big performance issues
>
> Status in OpenERP Addons (modules):
> Fix Released
>
> Bug description:
> Hello,
>
> We are experiencing major problems of performance on timesheets.
> OpenERP 6.0.2
> Performance issues occur with the module hr_timesheet_sheet.
>
> We have ~ lines per tables :
> - hr_timesheet_sheet : 7'500
> - hr_analytic_timesheet : 110'000
> - hr_attendance : 110'000
>
> Opening the "Timesheets" menu takes a few minutes for the 80 first rows.
> Then open the timesheet form view takes nearly 30 seconds.
> Each action on a timesheet (Sign-in/out, fill a timesheet line, ..) takes
> nearly 30 seconds again.
>
> It's clear that the main cause of the problem is : hr_timesheet.sheet.day
> This view computes the indicators for the whole tables and only
> afterwards we get the rows that we need.
>
> The second cause of the problem is the fields sheet_id in
> hr.attendance and hr.analytic.timesheet which are function fields.
>
> The function fields total_attendance_day, total_timesheet_day,
> total_difference_day, total_attendance, total_timesheet,
> total_difference of hr_timesheet_sheet.sheet are based on this view,
> so that's why each action on the timesheet take 30 seconds, because
> the view is computed again and again.
>
> Here is the actions that I found to bring an acceptable situation:
> - Removed the "By Day" page on the timesheet view, instead I added a
> link which open a new view (this will still be slow but it can be
> acceptable as a first solution as you decide to open it...).
> - I replaced the methods to compute the hr_timesheet_sheet.sheet
> function fields by full python instead of a cr.execute() on the view.
> - There is still a bottleneck, on a search of sheet_id in hr.attendance
> and hr.analytic.timesheet, it executes each time a sql query. I added
> store=True to the function field. The only issue I see with that is in the
> case of a modification of the employee_id of the timesheet, we can add a
> trigger on this modification, but I don't see how we can search lines to
> update. What are your thoughts ? (Is it useful to change the employee of a
> timesheet after its creation, can we block this ?)
>
> This improves a lot the performances, but there is still a lot of room
> to improve them !
>
> Maybe we have to replace this function fields / one2many_mod with
> one2many fields ?
>
> Here is the link to my branch (I propose a merge.)
>
> https://code.launchpad.net/~gbaconnier-c2c/openobject-addons/6.0-hr_timesheet_sheet-performance-improvements
>
> Thanks
> Guewen
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/openobject-addons/+bug/798732/+subscription...

Read more...

Revision history for this message
Purnendu Singh (OpenERP) (purnendu-singh) wrote :

Hello,

As the changes are not reflecting into trunk addons, i am changing the status of this bug.

Thanks,
Purnendu Singh

Changed in openobject-addons:
status: Fix Released → Fix Committed
Revision history for this message
Bharat Devnani (Open ERP) (bde-openerp) wrote :

Hello,

I have proposed a new branch lp:~openerp-dev/openobject-addons/trunk-bug-798732-new-bde with following Revision ID and Number:

Revision ID : <email address hidden>
Revision Number : 5651

As there was no diff in previous branch.

Thanks & Regards,
Devnani Bharat R.

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

On 11/11/2011 09:58 PM, Guewen Baconnier @ Camptocamp wrote:
> You changed the status to Fix Released put I cannot see the changes on the
> branch, it doesn't seems to be merged. Can you check please ?

This was probably a mistake due to the incorrect merge proposal with an
empty diff. Status is now corrected, and merge proposal still needs
proper testing and review.

Revision history for this message
Raphael Collet (OpenERP) (rco-openerp) wrote :

landed in trunk
revno: 6302 [merge]
revision-id: <email address hidden>

Changed in openobject-addons:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.