Merge lp:~numerigraphe-team/ocb-addons/7.0-fill-inventory-OOM into lp:ocb-addons

Proposed by Loïc Bellier - Numérigraphe
Status: Merged
Merged at revision: 10210
Proposed branch: lp:~numerigraphe-team/ocb-addons/7.0-fill-inventory-OOM
Merge into: lp:ocb-addons
Diff against target: 66 lines (+27/-20)
1 file modified
stock/wizard/stock_fill_inventory.py (+27/-20)
To merge this branch: bzr merge lp:~numerigraphe-team/ocb-addons/7.0-fill-inventory-OOM
Reviewer Review Type Date Requested Status
Laurent Mignon (Acsone) (community) code review Approve
Guewen Baconnier @ Camptocamp code review, test Approve
Lionel Sausin - Initiatives/Numérigraphe (community) co-author Abstain
Holger Brunn (Therp) code review Approve
Review via email: mp+217049@code.launchpad.net

Description of the change

We're having problems importing an inventory for a location that contains >630000 Stock Moves.
The problem seems to lie in the python loop based on the browse() of the Stock Moves: it preloads too much data and pushes the OS to an out-of-memory condition (which Linux responds to by killing the openerp server).
This patch solve this bug by splitting stock moves lines ids on separated list of 10 000 lines

Run GREEN on runbot.

pull request on standard 7.0: https://github.com/odoo/odoo/pull/341

To post a comment you must log in.
Revision history for this message
Lionel Sausin - Initiatives/Numérigraphe (ls-initiatives) :
review: Approve
Revision history for this message
Holger Brunn (Therp) (hbrunn) wrote :

I think you'd be better off using search(..., limit=your_limit, offset=your_offset).

If you don't want to make that change, at least the 10000 should be in a variable and you should use range(start, end, step)

review: Needs Fixing (code review)
Revision history for this message
Lionel Sausin - Initiatives/Numérigraphe (ls-initiatives) wrote :

Holger, using search with an offset is cooler when you expect to never load the end of the list (like displaying it for users).
In the case here, it would only trade python-server memory consumption for SQL round-trips. Is it worth?

Revision history for this message
Holger Brunn (Therp) (hbrunn) wrote :

You're right, it's not. But still I suggest doing the stepping part not by hand but as a parameter for range()

Revision history for this message
Holger Brunn (Therp) (hbrunn) :
review: Approve (code review)
Revision history for this message
Lionel Sausin - Initiatives/Numérigraphe (ls-initiatives) wrote :

Thanks Holger, I've updated Loïc's patch according to your suggestion.
Of course I'll abstain now that I'm co-author :)

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

LGTM.

Thanks!

review: Approve (code review, test)
Revision history for this message
Lionel Sausin - Initiatives/Numérigraphe (ls-initiatives) wrote :

Thank you. For the archives, the MP on official addons was turned into a pull request on github here: https://github.com/odoo/odoo/pull/341

Revision history for this message
Laurent Mignon (Acsone) (lmi) wrote :

Thanks for the MP.

LGTM for V7

Note that for V8, with the new API (https://github.com/odoo-dev/odoo/blob/master-apiculture/openerp/osv/orm.py), the cache is managed in an other way and is no more reset on each call to the browse method.

lmi

review: Approve (code review)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'stock/wizard/stock_fill_inventory.py'
--- stock/wizard/stock_fill_inventory.py 2014-03-24 09:14:18 +0000
+++ stock/wizard/stock_fill_inventory.py 2014-05-12 08:28:17 +0000
@@ -27,6 +27,9 @@
27 _name = "stock.fill.inventory"27 _name = "stock.fill.inventory"
28 _description = "Import Inventory"28 _description = "Import Inventory"
2929
30 # Maximum size of a batch of lines we can import without risking OOM
31 MAX_IMPORT_LINES = 10000
32
30 def _default_location(self, cr, uid, ids, context=None):33 def _default_location(self, cr, uid, ids, context=None):
31 try:34 try:
32 location = self.pool.get('ir.model.data').get_object(cr, uid, 'stock', 'stock_location_stock')35 location = self.pool.get('ir.model.data').get_object(cr, uid, 'stock', 'stock_location_stock')
@@ -103,28 +106,32 @@
103 for location in location_ids:106 for location in location_ids:
104 datas = {}107 datas = {}
105 res[location] = {}108 res[location] = {}
106 move_ids = move_obj.search(cr, uid, ['|',('location_dest_id','=',location),('location_id','=',location),('state','=','done')], context=context)109 all_move_ids = move_obj.search(cr, uid, ['|',('location_dest_id','=',location),('location_id','=',location),('state','=','done')], context=context)
107 local_context = dict(context)110 local_context = dict(context)
108 local_context['raise-exception'] = False111 local_context['raise-exception'] = False
109 for move in move_obj.browse(cr, uid, move_ids, context=context):112 # To avoid running out of memory, process limited batches
110 lot_id = move.prodlot_id.id113 for i in range(0, len(all_move_ids), self.MAX_IMPORT_LINES):
111 prod_id = move.product_id.id114 move_ids = all_move_ids[i * self.MAX_IMPORT_LINES:
112 if move.location_dest_id.id != move.location_id.id:115 (i + 1) * self.MAX_IMPORT_LINES]
113 if move.location_dest_id.id == location:116 for move in move_obj.browse(cr, uid, move_ids, context=context):
114 qty = uom_obj._compute_qty_obj(cr, uid, move.product_uom,move.product_qty, move.product_id.uom_id, context=local_context)117 lot_id = move.prodlot_id.id
115 else:118 prod_id = move.product_id.id
116 qty = -uom_obj._compute_qty_obj(cr, uid, move.product_uom,move.product_qty, move.product_id.uom_id, context=local_context)119 if move.location_dest_id.id != move.location_id.id:
117120 if move.location_dest_id.id == location:
118121 qty = uom_obj._compute_qty_obj(cr, uid, move.product_uom,move.product_qty, move.product_id.uom_id, context=local_context)
119 if datas.get((prod_id, lot_id)):122 else:
120 qty += datas[(prod_id, lot_id)]['product_qty']123 qty = -uom_obj._compute_qty_obj(cr, uid, move.product_uom,move.product_qty, move.product_id.uom_id, context=local_context)
121124
122 # Floating point sum could introduce tiny rounding errors :125
123 # Use the UoM API for the rounding (same UoM in & out).126 if datas.get((prod_id, lot_id)):
124 qty = uom_obj._compute_qty_obj(cr, uid,127 qty += datas[(prod_id, lot_id)]['product_qty']
125 move.product_id.uom_id, qty,128
126 move.product_id.uom_id)129 # Floating point sum could introduce tiny rounding errors :
127 datas[(prod_id, lot_id)] = {'product_id': prod_id, 'location_id': location, 'product_qty': qty, 'product_uom': move.product_id.uom_id.id, 'prod_lot_id': lot_id}130 # Use the UoM API for the rounding (same UoM in & out).
131 qty = uom_obj._compute_qty_obj(cr, uid,
132 move.product_id.uom_id, qty,
133 move.product_id.uom_id)
134 datas[(prod_id, lot_id)] = {'product_id': prod_id, 'location_id': location, 'product_qty': qty, 'product_uom': move.product_id.uom_id.id, 'prod_lot_id': lot_id}
128135
129 if datas:136 if datas:
130 flag = True137 flag = True