details: https://code.openbravo.com/erp/devel/pi/rev/5cd05d20d9cf changeset: 32603:5cd05d20d9cf user: Mark <markmm82 <at> gmail.com> date: Wed Aug 23 14:16:09 2017 -0400 summary: Fixes issue 36675: Transaction Cost not created for closing inventory lines
After cost was assigned to closing inventory lines transactions, when calling to insertTrxCosts() from calculateCosts() method (second call), CostingUtilsData.countTrxCosts(conn) was getting 0 in countTrx. It was caused because closing inventory lines cost calculation was done by OBDal connection, and methods invoked in CostingUtilsData are done using different connection provided by SQLC, and the transactions are not available at this moment. See: http://wiki.openbravo.com/wiki/Data_Access_Layer#Transaction_and_Session To find the best solution for this issue and don't affect the performance (see issue 35959), different solutions were tested to always use DAL and make process use the same connection. 1.- Doing a single insert-select query with limits is not supported in DAL. We tried it in two different ways (see attached TestCosting_v1.diff): - Use setMaxResults(). It only works for selects but not for inserts/updates. - Use setMaxResults() in select query and append it to insert query using getQueryString(). It appends the select without any limit. 2.- Another option could be to refactor insertTrxCosts() method to do a multiple insert query: We can create the select query with limits using setMaxResults(), iterate it using an scroll and create and save in each iteration a new TransactionCost. Flush won't be done in every iteration. When flush is done, it raises every insert in multiple single-row inserts instead of only one multiple-row insert. Single-row inserts performs worst than multiple-row insert, at least with not many rows (see attached TestCosting_v2.diff). 3.- Finally, we have refactor the process to avoid insertTrxCosts() method: Our solution makes an insertion in M_Transaction_Cost table each time we make the updation of related transaction cost in M_Transaction table. We will do single-row inserts in two methods: updateTrxLegacyCosts() and calculateCosts(), and multiple-row insert in one method: updateWithZeroCostRemainingTrx(). Thus, we split the number of TransactionCost records to be created in three different steps. details: https://code.openbravo.com/erp/devel/pi/rev/1dc42ad30d5c changeset: 32604:1dc42ad30d5c user: Alvaro Ferraz <alvaro.ferraz <at> openbravo.com> date: Mon Aug 28 14:45:28 2017 +0200 summary: Related to issue 36675: Fix some logs diffstat: src/org/openbravo/costing/CostingMigrationProcess.java | 218 +++++++++++----- src/org/openbravo/costing/CostingUtils_data.xsql | 62 ---- 2 files changed, 148 insertions(+), 132 deletions(-) diffs (truncated from 488 to 300 lines): diff -r 90a1381fae84 -r 1dc42ad30d5c src/org/openbravo/costing/CostingMigrationProcess.java --- a/src/org/openbravo/costing/CostingMigrationProcess.java Tue Aug 29 16:10:25 2017 +0200 +++ b/src/org/openbravo/costing/CostingMigrationProcess.java Mon Aug 28 14:45:28 2017 +0200 @@ -47,7 +47,6 @@ import org.openbravo.dal.service.OBCriteria; import org.openbravo.dal.service.OBDal; import org.openbravo.dal.service.OBQuery; -import org.openbravo.database.ConnectionProvider; import org.openbravo.erpCommon.ad_forms.ProductInfo; import org.openbravo.erpCommon.utility.OBDateUtils; import org.openbravo.erpCommon.utility.OBError; @@ -76,6 +75,8 @@ import org.openbravo.model.materialmgmt.transaction.InventoryCount; import org.openbravo.model.materialmgmt.transaction.InventoryCountLine; import org.openbravo.model.materialmgmt.transaction.MaterialTransaction; +import org.openbravo.model.materialmgmt.transaction.ShipmentInOut; +import org.openbravo.model.materialmgmt.transaction.ShipmentInOutLine; import org.openbravo.scheduling.Process; import org.openbravo.scheduling.ProcessBundle; import org.openbravo.scheduling.ProcessLogger; @@ -84,7 +85,6 @@ public class CostingMigrationProcess implements Process { private ProcessLogger logger; - private ConnectionProvider conn; private static final Logger log4j = Logger.getLogger(CostingMigrationProcess.class); private static CostingAlgorithm averageAlgorithm = null; private static final String alertRuleName = "Products with transactions without available cost on date."; @@ -97,7 +97,6 @@ private static final String valuedLegacy = "800088"; private static final String dimensionalLegacy = "800205"; private static final String processEntity = org.openbravo.model.ad.ui.Process.ENTITY_NAME; - private static final int maxTrx = 10000; @Override public void execute(ProcessBundle bundle) throws Exception { @@ -106,7 +105,6 @@ logger = bundle.getLogger(); OBError msg = new OBError(); - conn = bundle.getConnection(); msg.setType("Success"); msg.setTitle(OBMessageUtils.messageBD("Success")); try { @@ -392,7 +390,7 @@ } private void updateLegacyCosts() { - long start = System.currentTimeMillis(); + long start1 = System.currentTimeMillis(); log4j.debug("Starting updateLegacyCosts() at: " + new Date()); resetTransactionCosts(); @@ -406,36 +404,69 @@ client = OBDal.getInstance().get(Client.class, clientId); Currency clientCur = client.getCurrency(); int stdPrecission = clientCur.getStandardPrecision().intValue(); - log4j.debug("** Processing client: " + client.getIdentifier() + " with currency: " - + clientCur.getIdentifier()); for (Organization legalEntity : osp.getLegalEntitiesList()) { - log4j.debug("** Processing organization: " + legalEntity.getIdentifier()); + long start2 = System.currentTimeMillis(); + log4j.debug("Starting updateTrxLegacyCosts() at: " + new Date() + " for client: " + + client.getName() + " and organization: " + legalEntity.getName()); + Set<String> naturalTree = osp.getNaturalTree(legalEntity.getId()); ScrollableResults legacyCosts = getLegacyCostScroll(clientId, naturalTree); int i = 0; + + int pendingCosts = 0; + int batchSize = 100; + long t1 = 0; + long elapsedTime = 0; + if (log4j.isDebugEnabled()) { + pendingCosts = getLegacyCostCount(clientId, naturalTree); + t1 = System.currentTimeMillis(); + log4j.debug("Pending costs: " + pendingCosts); + } + try { while (legacyCosts.next()) { Costing cost = (Costing) legacyCosts.get(0); updateTrxLegacyCosts(cost, stdPrecission, naturalTree); - if ((++i % 100) == 0) { + if (++i % batchSize == 0) { OBDal.getInstance().flush(); OBDal.getInstance().getSession().clear(); + + if (log4j.isDebugEnabled()) { + long t2 = System.currentTimeMillis(); + long t = t2 - t1; + t1 = System.currentTimeMillis(); + int batch = i / batchSize; + pendingCosts -= batchSize; + int pendingBatches = (pendingCosts % batchSize == 0) ? (pendingCosts / batchSize) + : (pendingCosts / batchSize) + 1; + elapsedTime += t; + long avgTimePerBatch = elapsedTime / batch / 1000; + log4j.debug("Processing batch: " + batch + " (" + batchSize + " costs) took: " + t + + " ms."); + log4j.debug("Pending costs: " + pendingCosts); + log4j.debug("Average time per batch: " + avgTimePerBatch + + " seconds. Estimated time to finish: " + (avgTimePerBatch * pendingBatches) + + " seconds."); + } } } } finally { legacyCosts.close(); } SessionHandler.getInstance().commitAndStart(); + + long end2 = System.currentTimeMillis(); + log4j.debug("Ending updateTrxLegacyCosts() at: " + new Date() + ". Duration: " + + (end2 - start2) + " ms."); } } updateWithZeroCostRemainingTrx(); - insertTrxCosts(); insertStandardCosts(); - long end = System.currentTimeMillis(); - log4j.debug("Ending updateLegacyCosts() at: " + new Date() + ". Duration: " + (end - start) + long end1 = System.currentTimeMillis(); + log4j.debug("Ending updateLegacyCosts() at: " + new Date() + ". Duration: " + (end1 - start1) + " ms."); } @@ -597,6 +628,18 @@ trx.setCostingStatus("CC"); trx.setProcessed(true); OBDal.getInstance().save(trx); + + TransactionCost tc = OBProvider.getInstance().get(TransactionCost.class); + tc.setClient(trx.getClient()); + tc.setOrganization(trx.getOrganization()); + tc.setInventoryTransaction(trx); + tc.setCost(trx.getTransactionCost()); + tc.setCostDate(trx.getTransactionProcessDate()); + tc.setCurrency(trx.getCurrency()); + tc.setAccountingDate(trx.getGoodsShipmentLine() != null ? trx.getGoodsShipmentLine() + .getShipmentReceipt().getAccountingDate() : trx.getMovementDate()); + OBDal.getInstance().save(tc); + Currency legalEntityCur = FinancialUtils.getLegalEntityCurrency(trx.getOrganization()); BigDecimal cost = BigDecimal.ZERO; if (BigDecimal.ZERO.compareTo(trx.getMovementQuantity()) != 0) { @@ -616,23 +659,21 @@ // MovementQty is already negative so add to totalStock to decrease it. totalStock = totalStock.add(trx.getMovementQuantity()); - if ((i % 100) == 0) { + if (++i % 100 == 0) { OBDal.getInstance().flush(); OBDal.getInstance().getSession().clear(); cur = OBDal.getInstance().get(Currency.class, curId); } - i++; } } finally { icls.close(); } OBDal.getInstance().flush(); - insertTrxCosts(); long end = System.currentTimeMillis(); log4j.debug("Ending calculateCosts() at: " + new Date() + ". Duration: " + (end - start) - + " ms."); + + " ms. Updated: " + i + " transactions."); } private HashMap<String, BigDecimal> getCurrentValuedStock(String productId, String curId, @@ -756,10 +797,31 @@ return costingQry.scroll(ScrollMode.FORWARD_ONLY); } + private int getLegacyCostCount(String clientId, Set<String> naturalTree) { + StringBuffer where = new StringBuffer(); + where.append(" select count(c." + Costing.PROPERTY_ID + ") "); + where.append(" from " + Costing.ENTITY_NAME + " as c"); + where.append(" where c." + Costing.PROPERTY_CLIENT + ".id = :client"); + where.append(" and exists (select 1 from " + MaterialTransaction.ENTITY_NAME + " as trx"); + where.append(" where trx." + MaterialTransaction.PROPERTY_ORGANIZATION + ".id in (:orgs)"); + where.append(" and trx." + MaterialTransaction.PROPERTY_TRANSACTIONCOST + " is null"); + where.append(" and trx." + MaterialTransaction.PROPERTY_MOVEMENTDATE + " >= c." + + Costing.PROPERTY_STARTINGDATE); + where.append(" and trx." + MaterialTransaction.PROPERTY_PRODUCT + " = c." + + Costing.PROPERTY_PRODUCT); + where.append(" and trx." + MaterialTransaction.PROPERTY_MOVEMENTDATE + " -1 < (c." + + Costing.PROPERTY_ENDINGDATE + ") "); + where.append(" )"); + where.append(" and " + Costing.PROPERTY_COST + " is not null"); + + Query costingQry = OBDal.getInstance().getSession().createQuery(where.toString()); + costingQry.setParameter("client", clientId); + costingQry.setParameterList("orgs", naturalTree); + costingQry.setMaxResults(1); + return ((Long) costingQry.uniqueResult()).intValue(); + } + private void updateTrxLegacyCosts(Costing _cost, int standardPrecision, Set<String> naturalTree) { - long start = System.currentTimeMillis(); - log4j.debug("Starting updateTrxLegacyCosts() at: " + new Date()); - Costing cost = OBDal.getInstance().get(Costing.class, _cost.getId()); StringBuffer where = new StringBuffer(); where.append(MaterialTransaction.PROPERTY_PRODUCT + ".id = :product"); @@ -782,11 +844,10 @@ try { while (trxs.next()) { MaterialTransaction trx = (MaterialTransaction) trxs.get(0); - log4j.debug("********** UpdateTrxLegacyCosts process trx:" + trx.getIdentifier()); + Date accountingDate = trx.getGoodsShipmentLine() != null ? trx.getGoodsShipmentLine() + .getShipmentReceipt().getAccountingDate() : trx.getMovementDate(); - if (trx.getGoodsShipmentLine() != null - && trx.getGoodsShipmentLine().getShipmentReceipt().getAccountingDate() - .compareTo(trx.getMovementDate()) != 0) { + if (accountingDate.compareTo(trx.getMovementDate()) != 0) { // Shipments with accounting date different than the movement date gets the cost valid on // the accounting date. BigDecimal unitCost = new BigDecimal(new ProductInfo(cost.getProduct().getId(), @@ -795,32 +856,36 @@ new DalConnectionProvider(false), OBDal.getInstance().getConnection())); BigDecimal trxCost = unitCost.multiply(trx.getMovementQuantity().abs()).setScale( standardPrecision, BigDecimal.ROUND_HALF_UP); - trx.setTransactionCost(trxCost); } else { trx.setTransactionCost(cost.getCost().multiply(trx.getMovementQuantity().abs()) .setScale(standardPrecision, BigDecimal.ROUND_HALF_UP)); } - trx.setCurrency(cost.getCurrency()); trx.setCostCalculated(true); trx.setCostingStatus("CC"); trx.setProcessed(true); + OBDal.getInstance().save(trx); - if ((i % 100) == 0) { + TransactionCost tc = OBProvider.getInstance().get(TransactionCost.class); + tc.setClient(trx.getClient()); + tc.setOrganization(trx.getOrganization()); + tc.setInventoryTransaction(trx); + tc.setCost(trx.getTransactionCost()); + tc.setCostDate(trx.getTransactionProcessDate()); + tc.setCurrency(trx.getCurrency()); + tc.setAccountingDate(accountingDate); + OBDal.getInstance().save(tc); + + if (++i % 100 == 0) { OBDal.getInstance().flush(); OBDal.getInstance().getSession().clear(); cost = OBDal.getInstance().get(Costing.class, cost.getId()); } - i++; } } finally { trxs.close(); } - - long end = System.currentTimeMillis(); - log4j.debug("Ending updateTrxLegacyCosts() at: " + new Date() + ". Duration: " + (end - start) - + " ms. Updated: " + i + " transactions."); } /** @@ -833,7 +898,8 @@ long start = System.currentTimeMillis(); log4j.debug("Starting updateWithZeroCostRemainingTrx() at: " + new Date()); - int n = 0; + int n1 = 0; + int n2 = 0; TriggerHandler.getInstance().disable(); try { @@ -843,6 +909,47 @@ for (Organization org : osp.getLegalEntitiesList()) { final Set<String> childOrgs = osp.getChildTree(org.getId(), true); + StringBuffer insert = new StringBuffer(); + insert.append(" insert into " + TransactionCost.ENTITY_NAME); + insert.append(" (" + TransactionCost.PROPERTY_ID); + insert.append(", " + TransactionCost.PROPERTY_CLIENT); + insert.append(", " + TransactionCost.PROPERTY_ORGANIZATION); + insert.append(", " + TransactionCost.PROPERTY_CREATIONDATE); + insert.append(", " + TransactionCost.PROPERTY_CREATEDBY); + insert.append(", " + TransactionCost.PROPERTY_UPDATED); + insert.append(", " + TransactionCost.PROPERTY_UPDATEDBY); + insert.append(", " + TransactionCost.PROPERTY_ACTIVE); + insert.append(", " + TransactionCost.PROPERTY_INVENTORYTRANSACTION); + insert.append(", " + TransactionCost.PROPERTY_COST); + insert.append(", " + TransactionCost.PROPERTY_COSTDATE); + insert.append(", " + TransactionCost.PROPERTY_CURRENCY); + insert.append(", " + TransactionCost.PROPERTY_ACCOUNTINGDATE); + insert.append(")"); ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Openbravo-commits mailing list Openbravo-commits@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openbravo-commits
