Merge lp:~openerp-dev/openobject-addons/6.0-bug-770243-odo into lp:openobject-addons/6.0

Proposed by Olivier Dony (Odoo)
Status: Merged
Merged at revision: 4753
Proposed branch: lp:~openerp-dev/openobject-addons/6.0-bug-770243-odo
Merge into: lp:openobject-addons/6.0
Diff against target: 206 lines (+96/-84)
1 file modified
stock/stock.py (+96/-84)
To merge this branch: bzr merge lp:~openerp-dev/openobject-addons/6.0-bug-770243-odo
Reviewer Review Type Date Requested Status
OpenERP Publisher's Warranty Team Pending
Review via email: mp+63116@code.launchpad.net

Description of the change

This branch fixes the performance issues that happen during reservation of stock.moves for databases with a large number of stock locations.

The locking and reservation system have been streamlined and are a lot more efficient in terms of iterations and database queries.

Support team should review and merge this for OPW.

Thanks!

To post a comment you must log in.
Revision history for this message
Jay Vora (Serpent Consulting Services) (jayvora) wrote :

Olivier,

Thanks for proposing merge for one of most awaited bugs.

Sorry to say, but somehow the branch contains some conflicts.

Can you recheck?

Thanks.

Revision history for this message
Jay Vora (Serpent Consulting Services) (jayvora) wrote :

Interestingly,

The diff of the merge and the commit differ heavily.
http://bazaar.launchpad.net/~openerp-dev/openobject-addons/6.0-bug-770243-odo/revision/4574

I guess its a bug or I am missing something!?!

