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

[Loïc Bellier - Numérigraphe]

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

[Lionel Sausin - Initiatives/Numérigraphe (ls-initiatives)]

review: Approve

[Holger Brunn (Therp) (hbrunn)]

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)

[Lionel Sausin - Initiatives/Numérigraphe (ls-initiatives)]

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?

[Holger Brunn (Therp) (hbrunn)]

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

[Holger Brunn (Therp) (hbrunn)]

review: Approve (code review)

[Lionel Sausin - Initiatives/Numérigraphe (ls-initiatives)]

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)

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

LGTM.

Thanks!

review: Approve (code review, test)

[Lionel Sausin - Initiatives/Numérigraphe (ls-initiatives)]

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

[Laurent Mignon (Acsone) (lmi)]

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)

=== 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 @@
     _name = "stock.fill.inventory"
     _description = "Import Inventory"
 
+    # Maximum size of a batch of lines we can import without risking OOM
+    MAX_IMPORT_LINES = 10000
+
     def _default_location(self, cr, uid, ids, context=None):
         try:
             location = self.pool.get('ir.model.data').get_object(cr, uid, 'stock', 'stock_location_stock')
@@ -103,28 +106,32 @@
         for location in location_ids:
             datas = {}
             res[location] = {}
-            move_ids = move_obj.search(cr, uid, ['|',('location_dest_id','=',location),('location_id','=',location),('state','=','done')], context=context)
+            all_move_ids = move_obj.search(cr, uid, ['|',('location_dest_id','=',location),('location_id','=',location),('state','=','done')], context=context)
             local_context = dict(context)
             local_context['raise-exception'] = False
-            for move in move_obj.browse(cr, uid, move_ids, context=context):
-                lot_id = move.prodlot_id.id
-                prod_id = move.product_id.id
-                if move.location_dest_id.id != move.location_id.id:
-                    if move.location_dest_id.id == location:
-                        qty = uom_obj._compute_qty_obj(cr, uid, move.product_uom,move.product_qty, move.product_id.uom_id, context=local_context)
-                    else:
-                        qty = -uom_obj._compute_qty_obj(cr, uid, move.product_uom,move.product_qty, move.product_id.uom_id, context=local_context)
-
-
-                    if datas.get((prod_id, lot_id)):
-                        qty += datas[(prod_id, lot_id)]['product_qty']
-
-                    # Floating point sum could introduce tiny rounding errors :
-                    #     Use the UoM API for the rounding (same UoM in & out).
-                    qty = uom_obj._compute_qty_obj(cr, uid,
-                                                   move.product_id.uom_id, qty,
-                                                   move.product_id.uom_id)
-                    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}
+            # To avoid running out of memory, process limited batches
+            for i in range(0, len(all_move_ids), self.MAX_IMPORT_LINES):
+                move_ids = all_move_ids[i * self.MAX_IMPORT_LINES:
+                                        (i + 1) * self.MAX_IMPORT_LINES]
+                for move in move_obj.browse(cr, uid, move_ids, context=context):
+                    lot_id = move.prodlot_id.id
+                    prod_id = move.product_id.id
+                    if move.location_dest_id.id != move.location_id.id:
+                        if move.location_dest_id.id == location:
+                            qty = uom_obj._compute_qty_obj(cr, uid, move.product_uom,move.product_qty, move.product_id.uom_id, context=local_context)
+                        else:
+                            qty = -uom_obj._compute_qty_obj(cr, uid, move.product_uom,move.product_qty, move.product_id.uom_id, context=local_context)
+
+
+                        if datas.get((prod_id, lot_id)):
+                            qty += datas[(prod_id, lot_id)]['product_qty']
+
+                        # Floating point sum could introduce tiny rounding errors :
+                        #     Use the UoM API for the rounding (same UoM in & out).
+                        qty = uom_obj._compute_qty_obj(cr, uid,
+                                                       move.product_id.uom_id, qty,
+                                                       move.product_id.uom_id)
+                        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}
 
             if datas:
                 flag = True