Thanks.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'stock/stock.py'
--- stock/stock.py 2011-07-25 09:53:22 +0000
+++ stock/stock.py 2011-07-27 11:21:46 +0000
@@ -359,106 +359,118 @@
359 def _product_virtual_get(self, cr, uid, id, product_ids=False, context=None, states=['done']):359 def _product_virtual_get(self, cr, uid, id, product_ids=False, context=None, states=['done']):
360 return self._product_all_get(cr, uid, id, product_ids, context, ['confirmed', 'waiting', 'assigned', 'done'])360 return self._product_all_get(cr, uid, id, product_ids, context, ['confirmed', 'waiting', 'assigned', 'done'])
361361
362 def _try_lock_product_reserve(self, cr, uid, location_ids, product_id, product_qty, context=None):
363 try:
364 # Must lock with a separate select query than the ones used in _product_reserve
365 # because FOR UPDATE can't be used with aggregation/group by's
366 # (i.e. when individual rows aren't identifiable).
367 # We use a SAVEPOINT to be able to rollback this part of the transaction without
368 # failing the whole transaction in case the LOCK cannot be acquired.
369 cr.execute("SAVEPOINT stock_location_product_reserve")
370 # We lock all stock moves in states we are going to consider in the
371 # calculation. By locking all DONE move we prevent other transactions
372 # from reserving the same products, as they won't be allowed to SELECT
373 # them until we're done.
374 cr.execute("""SELECT id FROM stock_move
375 WHERE product_id=%s
376 AND (
377 (location_dest_id IN %s AND state = 'done')
378 OR
379 (location_id IN %s AND state in ('done', 'assigned'))
380 )
381 FOR UPDATE of stock_move NOWAIT""",
382 (product_id, location_ids, location_ids), log_exceptions=False)
383 except Exception:
384 # Here it's likely that the FOR UPDATE NOWAIT failed to get the LOCK,
385 # so we ROLLBACK to the SAVEPOINT to restore the transaction to its earlier
386 # state, we return False as if the products were not available, and log it:
387 cr.execute("ROLLBACK TO stock_location_product_reserve")
388 logger = logging.getLogger('stock.location')
389 logger.warn("Failed attempt to reserve %s x product %s, likely due to another transaction already in progress. Next attempt is likely to work. Detailed error available at DEBUG level.", product_qty, product_id)
390 logger.debug("Trace of the failed product reservation attempt: ", exc_info=True)
391 return False
392 return True
393
362 def _product_reserve(self, cr, uid, ids, product_id, product_qty, context=None, lock=False):394 def _product_reserve(self, cr, uid, ids, product_id, product_qty, context=None, lock=False):
363 """395 """
364 Attempt to find a quantity ``product_qty`` (in the product's default uom or the uom passed in ``context``) of product ``product_id``396 Attempt to find a quantity ``product_qty`` (in the product's default uom or the uom passed in ``context``) of product ``product_id``
365 in locations with id ``ids`` and their child locations. If ``lock`` is True, the stock.move lines397 in locations with id ``ids`` and their child locations. If ``lock`` is True, the stock.move lines
366 of product with id ``product_id`` in the searched location will be write-locked using Postgres's398 of product with id ``product_id`` in the searched location will be write-locked using Postgres's
367 "FOR UPDATE NOWAIT" option until the transaction is committed or rolled back, to prevent reservin399 "FOR UPDATE NOWAIT" option until the transaction is committed or rolled back, to prevent reserving
368 twice the same products.400 twice the same products.
369 If ``lock`` is True and the lock cannot be obtained (because another transaction has locked some of401 If ``lock`` is True and the lock cannot be obtained (because another transaction has locked some of
370 the same stock.move lines), a log line will be output and False will be returned, as if there was402 the same stock.move lines), a log line will be output and False will be returned, as if there was
371 not enough stock.403 not enough stock.
372404
373 :param product_id: Id of product to reserve405 :param product_id: Id of product to reserve
374 :param product_qty: Quantity of product to reserve (in the product's default uom or the uom passed in ``context``)406 :param product_qty: Quantity of product to reserve (in the uom passed in ``context``)
375 :param lock: if True, the stock.move lines of product with id ``product_id`` in all locations (and children locations) with ``ids`` will407 :param lock: if True, the stock.move lines of product with id ``product_id`` in all locations (and children locations) with ``ids`` will
376 be write-locked using postgres's "FOR UPDATE NOWAIT" option until the transaction is committed or rolled back. This is408 be write-locked using postgres's "FOR UPDATE NOWAIT" option until the transaction is committed or rolled back. This is
377 to prevent reserving twice the same products.409 to prevent reserving twice the same products.
378 :param context: optional context dictionary: it a 'uom' key is present it will be used instead of the default product uom to410 :param context: context dictionary with 'uom' key mapped to the ID of the UoM to use to compute the product quantities
379 compute the ``product_qty`` and in the return value.411 :return: List of pairs (qty, location_id) with the (partial) quantities that can be taken in each location to
380 :return: List of tuples in the form (qty, location_id) with the (partial) quantities that can be taken in each location to412 reach the requested product_qty (expressed in the requested uom), or False if not enough
381 reach the requested product_qty (``qty`` is expressed in the default uom of the product), of False if enough413 products could be found, or the lock could not be obtained (and ``lock`` was True).
382 products could not be found, or the lock could not be obtained (and ``lock`` was True).414 sum(qty) == ``product_qty``.
383 """415 """
416 if context is None:
417 context = {}
418 location_ids = self.search(cr, uid, [('location_id', 'child_of', ids)])
419 locations_tuple = tuple(location_ids)
420 if lock and not self._try_lock_product_reserve(cr, uid, locations_tuple, product_id, product_qty, context=context):
421 return False
422
423 # Giant query to obtain triplets of (product_uom, product_qty, location_id) summing all relevant
424 # stock moves quantities per location, with incoming quantities taken positive,
425 # and outgoing taken negative.
426 cr.execute("""SELECT x.product_uom, SUM(x.coeff * x.product_qty) as product_qty, x.loc_id as location_id
427 FROM (
428 SELECT 1.0 as coeff, product_uom, location_dest_id as loc_id,
429 sum(product_qty) AS product_qty
430 FROM stock_move
431 WHERE location_dest_id in %s AND
432 location_id != location_dest_id AND
433 product_id = %s AND
434 state = 'done'
435 GROUP BY location_dest_id, product_uom
436 UNION
437 SELECT -1.0 as coeff, product_uom, location_id as loc_id,
438 sum(product_qty) AS product_qty
439 FROM stock_move
440 WHERE location_id in %s AND
441 location_id != location_dest_id AND
442 product_id = %s AND
443 state in ('done', 'assigned')
444 GROUP BY location_id, product_uom
445 ) AS x
446 GROUP BY x.loc_id, x.product_uom
447 """,
448 (locations_tuple, product_id, locations_tuple, product_id))
449 sum_rows = cr.fetchall()
450
451 qty_by_location = {}
452 ProductUom = self.pool.get('product.uom')
453 target_uom = context.get('uom')
454 # Convert all UoMs into target UoM
455 for uom_id, qty, loc_id in sum_rows:
456 qty_by_location.setdefault(loc_id,0.0)
457 qty_by_location[loc_id] += ProductUom._compute_qty(cr, uid, uom_id, qty, target_uom)
458
459 # to compute final result we handle locations in the
460 # order in which they were returned by the original search().
384 result = []461 result = []
385 amount = 0.0462 for loc_id in location_ids:
386 if context is None:463 if loc_id not in qty_by_location:
387 context = {}464 #skip location without this product
388 pool_uom = self.pool.get('product.uom')465 continue
389 for id in self.search(cr, uid, [('location_id', 'child_of', ids)]):466 qty = qty_by_location[loc_id]
390 if lock:467 if qty <= 0.0:
391 try:468 continue
392 # Must lock with a separate select query because FOR UPDATE can't be used with469 qty = min(product_qty, qty)
393 # aggregation/group by's (when individual rows aren't identifiable).470 result.append((qty, loc_id))
394 # We use a SAVEPOINT to be able to rollback this part of the transaction without471 product_qty -= qty
395 # failing the whole transaction in case the LOCK cannot be acquired.472 if product_qty <= 0.0:
396 cr.execute("SAVEPOINT stock_location_product_reserve")473 return result
397 cr.execute("""SELECT id FROM stock_move
398 WHERE product_id=%s AND
399 (
400 (location_dest_id=%s AND
401 location_id<>%s AND
402 state='done')
403 OR
404 (location_id=%s AND
405 location_dest_id<>%s AND
406 state in ('done', 'assigned'))
407 )
408 FOR UPDATE of stock_move NOWAIT""", (product_id, id, id, id, id), log_exceptions=False)
409 except Exception:
410 # Here it's likely that the FOR UPDATE NOWAIT failed to get the LOCK,
411 # so we ROLLBACK to the SAVEPOINT to restore the transaction to its earlier
412 # state, we return False as if the products were not available, and log it:
413 cr.execute("ROLLBACK TO stock_location_product_reserve")
414 logger = logging.getLogger('stock.location')
415 logger.warn("Failed attempt to reserve %s x product %s, likely due to another transaction already in progress. Next attempt is likely to work. Detailed error available at DEBUG level.", product_qty, product_id)
416 logger.debug("Trace of the failed product reservation attempt: ", exc_info=True)
417 return False
418
419 # XXX TODO: rewrite this with one single query, possibly even the quantity conversion
420 cr.execute("""SELECT product_uom, sum(product_qty) AS product_qty
421 FROM stock_move
422 WHERE location_dest_id=%s AND
423 location_id<>%s AND
424 product_id=%s AND
425 state='done'
426 GROUP BY product_uom
427 """,
428 (id, id, product_id))
429 results = cr.dictfetchall()
430 cr.execute("""SELECT product_uom,-sum(product_qty) AS product_qty
431 FROM stock_move
432 WHERE location_id=%s AND
433 location_dest_id<>%s AND
434 product_id=%s AND
435 state in ('done', 'assigned')
436 GROUP BY product_uom
437 """,
438 (id, id, product_id))
439 results += cr.dictfetchall()
440 total = 0.0
441 results2 = 0.0
442
443 for r in results:
444 amount = pool_uom._compute_qty(cr, uid, r['product_uom'], r['product_qty'], context.get('uom', False))
445 results2 += amount
446 total += amount
447
448 if total <= 0.0:
449 continue
450
451 amount = results2
452 if amount > 0:
453 if amount > min(total, product_qty):
454 amount = min(product_qty, total)
455 result.append((amount, id))
456 product_qty -= amount
457 total -= amount
458 if product_qty <= 0.0:
459 return result
460 if total <= 0.0:
461 continue
462 return False474 return False
463475
464stock_location()476stock_location()