Re: svn commit: r1635192 - in /ofbiz/trunk/applications: order/servicedef/services.xml order/src/org/ofbiz/order/orde
Locked
Re: svn commit: r1635192 - in /ofbiz/trunk/applications: order/servicedef/services.xml order/src/org/ofbiz/order/orde
 
|
Jacques,
what was the measured difference between the method call to the Java method and the service call to the Java implementation? Jacopo On Oct 29, 2014, at 5:37 PM, [hidden email] wrote: > Author: jleroux > Date: Wed Oct 29 16:37:40 2014 > New Revision: 1635192 > > URL: http://svn.apache.org/r1635192 > Log: > The storeOrder service, implemented by the OrderService.createOrder() method, synchronously calls the countProductQuantityOrdered service implemented in Minilang. OOTB, the countProductQuantityOrdered service is only called by the storeOrder service implementation. It's called inside a loop on orderItems. > > While intentionally load testing a custom project with JMeter on a weak m1.small AWS machine, I noticed the overhead of the service call for each order item iteration was significant on this slow machine. With only 30 concurrent users the process was blocked by a timeout on the countProductQuantityOrdered service. > > I then decided to transform the minilang code into an OrderService.countProductQuantityOrdered() method that can be directly called inside OrderService.createOrder() and also implements the countProductQuantityOrdered service. Hence it avoids the overhead of the minilang service calls in loop while still providing a countProductQuantityOrdered service for possible external uses. For that I moved the definition from Product component to Order component to avoid the hard coded dependency of Order to Product. I then did not cross issues with the same load test. > > I also added a test for the countProductQuantityOrdered service by adding an order item of a virtual product (GZ-1006-1) in SalesOrderTest. > > Modified: > ofbiz/trunk/applications/order/servicedef/services.xml > ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java > ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java > ofbiz/trunk/applications/product/servicedef/services.xml > > Modified: ofbiz/trunk/applications/order/servicedef/services.xml > URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/servicedef/services.xml?rev=1635192&r1=1635191&r2=1635192&view=diff > ============================================================================== > --- ofbiz/trunk/applications/order/servicedef/services.xml (original) > +++ ofbiz/trunk/applications/order/servicedef/services.xml Wed Oct 29 16:37:40 2014 > @@ -1147,4 +1147,11 @@ under the License. > <auto-attributes mode="IN" entity-name="OrderItemGroupOrder" include="pk" optional="false"/> > </service> > > + <service name="countProductQuantityOrdered" engine="java" > + location="org.ofbiz.order.order.OrderServices" invoke="countProductQuantityOrdered" auth="true"> > + <description>count Product Quantity Ordered</description> > + <attribute name="productId" type="String" mode="IN" optional="false"/> > + <attribute name="quantity" type="BigDecimal" mode="IN" optional="false"/> > + </service> > + > </services> > > Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java > URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java?rev=1635192&r1=1635191&r2=1635192&view=diff > ============================================================================== > --- ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java (original) > +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java Wed Oct 29 16:37:40 2014 > @@ -280,15 +280,10 @@ public class OrderServices { > normalizedItemQuantities.put(currentProductId, currentQuantity.add(orderItem.getBigDecimal("quantity"))); > } > > - try { > - // count product ordered quantities > - // run this synchronously so it will run in the same transaction > - dispatcher.runSync("countProductQuantityOrdered", UtilMisc.<String, Object>toMap("productId", currentProductId, "quantity", orderItem.getBigDecimal("quantity"), "userLogin", userLogin)); > - } catch (GenericServiceException e1) { > - Debug.logError(e1, "Error calling countProductQuantityOrdered service", module); > - return ServiceUtil.returnError(UtilProperties.getMessage(resource_error, > - "OrderErrorCallingCountProductQuantityOrderedService",locale) + e1.toString()); > - } > + Map<String, Object> countContext = new HashMap<String, Object>(); > + countContext.put("productId", currentProductId); > + countContext.put("quantity", orderItem.getBigDecimal("quantity")); > + countProductQuantityOrdered(ctx, countContext); > } > } > > @@ -1152,6 +1147,54 @@ public class OrderServices { > > return successResult; > } > + > + public static Map<String, Object> countProductQuantityOrdered(DispatchContext ctx, Map<String, Object> context) { > + Delegator delegator = ctx.getDelegator(); > + Locale locale = (Locale) context.get("locale"); > + List<GenericValue> productCalculatedInfoList = null; > + GenericValue productCalculatedInfo = null; > + String productId = (String) context.get("productId"); > + BigDecimal quantity = (BigDecimal) context.get("quantity"); > + try { > + productCalculatedInfoList = delegator.findByAnd("ProductCalculatedInfo", UtilMisc.toMap("productId", productId), null, false); > + if (UtilValidate.isEmpty(productCalculatedInfoList)) { > + productCalculatedInfo = delegator.makeValue("ProductCalculatedInfo"); > + productCalculatedInfo.set("productId", productId); > + productCalculatedInfo.set("totalQuantityOrdered", quantity); > + productCalculatedInfo.create(); > + } else { > + productCalculatedInfo = productCalculatedInfoList.get(0); > + BigDecimal totalQuantityOrdered = productCalculatedInfo.getBigDecimal("totalQuantityOrdered"); > + if (totalQuantityOrdered == null) { > + productCalculatedInfo.set("totalQuantityOrdered", quantity); > + } else { > + productCalculatedInfo.set("totalQuantityOrdered", totalQuantityOrdered.add(quantity)); > + } > + } > + productCalculatedInfo.store(); > + } catch (GenericEntityException e) { > + Debug.logError(e, "Error calling countProductQuantityOrdered service", module); > + return ServiceUtil.returnError(UtilProperties.getMessage(resource_error, > + "OrderErrorCallingCountProductQuantityOrderedService",locale) + e.toString()); > + > + } > + > + String virtualProductId = null; > + try { > + GenericValue product = delegator.findOne("Product", UtilMisc.toMap("productId", productId), true); > + virtualProductId = ProductWorker.getVariantVirtualId(product); > + } catch (GenericEntityException e) { > + Debug.logError(e, "Error calling countProductQuantityOrdered service", module); > + return ServiceUtil.returnError(UtilProperties.getMessage(resource_error, > + "OrderErrorCallingCountProductQuantityOrderedService",locale) + e.toString()); > + } > + > + if (UtilValidate.isNotEmpty(virtualProductId)) { > + context.put("productId", virtualProductId); > + countProductQuantityOrdered(ctx, context); > + } > + return ServiceUtil.returnSuccess(); > + } > > public static void reserveInventory(Delegator delegator, LocalDispatcher dispatcher, GenericValue userLogin, Locale locale, List<GenericValue> orderItemShipGroupInfo, List<String> dropShipGroupIds, Map<String, GenericValue> itemValuesBySeqId, String orderTypeId, String productStoreId, List<String> resErrorMessages) throws GeneralException { > boolean isImmediatelyFulfilled = false; > @@ -1729,7 +1772,7 @@ public class OrderServices { > if (UtilValidate.isNotEmpty(orderItemSeqId)) { > createOrderAdjContext.put("orderItemSeqId", orderItemSeqId); > } else { > - createOrderAdjContext.put("orderItemSeqId", "_NA_"); > + createOrderAdjContext.put("orderItemSeqId", "_NA_"); > } > createOrderAdjContext.put("shipGroupSeqId", "_NA_"); > createOrderAdjContext.put("description", "Tax adjustment due to order change"); > @@ -5787,4 +5830,4 @@ public class OrderServices { > > return ServiceUtil.returnSuccess(); > } > -} > \ No newline at end of file > +} > > Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java > URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java?rev=1635192&r1=1635191&r2=1635192&view=diff > ============================================================================== > --- ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java (original) > +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java Wed Oct 29 16:37:40 2014 > @@ -114,8 +114,16 @@ public class SalesOrderTest extends OFBi > orderItem.set("unitPrice", new BigDecimal("38.4")); > orderItem.set("unitListPrice", new BigDecimal("48.0")); > orderItem.set("statusId", "ITEM_CREATED"); > - > orderItems.add(orderItem); > + > + orderItem = delegator.makeValue("OrderItem", UtilMisc.toMap("orderItemSeqId", "00002", "orderItemTypeId", "PRODUCT_ORDER_ITEM", "prodCatalogId", "DemoCatalog", "productId", "GZ-1006-1", "quantity", BigDecimal.ONE, "selectedAmount", BigDecimal.ZERO)); > + orderItem.set("isPromo", "N"); > + orderItem.set("isModifiedPrice", "N"); > + orderItem.set("unitPrice", new BigDecimal("1.99")); > + orderItem.set("unitListPrice", new BigDecimal("5.99")); > + orderItem.set("statusId", "ITEM_CREATED"); > + orderItems.add(orderItem); > + > ctx.put("orderItems", orderItems); > > List<GenericValue> orderTerms = FastList.newInstance(); > > Modified: ofbiz/trunk/applications/product/servicedef/services.xml > URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/servicedef/services.xml?rev=1635192&r1=1635191&r2=1635192&view=diff > ============================================================================== > --- ofbiz/trunk/applications/product/servicedef/services.xml (original) > +++ ofbiz/trunk/applications/product/servicedef/services.xml Wed Oct 29 16:37:40 2014 > @@ -175,13 +175,7 @@ under the License. > <attribute name="productId" type="String" mode="IN" optional="false"/> > <attribute name="weight" type="Long" mode="IN" optional="true"/> > </service> > - <service name="countProductQuantityOrdered" engine="simple" > - location="component://product/script/org/ofbiz/product/product/ProductServices.xml" invoke="countProductQuantityOrdered" auth="true"> > - <description>count Product Quantity Ordered</description> > - <attribute name="productId" type="String" mode="IN" optional="false"/> > - <attribute name="quantity" type="BigDecimal" mode="IN" optional="false"/> > - </service> > - > + > <service name="createProductReview" engine="simple" > location="component://product/script/org/ofbiz/product/product/ProductServices.xml" invoke="createProductReview" auth="true"> > <description>Create a product review entity</description> > > |
Re: svn commit: r1635192 - in /ofbiz/trunk/applications: order/servicedef/services.xml order/src/org/ofbiz/order/orde
|
|
I see a serious problem with this commit:
Map<String, Object> countContext = new HashMap<String, Object>(); is wrong. Use the DispatchContext to build the service context from the existing context Map - so the framework-inserted artifacts are included in the Map. Adrian Crum Sandglass Software www.sandglass-software.com On 10/29/2014 5:50 PM, Jacopo Cappellato wrote: > Jacques, > > what was the measured difference between the method call to the Java method and the service call to the Java implementation? > > Jacopo > > On Oct 29, 2014, at 5:37 PM, [hidden email] wrote: > >> Author: jleroux >> Date: Wed Oct 29 16:37:40 2014 >> New Revision: 1635192 >> >> URL: http://svn.apache.org/r1635192 >> Log: >> The storeOrder service, implemented by the OrderService.createOrder() method, synchronously calls the countProductQuantityOrdered service implemented in Minilang. OOTB, the countProductQuantityOrdered service is only called by the storeOrder service implementation. It's called inside a loop on orderItems. >> >> While intentionally load testing a custom project with JMeter on a weak m1.small AWS machine, I noticed the overhead of the service call for each order item iteration was significant on this slow machine. With only 30 concurrent users the process was blocked by a timeout on the countProductQuantityOrdered service. >> >> I then decided to transform the minilang code into an OrderService.countProductQuantityOrdered() method that can be directly called inside OrderService.createOrder() and also implements the countProductQuantityOrdered service. Hence it avoids the overhead of the minilang service calls in loop while still providing a countProductQuantityOrdered service for possible external uses. For that I moved the definition from Product component to Order component to avoid the hard coded dependency of Order to Product. I then did not cross issues with the same load test. >> >> I also added a test for the countProductQuantityOrdered service by adding an order item of a virtual product (GZ-1006-1) in SalesOrderTest. >> >> Modified: >> ofbiz/trunk/applications/order/servicedef/services.xml >> ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java >> ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java >> ofbiz/trunk/applications/product/servicedef/services.xml >> >> Modified: ofbiz/trunk/applications/order/servicedef/services.xml >> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/servicedef/services.xml?rev=1635192&r1=1635191&r2=1635192&view=diff >> ============================================================================== >> --- ofbiz/trunk/applications/order/servicedef/services.xml (original) >> +++ ofbiz/trunk/applications/order/servicedef/services.xml Wed Oct 29 16:37:40 2014 >> @@ -1147,4 +1147,11 @@ under the License. >> <auto-attributes mode="IN" entity-name="OrderItemGroupOrder" include="pk" optional="false"/> >> </service> >> >> + <service name="countProductQuantityOrdered" engine="java" >> + location="org.ofbiz.order.order.OrderServices" invoke="countProductQuantityOrdered" auth="true"> >> + <description>count Product Quantity Ordered</description> >> + <attribute name="productId" type="String" mode="IN" optional="false"/> >> + <attribute name="quantity" type="BigDecimal" mode="IN" optional="false"/> >> + </service> >> + >> </services> >> >> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java >> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java?rev=1635192&r1=1635191&r2=1635192&view=diff >> ============================================================================== >> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java (original) >> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java Wed Oct 29 16:37:40 2014 >> @@ -280,15 +280,10 @@ public class OrderServices { >> normalizedItemQuantities.put(currentProductId, currentQuantity.add(orderItem.getBigDecimal("quantity"))); >> } >> >> - try { >> - // count product ordered quantities >> - // run this synchronously so it will run in the same transaction >> - dispatcher.runSync("countProductQuantityOrdered", UtilMisc.<String, Object>toMap("productId", currentProductId, "quantity", orderItem.getBigDecimal("quantity"), "userLogin", userLogin)); >> - } catch (GenericServiceException e1) { >> - Debug.logError(e1, "Error calling countProductQuantityOrdered service", module); >> - return ServiceUtil.returnError(UtilProperties.getMessage(resource_error, >> - "OrderErrorCallingCountProductQuantityOrderedService",locale) + e1.toString()); >> - } >> + Map<String, Object> countContext = new HashMap<String, Object>(); >> + countContext.put("productId", currentProductId); >> + countContext.put("quantity", orderItem.getBigDecimal("quantity")); >> + countProductQuantityOrdered(ctx, countContext); >> } >> } >> >> @@ -1152,6 +1147,54 @@ public class OrderServices { >> >> return successResult; >> } >> + >> + public static Map<String, Object> countProductQuantityOrdered(DispatchContext ctx, Map<String, Object> context) { >> + Delegator delegator = ctx.getDelegator(); >> + Locale locale = (Locale) context.get("locale"); >> + List<GenericValue> productCalculatedInfoList = null; >> + GenericValue productCalculatedInfo = null; >> + String productId = (String) context.get("productId"); >> + BigDecimal quantity = (BigDecimal) context.get("quantity"); >> + try { >> + productCalculatedInfoList = delegator.findByAnd("ProductCalculatedInfo", UtilMisc.toMap("productId", productId), null, false); >> + if (UtilValidate.isEmpty(productCalculatedInfoList)) { >> + productCalculatedInfo = delegator.makeValue("ProductCalculatedInfo"); >> + productCalculatedInfo.set("productId", productId); >> + productCalculatedInfo.set("totalQuantityOrdered", quantity); >> + productCalculatedInfo.create(); >> + } else { >> + productCalculatedInfo = productCalculatedInfoList.get(0); >> + BigDecimal totalQuantityOrdered = productCalculatedInfo.getBigDecimal("totalQuantityOrdered"); >> + if (totalQuantityOrdered == null) { >> + productCalculatedInfo.set("totalQuantityOrdered", quantity); >> + } else { >> + productCalculatedInfo.set("totalQuantityOrdered", totalQuantityOrdered.add(quantity)); >> + } >> + } >> + productCalculatedInfo.store(); >> + } catch (GenericEntityException e) { >> + Debug.logError(e, "Error calling countProductQuantityOrdered service", module); >> + return ServiceUtil.returnError(UtilProperties.getMessage(resource_error, >> + "OrderErrorCallingCountProductQuantityOrderedService",locale) + e.toString()); >> + >> + } >> + >> + String virtualProductId = null; >> + try { >> + GenericValue product = delegator.findOne("Product", UtilMisc.toMap("productId", productId), true); >> + virtualProductId = ProductWorker.getVariantVirtualId(product); >> + } catch (GenericEntityException e) { >> + Debug.logError(e, "Error calling countProductQuantityOrdered service", module); >> + return ServiceUtil.returnError(UtilProperties.getMessage(resource_error, >> + "OrderErrorCallingCountProductQuantityOrderedService",locale) + e.toString()); >> + } >> + >> + if (UtilValidate.isNotEmpty(virtualProductId)) { >> + context.put("productId", virtualProductId); >> + countProductQuantityOrdered(ctx, context); >> + } >> + return ServiceUtil.returnSuccess(); >> + } >> >> public static void reserveInventory(Delegator delegator, LocalDispatcher dispatcher, GenericValue userLogin, Locale locale, List<GenericValue> orderItemShipGroupInfo, List<String> dropShipGroupIds, Map<String, GenericValue> itemValuesBySeqId, String orderTypeId, String productStoreId, List<String> resErrorMessages) throws GeneralException { >> boolean isImmediatelyFulfilled = false; >> @@ -1729,7 +1772,7 @@ public class OrderServices { >> if (UtilValidate.isNotEmpty(orderItemSeqId)) { >> createOrderAdjContext.put("orderItemSeqId", orderItemSeqId); >> } else { >> - createOrderAdjContext.put("orderItemSeqId", "_NA_"); >> + createOrderAdjContext.put("orderItemSeqId", "_NA_"); >> } >> createOrderAdjContext.put("shipGroupSeqId", "_NA_"); >> createOrderAdjContext.put("description", "Tax adjustment due to order change"); >> @@ -5787,4 +5830,4 @@ public class OrderServices { >> >> return ServiceUtil.returnSuccess(); >> } >> -} >> \ No newline at end of file >> +} >> >> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java >> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java?rev=1635192&r1=1635191&r2=1635192&view=diff >> ============================================================================== >> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java (original) >> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java Wed Oct 29 16:37:40 2014 >> @@ -114,8 +114,16 @@ public class SalesOrderTest extends OFBi >> orderItem.set("unitPrice", new BigDecimal("38.4")); >> orderItem.set("unitListPrice", new BigDecimal("48.0")); >> orderItem.set("statusId", "ITEM_CREATED"); >> - >> orderItems.add(orderItem); >> + >> + orderItem = delegator.makeValue("OrderItem", UtilMisc.toMap("orderItemSeqId", "00002", "orderItemTypeId", "PRODUCT_ORDER_ITEM", "prodCatalogId", "DemoCatalog", "productId", "GZ-1006-1", "quantity", BigDecimal.ONE, "selectedAmount", BigDecimal.ZERO)); >> + orderItem.set("isPromo", "N"); >> + orderItem.set("isModifiedPrice", "N"); >> + orderItem.set("unitPrice", new BigDecimal("1.99")); >> + orderItem.set("unitListPrice", new BigDecimal("5.99")); >> + orderItem.set("statusId", "ITEM_CREATED"); >> + orderItems.add(orderItem); >> + >> ctx.put("orderItems", orderItems); >> >> List<GenericValue> orderTerms = FastList.newInstance(); >> >> Modified: ofbiz/trunk/applications/product/servicedef/services.xml >> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/servicedef/services.xml?rev=1635192&r1=1635191&r2=1635192&view=diff >> ============================================================================== >> --- ofbiz/trunk/applications/product/servicedef/services.xml (original) >> +++ ofbiz/trunk/applications/product/servicedef/services.xml Wed Oct 29 16:37:40 2014 >> @@ -175,13 +175,7 @@ under the License. >> <attribute name="productId" type="String" mode="IN" optional="false"/> >> <attribute name="weight" type="Long" mode="IN" optional="true"/> >> </service> >> - <service name="countProductQuantityOrdered" engine="simple" >> - location="component://product/script/org/ofbiz/product/product/ProductServices.xml" invoke="countProductQuantityOrdered" auth="true"> >> - <description>count Product Quantity Ordered</description> >> - <attribute name="productId" type="String" mode="IN" optional="false"/> >> - <attribute name="quantity" type="BigDecimal" mode="IN" optional="false"/> >> - </service> >> - >> + >> <service name="createProductReview" engine="simple" >> location="component://product/script/org/ofbiz/product/product/ProductServices.xml" invoke="createProductReview" auth="true"> >> <description>Create a product review entity</description> >> >> > |
Re: svn commit: r1635192 - in /ofbiz/trunk/applications: order/servicedef/services.xml order/src/org/ofbiz/order/orde
|
|
In reply to this post by Jacopo Cappellato-4
I'm not Jacques, but I'm pretty sure there is very little difference.
The disadvantage to hacks like this: Bypassing the service engine like this will prevent any SECAs attached to the called service from being invoked. Adrian Crum Sandglass Software www.sandglass-software.com On 10/29/2014 5:50 PM, Jacopo Cappellato wrote: > Jacques, > > what was the measured difference between the method call to the Java method and the service call to the Java implementation? > > Jacopo > > On Oct 29, 2014, at 5:37 PM, [hidden email] wrote: > >> Author: jleroux >> Date: Wed Oct 29 16:37:40 2014 >> New Revision: 1635192 >> >> URL: http://svn.apache.org/r1635192 >> Log: >> The storeOrder service, implemented by the OrderService.createOrder() method, synchronously calls the countProductQuantityOrdered service implemented in Minilang. OOTB, the countProductQuantityOrdered service is only called by the storeOrder service implementation. It's called inside a loop on orderItems. >> >> While intentionally load testing a custom project with JMeter on a weak m1.small AWS machine, I noticed the overhead of the service call for each order item iteration was significant on this slow machine. With only 30 concurrent users the process was blocked by a timeout on the countProductQuantityOrdered service. >> >> I then decided to transform the minilang code into an OrderService.countProductQuantityOrdered() method that can be directly called inside OrderService.createOrder() and also implements the countProductQuantityOrdered service. Hence it avoids the overhead of the minilang service calls in loop while still providing a countProductQuantityOrdered service for possible external uses. For that I moved the definition from Product component to Order component to avoid the hard coded dependency of Order to Product. I then did not cross issues with the same load test. >> >> I also added a test for the countProductQuantityOrdered service by adding an order item of a virtual product (GZ-1006-1) in SalesOrderTest. >> >> Modified: >> ofbiz/trunk/applications/order/servicedef/services.xml >> ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java >> ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java >> ofbiz/trunk/applications/product/servicedef/services.xml >> >> Modified: ofbiz/trunk/applications/order/servicedef/services.xml >> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/servicedef/services.xml?rev=1635192&r1=1635191&r2=1635192&view=diff >> ============================================================================== >> --- ofbiz/trunk/applications/order/servicedef/services.xml (original) >> +++ ofbiz/trunk/applications/order/servicedef/services.xml Wed Oct 29 16:37:40 2014 >> @@ -1147,4 +1147,11 @@ under the License. >> <auto-attributes mode="IN" entity-name="OrderItemGroupOrder" include="pk" optional="false"/> >> </service> >> >> + <service name="countProductQuantityOrdered" engine="java" >> + location="org.ofbiz.order.order.OrderServices" invoke="countProductQuantityOrdered" auth="true"> >> + <description>count Product Quantity Ordered</description> >> + <attribute name="productId" type="String" mode="IN" optional="false"/> >> + <attribute name="quantity" type="BigDecimal" mode="IN" optional="false"/> >> + </service> >> + >> </services> >> >> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java >> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java?rev=1635192&r1=1635191&r2=1635192&view=diff >> ============================================================================== >> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java (original) >> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java Wed Oct 29 16:37:40 2014 >> @@ -280,15 +280,10 @@ public class OrderServices { >> normalizedItemQuantities.put(currentProductId, currentQuantity.add(orderItem.getBigDecimal("quantity"))); >> } >> >> - try { >> - // count product ordered quantities >> - // run this synchronously so it will run in the same transaction >> - dispatcher.runSync("countProductQuantityOrdered", UtilMisc.<String, Object>toMap("productId", currentProductId, "quantity", orderItem.getBigDecimal("quantity"), "userLogin", userLogin)); >> - } catch (GenericServiceException e1) { >> - Debug.logError(e1, "Error calling countProductQuantityOrdered service", module); >> - return ServiceUtil.returnError(UtilProperties.getMessage(resource_error, >> - "OrderErrorCallingCountProductQuantityOrderedService",locale) + e1.toString()); >> - } >> + Map<String, Object> countContext = new HashMap<String, Object>(); >> + countContext.put("productId", currentProductId); >> + countContext.put("quantity", orderItem.getBigDecimal("quantity")); >> + countProductQuantityOrdered(ctx, countContext); >> } >> } >> >> @@ -1152,6 +1147,54 @@ public class OrderServices { >> >> return successResult; >> } >> + >> + public static Map<String, Object> countProductQuantityOrdered(DispatchContext ctx, Map<String, Object> context) { >> + Delegator delegator = ctx.getDelegator(); >> + Locale locale = (Locale) context.get("locale"); >> + List<GenericValue> productCalculatedInfoList = null; >> + GenericValue productCalculatedInfo = null; >> + String productId = (String) context.get("productId"); >> + BigDecimal quantity = (BigDecimal) context.get("quantity"); >> + try { >> + productCalculatedInfoList = delegator.findByAnd("ProductCalculatedInfo", UtilMisc.toMap("productId", productId), null, false); >> + if (UtilValidate.isEmpty(productCalculatedInfoList)) { >> + productCalculatedInfo = delegator.makeValue("ProductCalculatedInfo"); >> + productCalculatedInfo.set("productId", productId); >> + productCalculatedInfo.set("totalQuantityOrdered", quantity); >> + productCalculatedInfo.create(); >> + } else { >> + productCalculatedInfo = productCalculatedInfoList.get(0); >> + BigDecimal totalQuantityOrdered = productCalculatedInfo.getBigDecimal("totalQuantityOrdered"); >> + if (totalQuantityOrdered == null) { >> + productCalculatedInfo.set("totalQuantityOrdered", quantity); >> + } else { >> + productCalculatedInfo.set("totalQuantityOrdered", totalQuantityOrdered.add(quantity)); >> + } >> + } >> + productCalculatedInfo.store(); >> + } catch (GenericEntityException e) { >> + Debug.logError(e, "Error calling countProductQuantityOrdered service", module); >> + return ServiceUtil.returnError(UtilProperties.getMessage(resource_error, >> + "OrderErrorCallingCountProductQuantityOrderedService",locale) + e.toString()); >> + >> + } >> + >> + String virtualProductId = null; >> + try { >> + GenericValue product = delegator.findOne("Product", UtilMisc.toMap("productId", productId), true); >> + virtualProductId = ProductWorker.getVariantVirtualId(product); >> + } catch (GenericEntityException e) { >> + Debug.logError(e, "Error calling countProductQuantityOrdered service", module); >> + return ServiceUtil.returnError(UtilProperties.getMessage(resource_error, >> + "OrderErrorCallingCountProductQuantityOrderedService",locale) + e.toString()); >> + } >> + >> + if (UtilValidate.isNotEmpty(virtualProductId)) { >> + context.put("productId", virtualProductId); >> + countProductQuantityOrdered(ctx, context); >> + } >> + return ServiceUtil.returnSuccess(); >> + } >> >> public static void reserveInventory(Delegator delegator, LocalDispatcher dispatcher, GenericValue userLogin, Locale locale, List<GenericValue> orderItemShipGroupInfo, List<String> dropShipGroupIds, Map<String, GenericValue> itemValuesBySeqId, String orderTypeId, String productStoreId, List<String> resErrorMessages) throws GeneralException { >> boolean isImmediatelyFulfilled = false; >> @@ -1729,7 +1772,7 @@ public class OrderServices { >> if (UtilValidate.isNotEmpty(orderItemSeqId)) { >> createOrderAdjContext.put("orderItemSeqId", orderItemSeqId); >> } else { >> - createOrderAdjContext.put("orderItemSeqId", "_NA_"); >> + createOrderAdjContext.put("orderItemSeqId", "_NA_"); >> } >> createOrderAdjContext.put("shipGroupSeqId", "_NA_"); >> createOrderAdjContext.put("description", "Tax adjustment due to order change"); >> @@ -5787,4 +5830,4 @@ public class OrderServices { >> >> return ServiceUtil.returnSuccess(); >> } >> -} >> \ No newline at end of file >> +} >> >> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java >> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java?rev=1635192&r1=1635191&r2=1635192&view=diff >> ============================================================================== >> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java (original) >> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java Wed Oct 29 16:37:40 2014 >> @@ -114,8 +114,16 @@ public class SalesOrderTest extends OFBi >> orderItem.set("unitPrice", new BigDecimal("38.4")); >> orderItem.set("unitListPrice", new BigDecimal("48.0")); >> orderItem.set("statusId", "ITEM_CREATED"); >> - >> orderItems.add(orderItem); >> + >> + orderItem = delegator.makeValue("OrderItem", UtilMisc.toMap("orderItemSeqId", "00002", "orderItemTypeId", "PRODUCT_ORDER_ITEM", "prodCatalogId", "DemoCatalog", "productId", "GZ-1006-1", "quantity", BigDecimal.ONE, "selectedAmount", BigDecimal.ZERO)); >> + orderItem.set("isPromo", "N"); >> + orderItem.set("isModifiedPrice", "N"); >> + orderItem.set("unitPrice", new BigDecimal("1.99")); >> + orderItem.set("unitListPrice", new BigDecimal("5.99")); >> + orderItem.set("statusId", "ITEM_CREATED"); >> + orderItems.add(orderItem); >> + >> ctx.put("orderItems", orderItems); >> >> List<GenericValue> orderTerms = FastList.newInstance(); >> >> Modified: ofbiz/trunk/applications/product/servicedef/services.xml >> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/servicedef/services.xml?rev=1635192&r1=1635191&r2=1635192&view=diff >> ============================================================================== >> --- ofbiz/trunk/applications/product/servicedef/services.xml (original) >> +++ ofbiz/trunk/applications/product/servicedef/services.xml Wed Oct 29 16:37:40 2014 >> @@ -175,13 +175,7 @@ under the License. >> <attribute name="productId" type="String" mode="IN" optional="false"/> >> <attribute name="weight" type="Long" mode="IN" optional="true"/> >> </service> >> - <service name="countProductQuantityOrdered" engine="simple" >> - location="component://product/script/org/ofbiz/product/product/ProductServices.xml" invoke="countProductQuantityOrdered" auth="true"> >> - <description>count Product Quantity Ordered</description> >> - <attribute name="productId" type="String" mode="IN" optional="false"/> >> - <attribute name="quantity" type="BigDecimal" mode="IN" optional="false"/> >> - </service> >> - >> + >> <service name="createProductReview" engine="simple" >> location="component://product/script/org/ofbiz/product/product/ProductServices.xml" invoke="createProductReview" auth="true"> >> <description>Create a product review entity</description> >> >> > |
Re: svn commit: r1635192 - in /ofbiz/trunk/applications: order/servicedef/services.xml order/src/org/ofbiz/order/orde
Administrator
|
In reply to this post by Jacopo Cappellato-4
Le 29/10/2014 18:50, Jacopo Cappellato a écrit : > Jacques, > > what was the measured difference between the method call to the Java method and the service call to the Java implementation? Apart what I reported in the commit comment and the Jira description, I did not make other measures. Before it was blocking the JMeter test after it passed. Jacques > > Jacopo > > On Oct 29, 2014, at 5:37 PM, [hidden email] wrote: > >> Author: jleroux >> Date: Wed Oct 29 16:37:40 2014 >> New Revision: 1635192 >> >> URL: http://svn.apache.org/r1635192 >> Log: >> The storeOrder service, implemented by the OrderService.createOrder() method, synchronously calls the countProductQuantityOrdered service implemented in Minilang. OOTB, the countProductQuantityOrdered service is only called by the storeOrder service implementation. It's called inside a loop on orderItems. >> >> While intentionally load testing a custom project with JMeter on a weak m1.small AWS machine, I noticed the overhead of the service call for each order item iteration was significant on this slow machine. With only 30 concurrent users the process was blocked by a timeout on the countProductQuantityOrdered service. >> >> I then decided to transform the minilang code into an OrderService.countProductQuantityOrdered() method that can be directly called inside OrderService.createOrder() and also implements the countProductQuantityOrdered service. Hence it avoids the overhead of the minilang service calls in loop while still providing a countProductQuantityOrdered service for possible external uses. For that I moved the definition from Product component to Order component to avoid the hard coded dependency of Order to Product. I then did not cross issues with the same load test. >> >> I also added a test for the countProductQuantityOrdered service by adding an order item of a virtual product (GZ-1006-1) in SalesOrderTest. >> >> Modified: >> ofbiz/trunk/applications/order/servicedef/services.xml >> ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java >> ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java >> ofbiz/trunk/applications/product/servicedef/services.xml >> >> Modified: ofbiz/trunk/applications/order/servicedef/services.xml >> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/servicedef/services.xml?rev=1635192&r1=1635191&r2=1635192&view=diff >> ============================================================================== >> --- ofbiz/trunk/applications/order/servicedef/services.xml (original) >> +++ ofbiz/trunk/applications/order/servicedef/services.xml Wed Oct 29 16:37:40 2014 >> @@ -1147,4 +1147,11 @@ under the License. >> <auto-attributes mode="IN" entity-name="OrderItemGroupOrder" include="pk" optional="false"/> >> </service> >> >> + <service name="countProductQuantityOrdered" engine="java" >> + location="org.ofbiz.order.order.OrderServices" invoke="countProductQuantityOrdered" auth="true"> >> + <description>count Product Quantity Ordered</description> >> + <attribute name="productId" type="String" mode="IN" optional="false"/> >> + <attribute name="quantity" type="BigDecimal" mode="IN" optional="false"/> >> + </service> >> + >> </services> >> >> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java >> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java?rev=1635192&r1=1635191&r2=1635192&view=diff >> ============================================================================== >> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java (original) >> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java Wed Oct 29 16:37:40 2014 >> @@ -280,15 +280,10 @@ public class OrderServices { >> normalizedItemQuantities.put(currentProductId, currentQuantity.add(orderItem.getBigDecimal("quantity"))); >> } >> >> - try { >> - // count product ordered quantities >> - // run this synchronously so it will run in the same transaction >> - dispatcher.runSync("countProductQuantityOrdered", UtilMisc.<String, Object>toMap("productId", currentProductId, "quantity", orderItem.getBigDecimal("quantity"), "userLogin", userLogin)); >> - } catch (GenericServiceException e1) { >> - Debug.logError(e1, "Error calling countProductQuantityOrdered service", module); >> - return ServiceUtil.returnError(UtilProperties.getMessage(resource_error, >> - "OrderErrorCallingCountProductQuantityOrderedService",locale) + e1.toString()); >> - } >> + Map<String, Object> countContext = new HashMap<String, Object>(); >> + countContext.put("productId", currentProductId); >> + countContext.put("quantity", orderItem.getBigDecimal("quantity")); >> + countProductQuantityOrdered(ctx, countContext); >> } >> } >> >> @@ -1152,6 +1147,54 @@ public class OrderServices { >> >> return successResult; >> } >> + >> + public static Map<String, Object> countProductQuantityOrdered(DispatchContext ctx, Map<String, Object> context) { >> + Delegator delegator = ctx.getDelegator(); >> + Locale locale = (Locale) context.get("locale"); >> + List<GenericValue> productCalculatedInfoList = null; >> + GenericValue productCalculatedInfo = null; >> + String productId = (String) context.get("productId"); >> + BigDecimal quantity = (BigDecimal) context.get("quantity"); >> + try { >> + productCalculatedInfoList = delegator.findByAnd("ProductCalculatedInfo", UtilMisc.toMap("productId", productId), null, false); >> + if (UtilValidate.isEmpty(productCalculatedInfoList)) { >> + productCalculatedInfo = delegator.makeValue("ProductCalculatedInfo"); >> + productCalculatedInfo.set("productId", productId); >> + productCalculatedInfo.set("totalQuantityOrdered", quantity); >> + productCalculatedInfo.create(); >> + } else { >> + productCalculatedInfo = productCalculatedInfoList.get(0); >> + BigDecimal totalQuantityOrdered = productCalculatedInfo.getBigDecimal("totalQuantityOrdered"); >> + if (totalQuantityOrdered == null) { >> + productCalculatedInfo.set("totalQuantityOrdered", quantity); >> + } else { >> + productCalculatedInfo.set("totalQuantityOrdered", totalQuantityOrdered.add(quantity)); >> + } >> + } >> + productCalculatedInfo.store(); >> + } catch (GenericEntityException e) { >> + Debug.logError(e, "Error calling countProductQuantityOrdered service", module); >> + return ServiceUtil.returnError(UtilProperties.getMessage(resource_error, >> + "OrderErrorCallingCountProductQuantityOrderedService",locale) + e.toString()); >> + >> + } >> + >> + String virtualProductId = null; >> + try { >> + GenericValue product = delegator.findOne("Product", UtilMisc.toMap("productId", productId), true); >> + virtualProductId = ProductWorker.getVariantVirtualId(product); >> + } catch (GenericEntityException e) { >> + Debug.logError(e, "Error calling countProductQuantityOrdered service", module); >> + return ServiceUtil.returnError(UtilProperties.getMessage(resource_error, >> + "OrderErrorCallingCountProductQuantityOrderedService",locale) + e.toString()); >> + } >> + >> + if (UtilValidate.isNotEmpty(virtualProductId)) { >> + context.put("productId", virtualProductId); >> + countProductQuantityOrdered(ctx, context); >> + } >> + return ServiceUtil.returnSuccess(); >> + } >> >> public static void reserveInventory(Delegator delegator, LocalDispatcher dispatcher, GenericValue userLogin, Locale locale, List<GenericValue> orderItemShipGroupInfo, List<String> dropShipGroupIds, Map<String, GenericValue> itemValuesBySeqId, String orderTypeId, String productStoreId, List<String> resErrorMessages) throws GeneralException { >> boolean isImmediatelyFulfilled = false; >> @@ -1729,7 +1772,7 @@ public class OrderServices { >> if (UtilValidate.isNotEmpty(orderItemSeqId)) { >> createOrderAdjContext.put("orderItemSeqId", orderItemSeqId); >> } else { >> - createOrderAdjContext.put("orderItemSeqId", "_NA_"); >> + createOrderAdjContext.put("orderItemSeqId", "_NA_"); >> } >> createOrderAdjContext.put("shipGroupSeqId", "_NA_"); >> createOrderAdjContext.put("description", "Tax adjustment due to order change"); >> @@ -5787,4 +5830,4 @@ public class OrderServices { >> >> return ServiceUtil.returnSuccess(); >> } >> -} >> \ No newline at end of file >> +} >> >> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java >> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java?rev=1635192&r1=1635191&r2=1635192&view=diff >> ============================================================================== >> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java (original) >> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java Wed Oct 29 16:37:40 2014 >> @@ -114,8 +114,16 @@ public class SalesOrderTest extends OFBi >> orderItem.set("unitPrice", new BigDecimal("38.4")); >> orderItem.set("unitListPrice", new BigDecimal("48.0")); >> orderItem.set("statusId", "ITEM_CREATED"); >> - >> orderItems.add(orderItem); >> + >> + orderItem = delegator.makeValue("OrderItem", UtilMisc.toMap("orderItemSeqId", "00002", "orderItemTypeId", "PRODUCT_ORDER_ITEM", "prodCatalogId", "DemoCatalog", "productId", "GZ-1006-1", "quantity", BigDecimal.ONE, "selectedAmount", BigDecimal.ZERO)); >> + orderItem.set("isPromo", "N"); >> + orderItem.set("isModifiedPrice", "N"); >> + orderItem.set("unitPrice", new BigDecimal("1.99")); >> + orderItem.set("unitListPrice", new BigDecimal("5.99")); >> + orderItem.set("statusId", "ITEM_CREATED"); >> + orderItems.add(orderItem); >> + >> ctx.put("orderItems", orderItems); >> >> List<GenericValue> orderTerms = FastList.newInstance(); >> >> Modified: ofbiz/trunk/applications/product/servicedef/services.xml >> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/servicedef/services.xml?rev=1635192&r1=1635191&r2=1635192&view=diff >> ============================================================================== >> --- ofbiz/trunk/applications/product/servicedef/services.xml (original) >> +++ ofbiz/trunk/applications/product/servicedef/services.xml Wed Oct 29 16:37:40 2014 >> @@ -175,13 +175,7 @@ under the License. >> <attribute name="productId" type="String" mode="IN" optional="false"/> >> <attribute name="weight" type="Long" mode="IN" optional="true"/> >> </service> >> - <service name="countProductQuantityOrdered" engine="simple" >> - location="component://product/script/org/ofbiz/product/product/ProductServices.xml" invoke="countProductQuantityOrdered" auth="true"> >> - <description>count Product Quantity Ordered</description> >> - <attribute name="productId" type="String" mode="IN" optional="false"/> >> - <attribute name="quantity" type="BigDecimal" mode="IN" optional="false"/> >> - </service> >> - >> + >> <service name="createProductReview" engine="simple" >> location="component://product/script/org/ofbiz/product/product/ProductServices.xml" invoke="createProductReview" auth="true"> >> <description>Create a product review entity</description> >> >> > > |
Re: svn commit: r1635192 - in /ofbiz/trunk/applications: order/servicedef/services.xml order/src/org/ofbiz/order/orde
|
Administrator
|
In reply to this post by Adrian Crum-3
Le 29/10/2014 19:02, Adrian Crum a écrit :
> I see a serious problem with this commit: > > Map<String, Object> countContext = new HashMap<String, Object>(); > > is wrong. > > Use the DispatchContext to build the service context from the existing context Map - so the framework-inserted artifacts are included in the Map. I don't see any reasons to add other parameters than the ones necessary for the service call Jacques > > Adrian Crum > Sandglass Software > www.sandglass-software.com > > On 10/29/2014 5:50 PM, Jacopo Cappellato wrote: >> Jacques, >> >> what was the measured difference between the method call to the Java method and the service call to the Java implementation? >> >> Jacopo >> >> On Oct 29, 2014, at 5:37 PM, [hidden email] wrote: >> >>> Author: jleroux >>> Date: Wed Oct 29 16:37:40 2014 >>> New Revision: 1635192 >>> >>> URL: http://svn.apache.org/r1635192 >>> Log: >>> The storeOrder service, implemented by the OrderService.createOrder() method, synchronously calls the countProductQuantityOrdered service >>> implemented in Minilang. OOTB, the countProductQuantityOrdered service is only called by the storeOrder service implementation. It's called inside >>> a loop on orderItems. >>> >>> While intentionally load testing a custom project with JMeter on a weak m1.small AWS machine, I noticed the overhead of the service call for each >>> order item iteration was significant on this slow machine. With only 30 concurrent users the process was blocked by a timeout on the >>> countProductQuantityOrdered service. >>> >>> I then decided to transform the minilang code into an OrderService.countProductQuantityOrdered() method that can be directly called inside >>> OrderService.createOrder() and also implements the countProductQuantityOrdered service. Hence it avoids the overhead of the minilang service >>> calls in loop while still providing a countProductQuantityOrdered service for possible external uses. For that I moved the definition from Product >>> component to Order component to avoid the hard coded dependency of Order to Product. I then did not cross issues with the same load test. >>> >>> I also added a test for the countProductQuantityOrdered service by adding an order item of a virtual product (GZ-1006-1) in SalesOrderTest. >>> >>> Modified: >>> ofbiz/trunk/applications/order/servicedef/services.xml >>> ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java >>> ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java >>> ofbiz/trunk/applications/product/servicedef/services.xml >>> >>> Modified: ofbiz/trunk/applications/order/servicedef/services.xml >>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/servicedef/services.xml?rev=1635192&r1=1635191&r2=1635192&view=diff >>> ============================================================================== >>> --- ofbiz/trunk/applications/order/servicedef/services.xml (original) >>> +++ ofbiz/trunk/applications/order/servicedef/services.xml Wed Oct 29 16:37:40 2014 >>> @@ -1147,4 +1147,11 @@ under the License. >>> <auto-attributes mode="IN" entity-name="OrderItemGroupOrder" include="pk" optional="false"/> >>> </service> >>> >>> + <service name="countProductQuantityOrdered" engine="java" >>> + location="org.ofbiz.order.order.OrderServices" invoke="countProductQuantityOrdered" auth="true"> >>> + <description>count Product Quantity Ordered</description> >>> + <attribute name="productId" type="String" mode="IN" optional="false"/> >>> + <attribute name="quantity" type="BigDecimal" mode="IN" optional="false"/> >>> + </service> >>> + >>> </services> >>> >>> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java >>> URL: >>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java?rev=1635192&r1=1635191&r2=1635192&view=diff >>> ============================================================================== >>> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java (original) >>> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java Wed Oct 29 16:37:40 2014 >>> @@ -280,15 +280,10 @@ public class OrderServices { >>> normalizedItemQuantities.put(currentProductId, currentQuantity.add(orderItem.getBigDecimal("quantity"))); >>> } >>> >>> - try { >>> - // count product ordered quantities >>> - // run this synchronously so it will run in the same transaction >>> - dispatcher.runSync("countProductQuantityOrdered", UtilMisc.<String, Object>toMap("productId", currentProductId, "quantity", >>> orderItem.getBigDecimal("quantity"), "userLogin", userLogin)); >>> - } catch (GenericServiceException e1) { >>> - Debug.logError(e1, "Error calling countProductQuantityOrdered service", module); >>> - return ServiceUtil.returnError(UtilProperties.getMessage(resource_error, >>> - "OrderErrorCallingCountProductQuantityOrderedService",locale) + e1.toString()); >>> - } >>> + Map<String, Object> countContext = new HashMap<String, Object>(); >>> + countContext.put("productId", currentProductId); >>> + countContext.put("quantity", orderItem.getBigDecimal("quantity")); >>> + countProductQuantityOrdered(ctx, countContext); >>> } >>> } >>> >>> @@ -1152,6 +1147,54 @@ public class OrderServices { >>> >>> return successResult; >>> } >>> + >>> + public static Map<String, Object> countProductQuantityOrdered(DispatchContext ctx, Map<String, Object> context) { >>> + Delegator delegator = ctx.getDelegator(); >>> + Locale locale = (Locale) context.get("locale"); >>> + List<GenericValue> productCalculatedInfoList = null; >>> + GenericValue productCalculatedInfo = null; >>> + String productId = (String) context.get("productId"); >>> + BigDecimal quantity = (BigDecimal) context.get("quantity"); >>> + try { >>> + productCalculatedInfoList = delegator.findByAnd("ProductCalculatedInfo", UtilMisc.toMap("productId", productId), null, false); >>> + if (UtilValidate.isEmpty(productCalculatedInfoList)) { >>> + productCalculatedInfo = delegator.makeValue("ProductCalculatedInfo"); >>> + productCalculatedInfo.set("productId", productId); >>> + productCalculatedInfo.set("totalQuantityOrdered", quantity); >>> + productCalculatedInfo.create(); >>> + } else { >>> + productCalculatedInfo = productCalculatedInfoList.get(0); >>> + BigDecimal totalQuantityOrdered = productCalculatedInfo.getBigDecimal("totalQuantityOrdered"); >>> + if (totalQuantityOrdered == null) { >>> + productCalculatedInfo.set("totalQuantityOrdered", quantity); >>> + } else { >>> + productCalculatedInfo.set("totalQuantityOrdered", totalQuantityOrdered.add(quantity)); >>> + } >>> + } >>> + productCalculatedInfo.store(); >>> + } catch (GenericEntityException e) { >>> + Debug.logError(e, "Error calling countProductQuantityOrdered service", module); >>> + return ServiceUtil.returnError(UtilProperties.getMessage(resource_error, >>> + "OrderErrorCallingCountProductQuantityOrderedService",locale) + e.toString()); >>> + >>> + } >>> + >>> + String virtualProductId = null; >>> + try { >>> + GenericValue product = delegator.findOne("Product", UtilMisc.toMap("productId", productId), true); >>> + virtualProductId = ProductWorker.getVariantVirtualId(product); >>> + } catch (GenericEntityException e) { >>> + Debug.logError(e, "Error calling countProductQuantityOrdered service", module); >>> + return ServiceUtil.returnError(UtilProperties.getMessage(resource_error, >>> + "OrderErrorCallingCountProductQuantityOrderedService",locale) + e.toString()); >>> + } >>> + >>> + if (UtilValidate.isNotEmpty(virtualProductId)) { >>> + context.put("productId", virtualProductId); >>> + countProductQuantityOrdered(ctx, context); >>> + } >>> + return ServiceUtil.returnSuccess(); >>> + } >>> >>> public static void reserveInventory(Delegator delegator, LocalDispatcher dispatcher, GenericValue userLogin, Locale locale, >>> List<GenericValue> orderItemShipGroupInfo, List<String> dropShipGroupIds, Map<String, GenericValue> itemValuesBySeqId, String orderTypeId, String >>> productStoreId, List<String> resErrorMessages) throws GeneralException { >>> boolean isImmediatelyFulfilled = false; >>> @@ -1729,7 +1772,7 @@ public class OrderServices { >>> if (UtilValidate.isNotEmpty(orderItemSeqId)) { >>> createOrderAdjContext.put("orderItemSeqId", orderItemSeqId); >>> } else { >>> - createOrderAdjContext.put("orderItemSeqId", "_NA_"); >>> + createOrderAdjContext.put("orderItemSeqId", "_NA_"); >>> } >>> createOrderAdjContext.put("shipGroupSeqId", "_NA_"); >>> createOrderAdjContext.put("description", "Tax adjustment due to order change"); >>> @@ -5787,4 +5830,4 @@ public class OrderServices { >>> >>> return ServiceUtil.returnSuccess(); >>> } >>> -} >>> \ No newline at end of file >>> +} >>> >>> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java >>> URL: >>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java?rev=1635192&r1=1635191&r2=1635192&view=diff >>> ============================================================================== >>> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java (original) >>> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java Wed Oct 29 16:37:40 2014 >>> @@ -114,8 +114,16 @@ public class SalesOrderTest extends OFBi >>> orderItem.set("unitPrice", new BigDecimal("38.4")); >>> orderItem.set("unitListPrice", new BigDecimal("48.0")); >>> orderItem.set("statusId", "ITEM_CREATED"); >>> - >>> orderItems.add(orderItem); >>> + >>> + orderItem = delegator.makeValue("OrderItem", UtilMisc.toMap("orderItemSeqId", "00002", "orderItemTypeId", "PRODUCT_ORDER_ITEM", >>> "prodCatalogId", "DemoCatalog", "productId", "GZ-1006-1", "quantity", BigDecimal.ONE, "selectedAmount", BigDecimal.ZERO)); >>> + orderItem.set("isPromo", "N"); >>> + orderItem.set("isModifiedPrice", "N"); >>> + orderItem.set("unitPrice", new BigDecimal("1.99")); >>> + orderItem.set("unitListPrice", new BigDecimal("5.99")); >>> + orderItem.set("statusId", "ITEM_CREATED"); >>> + orderItems.add(orderItem); >>> + >>> ctx.put("orderItems", orderItems); >>> >>> List<GenericValue> orderTerms = FastList.newInstance(); >>> >>> Modified: ofbiz/trunk/applications/product/servicedef/services.xml >>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/servicedef/services.xml?rev=1635192&r1=1635191&r2=1635192&view=diff >>> ============================================================================== >>> --- ofbiz/trunk/applications/product/servicedef/services.xml (original) >>> +++ ofbiz/trunk/applications/product/servicedef/services.xml Wed Oct 29 16:37:40 2014 >>> @@ -175,13 +175,7 @@ under the License. >>> <attribute name="productId" type="String" mode="IN" optional="false"/> >>> <attribute name="weight" type="Long" mode="IN" optional="true"/> >>> </service> >>> - <service name="countProductQuantityOrdered" engine="simple" >>> - location="component://product/script/org/ofbiz/product/product/ProductServices.xml" invoke="countProductQuantityOrdered" auth="true"> >>> - <description>count Product Quantity Ordered</description> >>> - <attribute name="productId" type="String" mode="IN" optional="false"/> >>> - <attribute name="quantity" type="BigDecimal" mode="IN" optional="false"/> >>> - </service> >>> - >>> + >>> <service name="createProductReview" engine="simple" >>> location="component://product/script/org/ofbiz/product/product/ProductServices.xml" invoke="createProductReview" auth="true"> >>> <description>Create a product review entity</description> >>> >>> >> > |
Re: svn commit: r1635192 - in /ofbiz/trunk/applications: order/servicedef/services.xml order/src/org/ofbiz/order/orde
|
Administrator
|
In reply to this post by Adrian Crum-3
Le 29/10/2014 19:22, Adrian Crum a écrit : > I'm not Jacques, but I'm pretty sure there is very little difference. The disadvantage to hacks like this: Bypassing the service engine like this > will prevent any SECAs attached to the called service from being invoked. OOTB there are any SECAs attached to the countProductQuantityOrdered. And I doubt there are in custom projects, they would be quite convoluted. ProductCalculatedInfo entity is destined for stats and reports. I don't see it used in business workflows: after my change this service is not used OOTB. I'm not even sure we need this service! Jacques > > Adrian Crum > Sandglass Software > www.sandglass-software.com > > On 10/29/2014 5:50 PM, Jacopo Cappellato wrote: >> Jacques, >> >> what was the measured difference between the method call to the Java method and the service call to the Java implementation? >> >> Jacopo >> >> On Oct 29, 2014, at 5:37 PM, [hidden email] wrote: >> >>> Author: jleroux >>> Date: Wed Oct 29 16:37:40 2014 >>> New Revision: 1635192 >>> >>> URL: http://svn.apache.org/r1635192 >>> Log: >>> The storeOrder service, implemented by the OrderService.createOrder() method, synchronously calls the countProductQuantityOrdered service >>> implemented in Minilang. OOTB, the countProductQuantityOrdered service is only called by the storeOrder service implementation. It's called inside >>> a loop on orderItems. >>> >>> While intentionally load testing a custom project with JMeter on a weak m1.small AWS machine, I noticed the overhead of the service call for each >>> order item iteration was significant on this slow machine. With only 30 concurrent users the process was blocked by a timeout on the >>> countProductQuantityOrdered service. >>> >>> I then decided to transform the minilang code into an OrderService.countProductQuantityOrdered() method that can be directly called inside >>> OrderService.createOrder() and also implements the countProductQuantityOrdered service. Hence it avoids the overhead of the minilang service >>> calls in loop while still providing a countProductQuantityOrdered service for possible external uses. For that I moved the definition from Product >>> component to Order component to avoid the hard coded dependency of Order to Product. I then did not cross issues with the same load test. >>> >>> I also added a test for the countProductQuantityOrdered service by adding an order item of a virtual product (GZ-1006-1) in SalesOrderTest. >>> >>> Modified: >>> ofbiz/trunk/applications/order/servicedef/services.xml >>> ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java >>> ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java >>> ofbiz/trunk/applications/product/servicedef/services.xml >>> >>> Modified: ofbiz/trunk/applications/order/servicedef/services.xml >>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/servicedef/services.xml?rev=1635192&r1=1635191&r2=1635192&view=diff >>> ============================================================================== >>> --- ofbiz/trunk/applications/order/servicedef/services.xml (original) >>> +++ ofbiz/trunk/applications/order/servicedef/services.xml Wed Oct 29 16:37:40 2014 >>> @@ -1147,4 +1147,11 @@ under the License. >>> <auto-attributes mode="IN" entity-name="OrderItemGroupOrder" include="pk" optional="false"/> >>> </service> >>> >>> + <service name="countProductQuantityOrdered" engine="java" >>> + location="org.ofbiz.order.order.OrderServices" invoke="countProductQuantityOrdered" auth="true"> >>> + <description>count Product Quantity Ordered</description> >>> + <attribute name="productId" type="String" mode="IN" optional="false"/> >>> + <attribute name="quantity" type="BigDecimal" mode="IN" optional="false"/> >>> + </service> >>> + >>> </services> >>> >>> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java >>> URL: >>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java?rev=1635192&r1=1635191&r2=1635192&view=diff >>> ============================================================================== >>> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java (original) >>> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java Wed Oct 29 16:37:40 2014 >>> @@ -280,15 +280,10 @@ public class OrderServices { >>> normalizedItemQuantities.put(currentProductId, currentQuantity.add(orderItem.getBigDecimal("quantity"))); >>> } >>> >>> - try { >>> - // count product ordered quantities >>> - // run this synchronously so it will run in the same transaction >>> - dispatcher.runSync("countProductQuantityOrdered", UtilMisc.<String, Object>toMap("productId", currentProductId, "quantity", >>> orderItem.getBigDecimal("quantity"), "userLogin", userLogin)); >>> - } catch (GenericServiceException e1) { >>> - Debug.logError(e1, "Error calling countProductQuantityOrdered service", module); >>> - return ServiceUtil.returnError(UtilProperties.getMessage(resource_error, >>> - "OrderErrorCallingCountProductQuantityOrderedService",locale) + e1.toString()); >>> - } >>> + Map<String, Object> countContext = new HashMap<String, Object>(); >>> + countContext.put("productId", currentProductId); >>> + countContext.put("quantity", orderItem.getBigDecimal("quantity")); >>> + countProductQuantityOrdered(ctx, countContext); >>> } >>> } >>> >>> @@ -1152,6 +1147,54 @@ public class OrderServices { >>> >>> return successResult; >>> } >>> + >>> + public static Map<String, Object> countProductQuantityOrdered(DispatchContext ctx, Map<String, Object> context) { >>> + Delegator delegator = ctx.getDelegator(); >>> + Locale locale = (Locale) context.get("locale"); >>> + List<GenericValue> productCalculatedInfoList = null; >>> + GenericValue productCalculatedInfo = null; >>> + String productId = (String) context.get("productId"); >>> + BigDecimal quantity = (BigDecimal) context.get("quantity"); >>> + try { >>> + productCalculatedInfoList = delegator.findByAnd("ProductCalculatedInfo", UtilMisc.toMap("productId", productId), null, false); >>> + if (UtilValidate.isEmpty(productCalculatedInfoList)) { >>> + productCalculatedInfo = delegator.makeValue("ProductCalculatedInfo"); >>> + productCalculatedInfo.set("productId", productId); >>> + productCalculatedInfo.set("totalQuantityOrdered", quantity); >>> + productCalculatedInfo.create(); >>> + } else { >>> + productCalculatedInfo = productCalculatedInfoList.get(0); >>> + BigDecimal totalQuantityOrdered = productCalculatedInfo.getBigDecimal("totalQuantityOrdered"); >>> + if (totalQuantityOrdered == null) { >>> + productCalculatedInfo.set("totalQuantityOrdered", quantity); >>> + } else { >>> + productCalculatedInfo.set("totalQuantityOrdered", totalQuantityOrdered.add(quantity)); >>> + } >>> + } >>> + productCalculatedInfo.store(); >>> + } catch (GenericEntityException e) { >>> + Debug.logError(e, "Error calling countProductQuantityOrdered service", module); >>> + return ServiceUtil.returnError(UtilProperties.getMessage(resource_error, >>> + "OrderErrorCallingCountProductQuantityOrderedService",locale) + e.toString()); >>> + >>> + } >>> + >>> + String virtualProductId = null; >>> + try { >>> + GenericValue product = delegator.findOne("Product", UtilMisc.toMap("productId", productId), true); >>> + virtualProductId = ProductWorker.getVariantVirtualId(product); >>> + } catch (GenericEntityException e) { >>> + Debug.logError(e, "Error calling countProductQuantityOrdered service", module); >>> + return ServiceUtil.returnError(UtilProperties.getMessage(resource_error, >>> + "OrderErrorCallingCountProductQuantityOrderedService",locale) + e.toString()); >>> + } >>> + >>> + if (UtilValidate.isNotEmpty(virtualProductId)) { >>> + context.put("productId", virtualProductId); >>> + countProductQuantityOrdered(ctx, context); >>> + } >>> + return ServiceUtil.returnSuccess(); >>> + } >>> >>> public static void reserveInventory(Delegator delegator, LocalDispatcher dispatcher, GenericValue userLogin, Locale locale, >>> List<GenericValue> orderItemShipGroupInfo, List<String> dropShipGroupIds, Map<String, GenericValue> itemValuesBySeqId, String orderTypeId, String >>> productStoreId, List<String> resErrorMessages) throws GeneralException { >>> boolean isImmediatelyFulfilled = false; >>> @@ -1729,7 +1772,7 @@ public class OrderServices { >>> if (UtilValidate.isNotEmpty(orderItemSeqId)) { >>> createOrderAdjContext.put("orderItemSeqId", orderItemSeqId); >>> } else { >>> - createOrderAdjContext.put("orderItemSeqId", "_NA_"); >>> + createOrderAdjContext.put("orderItemSeqId", "_NA_"); >>> } >>> createOrderAdjContext.put("shipGroupSeqId", "_NA_"); >>> createOrderAdjContext.put("description", "Tax adjustment due to order change"); >>> @@ -5787,4 +5830,4 @@ public class OrderServices { >>> >>> return ServiceUtil.returnSuccess(); >>> } >>> -} >>> \ No newline at end of file >>> +} >>> >>> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java >>> URL: >>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java?rev=1635192&r1=1635191&r2=1635192&view=diff >>> ============================================================================== >>> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java (original) >>> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java Wed Oct 29 16:37:40 2014 >>> @@ -114,8 +114,16 @@ public class SalesOrderTest extends OFBi >>> orderItem.set("unitPrice", new BigDecimal("38.4")); >>> orderItem.set("unitListPrice", new BigDecimal("48.0")); >>> orderItem.set("statusId", "ITEM_CREATED"); >>> - >>> orderItems.add(orderItem); >>> + >>> + orderItem = delegator.makeValue("OrderItem", UtilMisc.toMap("orderItemSeqId", "00002", "orderItemTypeId", "PRODUCT_ORDER_ITEM", >>> "prodCatalogId", "DemoCatalog", "productId", "GZ-1006-1", "quantity", BigDecimal.ONE, "selectedAmount", BigDecimal.ZERO)); >>> + orderItem.set("isPromo", "N"); >>> + orderItem.set("isModifiedPrice", "N"); >>> + orderItem.set("unitPrice", new BigDecimal("1.99")); >>> + orderItem.set("unitListPrice", new BigDecimal("5.99")); >>> + orderItem.set("statusId", "ITEM_CREATED"); >>> + orderItems.add(orderItem); >>> + >>> ctx.put("orderItems", orderItems); >>> >>> List<GenericValue> orderTerms = FastList.newInstance(); >>> >>> Modified: ofbiz/trunk/applications/product/servicedef/services.xml >>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/servicedef/services.xml?rev=1635192&r1=1635191&r2=1635192&view=diff >>> ============================================================================== >>> --- ofbiz/trunk/applications/product/servicedef/services.xml (original) >>> +++ ofbiz/trunk/applications/product/servicedef/services.xml Wed Oct 29 16:37:40 2014 >>> @@ -175,13 +175,7 @@ under the License. >>> <attribute name="productId" type="String" mode="IN" optional="false"/> >>> <attribute name="weight" type="Long" mode="IN" optional="true"/> >>> </service> >>> - <service name="countProductQuantityOrdered" engine="simple" >>> - location="component://product/script/org/ofbiz/product/product/ProductServices.xml" invoke="countProductQuantityOrdered" auth="true"> >>> - <description>count Product Quantity Ordered</description> >>> - <attribute name="productId" type="String" mode="IN" optional="false"/> >>> - <attribute name="quantity" type="BigDecimal" mode="IN" optional="false"/> >>> - </service> >>> - >>> + >>> <service name="createProductReview" engine="simple" >>> location="component://product/script/org/ofbiz/product/product/ProductServices.xml" invoke="createProductReview" auth="true"> >>> <description>Create a product review entity</description> >>> >>> >> > |
Re: svn commit: r1635192 - in /ofbiz/trunk/applications: order/servicedef/services.xml order/src/org/ofbiz/order/orde
|
|
In reply to this post by Jacques Le Roux
The reason is this:
Don't do that! Use best practices or revert it. Adrian Crum Sandglass Software www.sandglass-software.com On 10/29/2014 7:26 PM, Jacques Le Roux wrote: > Le 29/10/2014 19:02, Adrian Crum a écrit : >> I see a serious problem with this commit: >> >> Map<String, Object> countContext = new HashMap<String, Object>(); >> >> is wrong. >> >> Use the DispatchContext to build the service context from the existing >> context Map - so the framework-inserted artifacts are included in the >> Map. > > I don't see any reasons to add other parameters than the ones necessary > for the service call > > Jacques > >> >> Adrian Crum >> Sandglass Software >> www.sandglass-software.com >> >> On 10/29/2014 5:50 PM, Jacopo Cappellato wrote: >>> Jacques, >>> >>> what was the measured difference between the method call to the Java >>> method and the service call to the Java implementation? >>> >>> Jacopo >>> >>> On Oct 29, 2014, at 5:37 PM, [hidden email] wrote: >>> >>>> Author: jleroux >>>> Date: Wed Oct 29 16:37:40 2014 >>>> New Revision: 1635192 >>>> >>>> URL: http://svn.apache.org/r1635192 >>>> Log: >>>> The storeOrder service, implemented by the >>>> OrderService.createOrder() method, synchronously calls the >>>> countProductQuantityOrdered service implemented in Minilang. OOTB, >>>> the countProductQuantityOrdered service is only called by the >>>> storeOrder service implementation. It's called inside a loop on >>>> orderItems. >>>> >>>> While intentionally load testing a custom project with JMeter on a >>>> weak m1.small AWS machine, I noticed the overhead of the service >>>> call for each order item iteration was significant on this slow >>>> machine. With only 30 concurrent users the process was blocked by a >>>> timeout on the countProductQuantityOrdered service. >>>> >>>> I then decided to transform the minilang code into an >>>> OrderService.countProductQuantityOrdered() method that can be >>>> directly called inside OrderService.createOrder() and also >>>> implements the countProductQuantityOrdered service. Hence it avoids >>>> the overhead of the minilang service calls in loop while still >>>> providing a countProductQuantityOrdered service for possible >>>> external uses. For that I moved the definition from Product >>>> component to Order component to avoid the hard coded dependency of >>>> Order to Product. I then did not cross issues with the same load test. >>>> >>>> I also added a test for the countProductQuantityOrdered service by >>>> adding an order item of a virtual product (GZ-1006-1) in >>>> SalesOrderTest. >>>> >>>> Modified: >>>> ofbiz/trunk/applications/order/servicedef/services.xml >>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java >>>> >>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java >>>> >>>> ofbiz/trunk/applications/product/servicedef/services.xml >>>> >>>> Modified: ofbiz/trunk/applications/order/servicedef/services.xml >>>> URL: >>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/servicedef/services.xml?rev=1635192&r1=1635191&r2=1635192&view=diff >>>> >>>> ============================================================================== >>>> >>>> --- ofbiz/trunk/applications/order/servicedef/services.xml (original) >>>> +++ ofbiz/trunk/applications/order/servicedef/services.xml Wed Oct >>>> 29 16:37:40 2014 >>>> @@ -1147,4 +1147,11 @@ under the License. >>>> <auto-attributes mode="IN" >>>> entity-name="OrderItemGroupOrder" include="pk" optional="false"/> >>>> </service> >>>> >>>> + <service name="countProductQuantityOrdered" engine="java" >>>> + location="org.ofbiz.order.order.OrderServices" >>>> invoke="countProductQuantityOrdered" auth="true"> >>>> + <description>count Product Quantity Ordered</description> >>>> + <attribute name="productId" type="String" mode="IN" >>>> optional="false"/> >>>> + <attribute name="quantity" type="BigDecimal" mode="IN" >>>> optional="false"/> >>>> + </service> >>>> + >>>> </services> >>>> >>>> Modified: >>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java >>>> >>>> URL: >>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java?rev=1635192&r1=1635191&r2=1635192&view=diff >>>> >>>> ============================================================================== >>>> >>>> --- >>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java >>>> (original) >>>> +++ >>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java >>>> Wed Oct 29 16:37:40 2014 >>>> @@ -280,15 +280,10 @@ public class OrderServices { >>>> normalizedItemQuantities.put(currentProductId, >>>> currentQuantity.add(orderItem.getBigDecimal("quantity"))); >>>> } >>>> >>>> - try { >>>> - // count product ordered quantities >>>> - // run this synchronously so it will run in the >>>> same transaction >>>> - dispatcher.runSync("countProductQuantityOrdered", >>>> UtilMisc.<String, Object>toMap("productId", currentProductId, >>>> "quantity", orderItem.getBigDecimal("quantity"), "userLogin", >>>> userLogin)); >>>> - } catch (GenericServiceException e1) { >>>> - Debug.logError(e1, "Error calling >>>> countProductQuantityOrdered service", module); >>>> - return >>>> ServiceUtil.returnError(UtilProperties.getMessage(resource_error, >>>> - "OrderErrorCallingCountProductQuantityOrderedService",locale) + >>>> e1.toString()); >>>> - } >>>> + Map<String, Object> countContext = new >>>> HashMap<String, Object>(); >>>> + countContext.put("productId", currentProductId); >>>> + countContext.put("quantity", >>>> orderItem.getBigDecimal("quantity")); >>>> + countProductQuantityOrdered(ctx, countContext); >>>> } >>>> } >>>> >>>> @@ -1152,6 +1147,54 @@ public class OrderServices { >>>> >>>> return successResult; >>>> } >>>> + >>>> + public static Map<String, Object> >>>> countProductQuantityOrdered(DispatchContext ctx, Map<String, Object> >>>> context) { >>>> + Delegator delegator = ctx.getDelegator(); >>>> + Locale locale = (Locale) context.get("locale"); >>>> + List<GenericValue> productCalculatedInfoList = null; >>>> + GenericValue productCalculatedInfo = null; >>>> + String productId = (String) context.get("productId"); >>>> + BigDecimal quantity = (BigDecimal) context.get("quantity"); >>>> + try { >>>> + productCalculatedInfoList = >>>> delegator.findByAnd("ProductCalculatedInfo", >>>> UtilMisc.toMap("productId", productId), null, false); >>>> + if (UtilValidate.isEmpty(productCalculatedInfoList)) { >>>> + productCalculatedInfo = >>>> delegator.makeValue("ProductCalculatedInfo"); >>>> + productCalculatedInfo.set("productId", productId); >>>> + productCalculatedInfo.set("totalQuantityOrdered", quantity); >>>> + productCalculatedInfo.create(); >>>> + } else { >>>> + productCalculatedInfo = >>>> productCalculatedInfoList.get(0); >>>> + BigDecimal totalQuantityOrdered = >>>> productCalculatedInfo.getBigDecimal("totalQuantityOrdered"); >>>> + if (totalQuantityOrdered == null) { >>>> + productCalculatedInfo.set("totalQuantityOrdered", quantity); >>>> + } else { >>>> + productCalculatedInfo.set("totalQuantityOrdered", >>>> totalQuantityOrdered.add(quantity)); >>>> + } >>>> + } >>>> + productCalculatedInfo.store(); >>>> + } catch (GenericEntityException e) { >>>> + Debug.logError(e, "Error calling >>>> countProductQuantityOrdered service", module); >>>> + return >>>> ServiceUtil.returnError(UtilProperties.getMessage(resource_error, >>>> + "OrderErrorCallingCountProductQuantityOrderedService",locale) + >>>> e.toString()); >>>> + >>>> + } >>>> + >>>> + String virtualProductId = null; >>>> + try { >>>> + GenericValue product = delegator.findOne("Product", >>>> UtilMisc.toMap("productId", productId), true); >>>> + virtualProductId = >>>> ProductWorker.getVariantVirtualId(product); >>>> + } catch (GenericEntityException e) { >>>> + Debug.logError(e, "Error calling >>>> countProductQuantityOrdered service", module); >>>> + return >>>> ServiceUtil.returnError(UtilProperties.getMessage(resource_error, >>>> + "OrderErrorCallingCountProductQuantityOrderedService",locale) + >>>> e.toString()); >>>> + } >>>> + >>>> + if (UtilValidate.isNotEmpty(virtualProductId)) { >>>> + context.put("productId", virtualProductId); >>>> + countProductQuantityOrdered(ctx, context); >>>> + } >>>> + return ServiceUtil.returnSuccess(); >>>> + } >>>> >>>> public static void reserveInventory(Delegator delegator, >>>> LocalDispatcher dispatcher, GenericValue userLogin, Locale locale, >>>> List<GenericValue> orderItemShipGroupInfo, List<String> >>>> dropShipGroupIds, Map<String, GenericValue> itemValuesBySeqId, >>>> String orderTypeId, String productStoreId, List<String> >>>> resErrorMessages) throws GeneralException { >>>> boolean isImmediatelyFulfilled = false; >>>> @@ -1729,7 +1772,7 @@ public class OrderServices { >>>> if (UtilValidate.isNotEmpty(orderItemSeqId)) { >>>> createOrderAdjContext.put("orderItemSeqId", orderItemSeqId); >>>> } else { >>>> - createOrderAdjContext.put("orderItemSeqId", "_NA_"); >>>> + createOrderAdjContext.put("orderItemSeqId", "_NA_"); >>>> } >>>> createOrderAdjContext.put("shipGroupSeqId", "_NA_"); >>>> createOrderAdjContext.put("description", "Tax >>>> adjustment due to order change"); >>>> @@ -5787,4 +5830,4 @@ public class OrderServices { >>>> >>>> return ServiceUtil.returnSuccess(); >>>> } >>>> -} >>>> \ No newline at end of file >>>> +} >>>> >>>> Modified: >>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java >>>> >>>> URL: >>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java?rev=1635192&r1=1635191&r2=1635192&view=diff >>>> >>>> ============================================================================== >>>> >>>> --- >>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java >>>> (original) >>>> +++ >>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java >>>> Wed Oct 29 16:37:40 2014 >>>> @@ -114,8 +114,16 @@ public class SalesOrderTest extends OFBi >>>> orderItem.set("unitPrice", new BigDecimal("38.4")); >>>> orderItem.set("unitListPrice", new BigDecimal("48.0")); >>>> orderItem.set("statusId", "ITEM_CREATED"); >>>> - >>>> orderItems.add(orderItem); >>>> + >>>> + orderItem = delegator.makeValue("OrderItem", >>>> UtilMisc.toMap("orderItemSeqId", "00002", "orderItemTypeId", >>>> "PRODUCT_ORDER_ITEM", "prodCatalogId", "DemoCatalog", "productId", >>>> "GZ-1006-1", "quantity", BigDecimal.ONE, "selectedAmount", >>>> BigDecimal.ZERO)); >>>> + orderItem.set("isPromo", "N"); >>>> + orderItem.set("isModifiedPrice", "N"); >>>> + orderItem.set("unitPrice", new BigDecimal("1.99")); >>>> + orderItem.set("unitListPrice", new BigDecimal("5.99")); >>>> + orderItem.set("statusId", "ITEM_CREATED"); >>>> + orderItems.add(orderItem); >>>> + >>>> ctx.put("orderItems", orderItems); >>>> >>>> List<GenericValue> orderTerms = FastList.newInstance(); >>>> >>>> Modified: ofbiz/trunk/applications/product/servicedef/services.xml >>>> URL: >>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/servicedef/services.xml?rev=1635192&r1=1635191&r2=1635192&view=diff >>>> >>>> ============================================================================== >>>> >>>> --- ofbiz/trunk/applications/product/servicedef/services.xml (original) >>>> +++ ofbiz/trunk/applications/product/servicedef/services.xml Wed Oct >>>> 29 16:37:40 2014 >>>> @@ -175,13 +175,7 @@ under the License. >>>> <attribute name="productId" type="String" mode="IN" >>>> optional="false"/> >>>> <attribute name="weight" type="Long" mode="IN" >>>> optional="true"/> >>>> </service> >>>> - <service name="countProductQuantityOrdered" engine="simple" >>>> - >>>> location="component://product/script/org/ofbiz/product/product/ProductServices.xml" >>>> invoke="countProductQuantityOrdered" auth="true"> >>>> - <description>count Product Quantity Ordered</description> >>>> - <attribute name="productId" type="String" mode="IN" >>>> optional="false"/> >>>> - <attribute name="quantity" type="BigDecimal" mode="IN" >>>> optional="false"/> >>>> - </service> >>>> - >>>> + >>>> <service name="createProductReview" engine="simple" >>>> location="component://product/script/org/ofbiz/product/product/ProductServices.xml" >>>> invoke="createProductReview" auth="true"> >>>> <description>Create a product review entity</description> >>>> >>>> >>> >> |
Re: svn commit: r1635192 - in /ofbiz/trunk/applications: order/servicedef/services.xml order/src/org/ofbiz/order/orde
|
|
In reply to this post by Jacques Le Roux
This commit is an ugly hack. Why are you defending it? Just fix it.
Adrian Crum Sandglass Software www.sandglass-software.com On 10/29/2014 7:37 PM, Jacques Le Roux wrote: > > Le 29/10/2014 19:22, Adrian Crum a écrit : >> I'm not Jacques, but I'm pretty sure there is very little difference. >> The disadvantage to hacks like this: Bypassing the service engine like >> this will prevent any SECAs attached to the called service from being >> invoked. > > OOTB there are any SECAs attached to the countProductQuantityOrdered. > And I doubt there are in custom projects, they would be quite > convoluted. ProductCalculatedInfo entity is destined for stats and > reports. I don't see it used in business workflows: after my change > this service is not used OOTB. I'm not even sure we need this service! > > Jacques > >> >> Adrian Crum >> Sandglass Software >> www.sandglass-software.com >> >> On 10/29/2014 5:50 PM, Jacopo Cappellato wrote: >>> Jacques, >>> >>> what was the measured difference between the method call to the Java >>> method and the service call to the Java implementation? >>> >>> Jacopo >>> >>> On Oct 29, 2014, at 5:37 PM, [hidden email] wrote: >>> >>>> Author: jleroux >>>> Date: Wed Oct 29 16:37:40 2014 >>>> New Revision: 1635192 >>>> >>>> URL: http://svn.apache.org/r1635192 >>>> Log: >>>> The storeOrder service, implemented by the >>>> OrderService.createOrder() method, synchronously calls the >>>> countProductQuantityOrdered service implemented in Minilang. OOTB, >>>> the countProductQuantityOrdered service is only called by the >>>> storeOrder service implementation. It's called inside a loop on >>>> orderItems. >>>> >>>> While intentionally load testing a custom project with JMeter on a >>>> weak m1.small AWS machine, I noticed the overhead of the service >>>> call for each order item iteration was significant on this slow >>>> machine. With only 30 concurrent users the process was blocked by a >>>> timeout on the countProductQuantityOrdered service. >>>> >>>> I then decided to transform the minilang code into an >>>> OrderService.countProductQuantityOrdered() method that can be >>>> directly called inside OrderService.createOrder() and also >>>> implements the countProductQuantityOrdered service. Hence it avoids >>>> the overhead of the minilang service calls in loop while still >>>> providing a countProductQuantityOrdered service for possible >>>> external uses. For that I moved the definition from Product >>>> component to Order component to avoid the hard coded dependency of >>>> Order to Product. I then did not cross issues with the same load test. >>>> >>>> I also added a test for the countProductQuantityOrdered service by >>>> adding an order item of a virtual product (GZ-1006-1) in >>>> SalesOrderTest. >>>> >>>> Modified: >>>> ofbiz/trunk/applications/order/servicedef/services.xml >>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java >>>> >>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java >>>> >>>> ofbiz/trunk/applications/product/servicedef/services.xml >>>> >>>> Modified: ofbiz/trunk/applications/order/servicedef/services.xml >>>> URL: >>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/servicedef/services.xml?rev=1635192&r1=1635191&r2=1635192&view=diff >>>> >>>> ============================================================================== >>>> >>>> --- ofbiz/trunk/applications/order/servicedef/services.xml (original) >>>> +++ ofbiz/trunk/applications/order/servicedef/services.xml Wed Oct >>>> 29 16:37:40 2014 >>>> @@ -1147,4 +1147,11 @@ under the License. >>>> <auto-attributes mode="IN" >>>> entity-name="OrderItemGroupOrder" include="pk" optional="false"/> >>>> </service> >>>> >>>> + <service name="countProductQuantityOrdered" engine="java" >>>> + location="org.ofbiz.order.order.OrderServices" >>>> invoke="countProductQuantityOrdered" auth="true"> >>>> + <description>count Product Quantity Ordered</description> >>>> + <attribute name="productId" type="String" mode="IN" >>>> optional="false"/> >>>> + <attribute name="quantity" type="BigDecimal" mode="IN" >>>> optional="false"/> >>>> + </service> >>>> + >>>> </services> >>>> >>>> Modified: >>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java >>>> >>>> URL: >>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java?rev=1635192&r1=1635191&r2=1635192&view=diff >>>> >>>> ============================================================================== >>>> >>>> --- >>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java >>>> (original) >>>> +++ >>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java >>>> Wed Oct 29 16:37:40 2014 >>>> @@ -280,15 +280,10 @@ public class OrderServices { >>>> normalizedItemQuantities.put(currentProductId, >>>> currentQuantity.add(orderItem.getBigDecimal("quantity"))); >>>> } >>>> >>>> - try { >>>> - // count product ordered quantities >>>> - // run this synchronously so it will run in the >>>> same transaction >>>> - dispatcher.runSync("countProductQuantityOrdered", >>>> UtilMisc.<String, Object>toMap("productId", currentProductId, >>>> "quantity", orderItem.getBigDecimal("quantity"), "userLogin", >>>> userLogin)); >>>> - } catch (GenericServiceException e1) { >>>> - Debug.logError(e1, "Error calling >>>> countProductQuantityOrdered service", module); >>>> - return >>>> ServiceUtil.returnError(UtilProperties.getMessage(resource_error, >>>> - "OrderErrorCallingCountProductQuantityOrderedService",locale) + >>>> e1.toString()); >>>> - } >>>> + Map<String, Object> countContext = new >>>> HashMap<String, Object>(); >>>> + countContext.put("productId", currentProductId); >>>> + countContext.put("quantity", >>>> orderItem.getBigDecimal("quantity")); >>>> + countProductQuantityOrdered(ctx, countContext); >>>> } >>>> } >>>> >>>> @@ -1152,6 +1147,54 @@ public class OrderServices { >>>> >>>> return successResult; >>>> } >>>> + >>>> + public static Map<String, Object> >>>> countProductQuantityOrdered(DispatchContext ctx, Map<String, Object> >>>> context) { >>>> + Delegator delegator = ctx.getDelegator(); >>>> + Locale locale = (Locale) context.get("locale"); >>>> + List<GenericValue> productCalculatedInfoList = null; >>>> + GenericValue productCalculatedInfo = null; >>>> + String productId = (String) context.get("productId"); >>>> + BigDecimal quantity = (BigDecimal) context.get("quantity"); >>>> + try { >>>> + productCalculatedInfoList = >>>> delegator.findByAnd("ProductCalculatedInfo", >>>> UtilMisc.toMap("productId", productId), null, false); >>>> + if (UtilValidate.isEmpty(productCalculatedInfoList)) { >>>> + productCalculatedInfo = >>>> delegator.makeValue("ProductCalculatedInfo"); >>>> + productCalculatedInfo.set("productId", productId); >>>> + productCalculatedInfo.set("totalQuantityOrdered", quantity); >>>> + productCalculatedInfo.create(); >>>> + } else { >>>> + productCalculatedInfo = >>>> productCalculatedInfoList.get(0); >>>> + BigDecimal totalQuantityOrdered = >>>> productCalculatedInfo.getBigDecimal("totalQuantityOrdered"); >>>> + if (totalQuantityOrdered == null) { >>>> + productCalculatedInfo.set("totalQuantityOrdered", quantity); >>>> + } else { >>>> + productCalculatedInfo.set("totalQuantityOrdered", >>>> totalQuantityOrdered.add(quantity)); >>>> + } >>>> + } >>>> + productCalculatedInfo.store(); >>>> + } catch (GenericEntityException e) { >>>> + Debug.logError(e, "Error calling >>>> countProductQuantityOrdered service", module); >>>> + return >>>> ServiceUtil.returnError(UtilProperties.getMessage(resource_error, >>>> + "OrderErrorCallingCountProductQuantityOrderedService",locale) + >>>> e.toString()); >>>> + >>>> + } >>>> + >>>> + String virtualProductId = null; >>>> + try { >>>> + GenericValue product = delegator.findOne("Product", >>>> UtilMisc.toMap("productId", productId), true); >>>> + virtualProductId = >>>> ProductWorker.getVariantVirtualId(product); >>>> + } catch (GenericEntityException e) { >>>> + Debug.logError(e, "Error calling >>>> countProductQuantityOrdered service", module); >>>> + return >>>> ServiceUtil.returnError(UtilProperties.getMessage(resource_error, >>>> + "OrderErrorCallingCountProductQuantityOrderedService",locale) + >>>> e.toString()); >>>> + } >>>> + >>>> + if (UtilValidate.isNotEmpty(virtualProductId)) { >>>> + context.put("productId", virtualProductId); >>>> + countProductQuantityOrdered(ctx, context); >>>> + } >>>> + return ServiceUtil.returnSuccess(); >>>> + } >>>> >>>> public static void reserveInventory(Delegator delegator, >>>> LocalDispatcher dispatcher, GenericValue userLogin, Locale locale, >>>> List<GenericValue> orderItemShipGroupInfo, List<String> >>>> dropShipGroupIds, Map<String, GenericValue> itemValuesBySeqId, >>>> String orderTypeId, String productStoreId, List<String> >>>> resErrorMessages) throws GeneralException { >>>> boolean isImmediatelyFulfilled = false; >>>> @@ -1729,7 +1772,7 @@ public class OrderServices { >>>> if (UtilValidate.isNotEmpty(orderItemSeqId)) { >>>> createOrderAdjContext.put("orderItemSeqId", orderItemSeqId); >>>> } else { >>>> - createOrderAdjContext.put("orderItemSeqId", "_NA_"); >>>> + createOrderAdjContext.put("orderItemSeqId", "_NA_"); >>>> } >>>> createOrderAdjContext.put("shipGroupSeqId", "_NA_"); >>>> createOrderAdjContext.put("description", "Tax >>>> adjustment due to order change"); >>>> @@ -5787,4 +5830,4 @@ public class OrderServices { >>>> >>>> return ServiceUtil.returnSuccess(); >>>> } >>>> -} >>>> \ No newline at end of file >>>> +} >>>> >>>> Modified: >>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java >>>> >>>> URL: >>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java?rev=1635192&r1=1635191&r2=1635192&view=diff >>>> >>>> ============================================================================== >>>> >>>> --- >>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java >>>> (original) >>>> +++ >>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java >>>> Wed Oct 29 16:37:40 2014 >>>> @@ -114,8 +114,16 @@ public class SalesOrderTest extends OFBi >>>> orderItem.set("unitPrice", new BigDecimal("38.4")); >>>> orderItem.set("unitListPrice", new BigDecimal("48.0")); >>>> orderItem.set("statusId", "ITEM_CREATED"); >>>> - >>>> orderItems.add(orderItem); >>>> + >>>> + orderItem = delegator.makeValue("OrderItem", >>>> UtilMisc.toMap("orderItemSeqId", "00002", "orderItemTypeId", >>>> "PRODUCT_ORDER_ITEM", "prodCatalogId", "DemoCatalog", "productId", >>>> "GZ-1006-1", "quantity", BigDecimal.ONE, "selectedAmount", >>>> BigDecimal.ZERO)); >>>> + orderItem.set("isPromo", "N"); >>>> + orderItem.set("isModifiedPrice", "N"); >>>> + orderItem.set("unitPrice", new BigDecimal("1.99")); >>>> + orderItem.set("unitListPrice", new BigDecimal("5.99")); >>>> + orderItem.set("statusId", "ITEM_CREATED"); >>>> + orderItems.add(orderItem); >>>> + >>>> ctx.put("orderItems", orderItems); >>>> >>>> List<GenericValue> orderTerms = FastList.newInstance(); >>>> >>>> Modified: ofbiz/trunk/applications/product/servicedef/services.xml >>>> URL: >>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/servicedef/services.xml?rev=1635192&r1=1635191&r2=1635192&view=diff >>>> >>>> ============================================================================== >>>> >>>> --- ofbiz/trunk/applications/product/servicedef/services.xml (original) >>>> +++ ofbiz/trunk/applications/product/servicedef/services.xml Wed Oct >>>> 29 16:37:40 2014 >>>> @@ -175,13 +175,7 @@ under the License. >>>> <attribute name="productId" type="String" mode="IN" >>>> optional="false"/> >>>> <attribute name="weight" type="Long" mode="IN" >>>> optional="true"/> >>>> </service> >>>> - <service name="countProductQuantityOrdered" engine="simple" >>>> - >>>> location="component://product/script/org/ofbiz/product/product/ProductServices.xml" >>>> invoke="countProductQuantityOrdered" auth="true"> >>>> - <description>count Product Quantity Ordered</description> >>>> - <attribute name="productId" type="String" mode="IN" >>>> optional="false"/> >>>> - <attribute name="quantity" type="BigDecimal" mode="IN" >>>> optional="false"/> >>>> - </service> >>>> - >>>> + >>>> <service name="createProductReview" engine="simple" >>>> location="component://product/script/org/ofbiz/product/product/ProductServices.xml" >>>> invoke="createProductReview" auth="true"> >>>> <description>Create a product review entity</description> >>>> >>>> >>> >> |
Re: svn commit: r1635192 - in /ofbiz/trunk/applications: order/servicedef/services.xml order/src/org/ofbiz/order/orde
|
Administrator
|
You mean by using DispatchContext? What does it add in this context?
Jacques Le 29/10/2014 20:44, Adrian Crum a écrit : > This commit is an ugly hack. Why are you defending it? Just fix it. > > Adrian Crum > Sandglass Software > www.sandglass-software.com > > On 10/29/2014 7:37 PM, Jacques Le Roux wrote: >> >> Le 29/10/2014 19:22, Adrian Crum a écrit : >>> I'm not Jacques, but I'm pretty sure there is very little difference. >>> The disadvantage to hacks like this: Bypassing the service engine like >>> this will prevent any SECAs attached to the called service from being >>> invoked. >> >> OOTB there are any SECAs attached to the countProductQuantityOrdered. >> And I doubt there are in custom projects, they would be quite >> convoluted. ProductCalculatedInfo entity is destined for stats and >> reports. I don't see it used in business workflows: after my change >> this service is not used OOTB. I'm not even sure we need this service! >> >> Jacques >> >>> >>> Adrian Crum >>> Sandglass Software >>> www.sandglass-software.com >>> >>> On 10/29/2014 5:50 PM, Jacopo Cappellato wrote: >>>> Jacques, >>>> >>>> what was the measured difference between the method call to the Java >>>> method and the service call to the Java implementation? >>>> >>>> Jacopo >>>> >>>> On Oct 29, 2014, at 5:37 PM, [hidden email] wrote: >>>> >>>>> Author: jleroux >>>>> Date: Wed Oct 29 16:37:40 2014 >>>>> New Revision: 1635192 >>>>> >>>>> URL: http://svn.apache.org/r1635192 >>>>> Log: >>>>> The storeOrder service, implemented by the >>>>> OrderService.createOrder() method, synchronously calls the >>>>> countProductQuantityOrdered service implemented in Minilang. OOTB, >>>>> the countProductQuantityOrdered service is only called by the >>>>> storeOrder service implementation. It's called inside a loop on >>>>> orderItems. >>>>> >>>>> While intentionally load testing a custom project with JMeter on a >>>>> weak m1.small AWS machine, I noticed the overhead of the service >>>>> call for each order item iteration was significant on this slow >>>>> machine. With only 30 concurrent users the process was blocked by a >>>>> timeout on the countProductQuantityOrdered service. >>>>> >>>>> I then decided to transform the minilang code into an >>>>> OrderService.countProductQuantityOrdered() method that can be >>>>> directly called inside OrderService.createOrder() and also >>>>> implements the countProductQuantityOrdered service. Hence it avoids >>>>> the overhead of the minilang service calls in loop while still >>>>> providing a countProductQuantityOrdered service for possible >>>>> external uses. For that I moved the definition from Product >>>>> component to Order component to avoid the hard coded dependency of >>>>> Order to Product. I then did not cross issues with the same load test. >>>>> >>>>> I also added a test for the countProductQuantityOrdered service by >>>>> adding an order item of a virtual product (GZ-1006-1) in >>>>> SalesOrderTest. >>>>> >>>>> Modified: >>>>> ofbiz/trunk/applications/order/servicedef/services.xml >>>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java >>>>> >>>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java >>>>> >>>>> ofbiz/trunk/applications/product/servicedef/services.xml >>>>> >>>>> Modified: ofbiz/trunk/applications/order/servicedef/services.xml >>>>> URL: >>>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/servicedef/services.xml?rev=1635192&r1=1635191&r2=1635192&view=diff >>>>> >>>>> ============================================================================== >>>>> >>>>> --- ofbiz/trunk/applications/order/servicedef/services.xml (original) >>>>> +++ ofbiz/trunk/applications/order/servicedef/services.xml Wed Oct >>>>> 29 16:37:40 2014 >>>>> @@ -1147,4 +1147,11 @@ under the License. >>>>> <auto-attributes mode="IN" >>>>> entity-name="OrderItemGroupOrder" include="pk" optional="false"/> >>>>> </service> >>>>> >>>>> + <service name="countProductQuantityOrdered" engine="java" >>>>> + location="org.ofbiz.order.order.OrderServices" >>>>> invoke="countProductQuantityOrdered" auth="true"> >>>>> + <description>count Product Quantity Ordered</description> >>>>> + <attribute name="productId" type="String" mode="IN" >>>>> optional="false"/> >>>>> + <attribute name="quantity" type="BigDecimal" mode="IN" >>>>> optional="false"/> >>>>> + </service> >>>>> + >>>>> </services> >>>>> >>>>> Modified: >>>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java >>>>> >>>>> URL: >>>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java?rev=1635192&r1=1635191&r2=1635192&view=diff >>>>> >>>>> >>>>> ============================================================================== >>>>> >>>>> --- >>>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java >>>>> (original) >>>>> +++ >>>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java >>>>> Wed Oct 29 16:37:40 2014 >>>>> @@ -280,15 +280,10 @@ public class OrderServices { >>>>> normalizedItemQuantities.put(currentProductId, >>>>> currentQuantity.add(orderItem.getBigDecimal("quantity"))); >>>>> } >>>>> >>>>> - try { >>>>> - // count product ordered quantities >>>>> - // run this synchronously so it will run in the >>>>> same transaction >>>>> - dispatcher.runSync("countProductQuantityOrdered", >>>>> UtilMisc.<String, Object>toMap("productId", currentProductId, >>>>> "quantity", orderItem.getBigDecimal("quantity"), "userLogin", >>>>> userLogin)); >>>>> - } catch (GenericServiceException e1) { >>>>> - Debug.logError(e1, "Error calling >>>>> countProductQuantityOrdered service", module); >>>>> - return >>>>> ServiceUtil.returnError(UtilProperties.getMessage(resource_error, >>>>> - "OrderErrorCallingCountProductQuantityOrderedService",locale) + >>>>> e1.toString()); >>>>> - } >>>>> + Map<String, Object> countContext = new >>>>> HashMap<String, Object>(); >>>>> + countContext.put("productId", currentProductId); >>>>> + countContext.put("quantity", >>>>> orderItem.getBigDecimal("quantity")); >>>>> + countProductQuantityOrdered(ctx, countContext); >>>>> } >>>>> } >>>>> >>>>> @@ -1152,6 +1147,54 @@ public class OrderServices { >>>>> >>>>> return successResult; >>>>> } >>>>> + >>>>> + public static Map<String, Object> >>>>> countProductQuantityOrdered(DispatchContext ctx, Map<String, Object> >>>>> context) { >>>>> + Delegator delegator = ctx.getDelegator(); >>>>> + Locale locale = (Locale) context.get("locale"); >>>>> + List<GenericValue> productCalculatedInfoList = null; >>>>> + GenericValue productCalculatedInfo = null; >>>>> + String productId = (String) context.get("productId"); >>>>> + BigDecimal quantity = (BigDecimal) context.get("quantity"); >>>>> + try { >>>>> + productCalculatedInfoList = >>>>> delegator.findByAnd("ProductCalculatedInfo", >>>>> UtilMisc.toMap("productId", productId), null, false); >>>>> + if (UtilValidate.isEmpty(productCalculatedInfoList)) { >>>>> + productCalculatedInfo = >>>>> delegator.makeValue("ProductCalculatedInfo"); >>>>> + productCalculatedInfo.set("productId", productId); >>>>> + productCalculatedInfo.set("totalQuantityOrdered", quantity); >>>>> + productCalculatedInfo.create(); >>>>> + } else { >>>>> + productCalculatedInfo = >>>>> productCalculatedInfoList.get(0); >>>>> + BigDecimal totalQuantityOrdered = >>>>> productCalculatedInfo.getBigDecimal("totalQuantityOrdered"); >>>>> + if (totalQuantityOrdered == null) { >>>>> + productCalculatedInfo.set("totalQuantityOrdered", quantity); >>>>> + } else { >>>>> + productCalculatedInfo.set("totalQuantityOrdered", >>>>> totalQuantityOrdered.add(quantity)); >>>>> + } >>>>> + } >>>>> + productCalculatedInfo.store(); >>>>> + } catch (GenericEntityException e) { >>>>> + Debug.logError(e, "Error calling >>>>> countProductQuantityOrdered service", module); >>>>> + return >>>>> ServiceUtil.returnError(UtilProperties.getMessage(resource_error, >>>>> + "OrderErrorCallingCountProductQuantityOrderedService",locale) + >>>>> e.toString()); >>>>> + >>>>> + } >>>>> + >>>>> + String virtualProductId = null; >>>>> + try { >>>>> + GenericValue product = delegator.findOne("Product", >>>>> UtilMisc.toMap("productId", productId), true); >>>>> + virtualProductId = >>>>> ProductWorker.getVariantVirtualId(product); >>>>> + } catch (GenericEntityException e) { >>>>> + Debug.logError(e, "Error calling >>>>> countProductQuantityOrdered service", module); >>>>> + return >>>>> ServiceUtil.returnError(UtilProperties.getMessage(resource_error, >>>>> + "OrderErrorCallingCountProductQuantityOrderedService",locale) + >>>>> e.toString()); >>>>> + } >>>>> + >>>>> + if (UtilValidate.isNotEmpty(virtualProductId)) { >>>>> + context.put("productId", virtualProductId); >>>>> + countProductQuantityOrdered(ctx, context); >>>>> + } >>>>> + return ServiceUtil.returnSuccess(); >>>>> + } >>>>> >>>>> public static void reserveInventory(Delegator delegator, >>>>> LocalDispatcher dispatcher, GenericValue userLogin, Locale locale, >>>>> List<GenericValue> orderItemShipGroupInfo, List<String> >>>>> dropShipGroupIds, Map<String, GenericValue> itemValuesBySeqId, >>>>> String orderTypeId, String productStoreId, List<String> >>>>> resErrorMessages) throws GeneralException { >>>>> boolean isImmediatelyFulfilled = false; >>>>> @@ -1729,7 +1772,7 @@ public class OrderServices { >>>>> if (UtilValidate.isNotEmpty(orderItemSeqId)) { >>>>> createOrderAdjContext.put("orderItemSeqId", orderItemSeqId); >>>>> } else { >>>>> - createOrderAdjContext.put("orderItemSeqId", "_NA_"); >>>>> + createOrderAdjContext.put("orderItemSeqId", "_NA_"); >>>>> } >>>>> createOrderAdjContext.put("shipGroupSeqId", "_NA_"); >>>>> createOrderAdjContext.put("description", "Tax >>>>> adjustment due to order change"); >>>>> @@ -5787,4 +5830,4 @@ public class OrderServices { >>>>> >>>>> return ServiceUtil.returnSuccess(); >>>>> } >>>>> -} >>>>> \ No newline at end of file >>>>> +} >>>>> >>>>> Modified: >>>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java >>>>> >>>>> URL: >>>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java?rev=1635192&r1=1635191&r2=1635192&view=diff >>>>> >>>>> >>>>> ============================================================================== >>>>> >>>>> --- >>>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java >>>>> (original) >>>>> +++ >>>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java >>>>> Wed Oct 29 16:37:40 2014 >>>>> @@ -114,8 +114,16 @@ public class SalesOrderTest extends OFBi >>>>> orderItem.set("unitPrice", new BigDecimal("38.4")); >>>>> orderItem.set("unitListPrice", new BigDecimal("48.0")); >>>>> orderItem.set("statusId", "ITEM_CREATED"); >>>>> - >>>>> orderItems.add(orderItem); >>>>> + >>>>> + orderItem = delegator.makeValue("OrderItem", >>>>> UtilMisc.toMap("orderItemSeqId", "00002", "orderItemTypeId", >>>>> "PRODUCT_ORDER_ITEM", "prodCatalogId", "DemoCatalog", "productId", >>>>> "GZ-1006-1", "quantity", BigDecimal.ONE, "selectedAmount", >>>>> BigDecimal.ZERO)); >>>>> + orderItem.set("isPromo", "N"); >>>>> + orderItem.set("isModifiedPrice", "N"); >>>>> + orderItem.set("unitPrice", new BigDecimal("1.99")); >>>>> + orderItem.set("unitListPrice", new BigDecimal("5.99")); >>>>> + orderItem.set("statusId", "ITEM_CREATED"); >>>>> + orderItems.add(orderItem); >>>>> + >>>>> ctx.put("orderItems", orderItems); >>>>> >>>>> List<GenericValue> orderTerms = FastList.newInstance(); >>>>> >>>>> Modified: ofbiz/trunk/applications/product/servicedef/services.xml >>>>> URL: >>>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/servicedef/services.xml?rev=1635192&r1=1635191&r2=1635192&view=diff >>>>> >>>>> ============================================================================== >>>>> >>>>> --- ofbiz/trunk/applications/product/servicedef/services.xml (original) >>>>> +++ ofbiz/trunk/applications/product/servicedef/services.xml Wed Oct >>>>> 29 16:37:40 2014 >>>>> @@ -175,13 +175,7 @@ under the License. >>>>> <attribute name="productId" type="String" mode="IN" >>>>> optional="false"/> >>>>> <attribute name="weight" type="Long" mode="IN" >>>>> optional="true"/> >>>>> </service> >>>>> - <service name="countProductQuantityOrdered" engine="simple" >>>>> - >>>>> location="component://product/script/org/ofbiz/product/product/ProductServices.xml" >>>>> invoke="countProductQuantityOrdered" auth="true"> >>>>> - <description>count Product Quantity Ordered</description> >>>>> - <attribute name="productId" type="String" mode="IN" >>>>> optional="false"/> >>>>> - <attribute name="quantity" type="BigDecimal" mode="IN" >>>>> optional="false"/> >>>>> - </service> >>>>> - >>>>> + >>>>> <service name="createProductReview" engine="simple" >>>>> location="component://product/script/org/ofbiz/product/product/ProductServices.xml" >>>>> invoke="createProductReview" auth="true"> >>>>> <description>Create a product review entity</description> >>>>> >>>>> >>>> >>> > |
Re: svn commit: r1635192 - in /ofbiz/trunk/applications: order/servicedef/services.xml order/src/org/ofbiz/order/orde
|
|
In reply to this post by Jacques Le Roux
On Oct 29, 2014, at 8:24 PM, Jacques Le Roux <[hidden email]> wrote:
> Apart what I reported in the commit comment and the Jira description, I did not make other measures. Before it was blocking the JMeter test after it passed. I suspect that the main difference could be Minilang vs Java, not between service vs method call: this is why I think it would be interesting to test the latter (java service vs java method). We should not switch to direct Java method calls so lightly, at least not before trying our best to find the root causes (in the service call chain) of the low performance. This effort would (if we find the bottleneck in the service call and manage to fix it) improve the whole system in one shot rather just fixing one service. Even if the difference is Minilang vs Java, it may be interesting to try to figure out what is the reason for it (I suspect it could be the larger number of reflexion) and try to fix it. I am willing to help you in these tasks but in the meantime I would change you latest code to use a service call (to Java) rather than a direct method call, mostly for consistency with the rest of the codebase. Kind regards, Jacopo |
Re: svn commit: r1635192 - in /ofbiz/trunk/applications: order/servicedef/services.xml order/src/org/ofbiz/order/orde
|
|
Thanks Jacopo.
I noticed the Mini-language service uses two deprecated elements: https://cwiki.apache.org/confluence/display/OFBADMIN/Mini-language+%28minilang%29+Reference <calculate> should be replaced with <set> and <call-class-method> should be replaced with <script>. Perhaps if that service was updated performance would improve. Adrian Crum Sandglass Software www.sandglass-software.com On 10/30/2014 5:57 AM, Jacopo Cappellato wrote: > On Oct 29, 2014, at 8:24 PM, Jacques Le Roux <[hidden email]> wrote: > >> Apart what I reported in the commit comment and the Jira description, I did not make other measures. Before it was blocking the JMeter test after it passed. > > I suspect that the main difference could be Minilang vs Java, not between service vs method call: this is why I think it would be interesting to test the latter (java service vs java method). > We should not switch to direct Java method calls so lightly, at least not before trying our best to find the root causes (in the service call chain) of the low performance. > This effort would (if we find the bottleneck in the service call and manage to fix it) improve the whole system in one shot rather just fixing one service. > Even if the difference is Minilang vs Java, it may be interesting to try to figure out what is the reason for it (I suspect it could be the larger number of reflexion) and try to fix it. > I am willing to help you in these tasks but in the meantime I would change you latest code to use a service call (to Java) rather than a direct method call, mostly for consistency with the rest of the codebase. > > Kind regards, > > Jacopo > |
Re: svn commit: r1635192 - in /ofbiz/trunk/applications: order/servicedef/services.xml order/src/org/ofbiz/order/orde
|
Administrator
|
In reply to this post by Jacopo Cappellato-4
OK fine, I can try to keep the service call easily here, I will let you know
Jacques Le 30/10/2014 06:57, Jacopo Cappellato a écrit : > On Oct 29, 2014, at 8:24 PM, Jacques Le Roux <[hidden email]> wrote: > >> Apart what I reported in the commit comment and the Jira description, I did not make other measures. Before it was blocking the JMeter test after it passed. > I suspect that the main difference could be Minilang vs Java, not between service vs method call: this is why I think it would be interesting to test the latter (java service vs java method). > We should not switch to direct Java method calls so lightly, at least not before trying our best to find the root causes (in the service call chain) of the low performance. > This effort would (if we find the bottleneck in the service call and manage to fix it) improve the whole system in one shot rather just fixing one service. > Even if the difference is Minilang vs Java, it may be interesting to try to figure out what is the reason for it (I suspect it could be the larger number of reflexion) and try to fix it. > I am willing to help you in these tasks but in the meantime I would change you latest code to use a service call (to Java) rather than a direct method call, mostly for consistency with the rest of the codebase. > > Kind regards, > > Jacopo > > > |
Re: svn commit: r1635192 - in /ofbiz/trunk/applications: order/servicedef/services.xml order/src/org/ofbiz/order/orde
|
|
In reply to this post by Adrian Crum-3
I have serious doubts there's any significant performance problems with this simple method. Given this problem occurred during load testing I would be almost certain that the problem is lock contention on the ProductCalculatedInfo records.
It's the same old story: - Long running order creation transaction - Updating a record that will be touched by many order creation transactions, causing a lock on the record until the transaction is committed - The transactions queue up, eventually they have to wait too long and the lock wait timer runs out or the transaction timeout is hit The service should be executed in separate transaction after the order submission transaction is committed. Regards Scott On 30/10/2014, at 7:23 pm, Adrian Crum <[hidden email]> wrote: > Thanks Jacopo. > > I noticed the Mini-language service uses two deprecated elements: > > https://cwiki.apache.org/confluence/display/OFBADMIN/Mini-language+%28minilang%29+Reference > > <calculate> should be replaced with <set> and <call-class-method> should be replaced with <script>. > > Perhaps if that service was updated performance would improve. > > Adrian Crum > Sandglass Software > www.sandglass-software.com > > On 10/30/2014 5:57 AM, Jacopo Cappellato wrote: >> On Oct 29, 2014, at 8:24 PM, Jacques Le Roux <[hidden email]> wrote: >> >>> Apart what I reported in the commit comment and the Jira description, I did not make other measures. Before it was blocking the JMeter test after it passed. >> >> I suspect that the main difference could be Minilang vs Java, not between service vs method call: this is why I think it would be interesting to test the latter (java service vs java method). >> We should not switch to direct Java method calls so lightly, at least not before trying our best to find the root causes (in the service call chain) of the low performance. >> This effort would (if we find the bottleneck in the service call and manage to fix it) improve the whole system in one shot rather just fixing one service. >> Even if the difference is Minilang vs Java, it may be interesting to try to figure out what is the reason for it (I suspect it could be the larger number of reflexion) and try to fix it. >> I am willing to help you in these tasks but in the meantime I would change you latest code to use a service call (to Java) rather than a direct method call, mostly for consistency with the rest of the codebase. >> >> Kind regards, >> >> Jacopo >> |
Re: svn commit: r1635192 - in /ofbiz/trunk/applications: order/servicedef/services.xml order/src/org/ofbiz/order/orde
|
|
Or as an alternative it could be run in the same transaction right before committing, to reduce the amount of time the lock is held.
Regards Scott On 30/10/2014, at 7:35 pm, Scott Gray <[hidden email]> wrote: > I have serious doubts there's any significant performance problems with this simple method. Given this problem occurred during load testing I would be almost certain that the problem is lock contention on the ProductCalculatedInfo records. > > It's the same old story: > - Long running order creation transaction > - Updating a record that will be touched by many order creation transactions, causing a lock on the record until the transaction is committed > - The transactions queue up, eventually they have to wait too long and the lock wait timer runs out or the transaction timeout is hit > > The service should be executed in separate transaction after the order submission transaction is committed. > > Regards > Scott > > On 30/10/2014, at 7:23 pm, Adrian Crum <[hidden email]> wrote: > >> Thanks Jacopo. >> >> I noticed the Mini-language service uses two deprecated elements: >> >> https://cwiki.apache.org/confluence/display/OFBADMIN/Mini-language+%28minilang%29+Reference >> >> <calculate> should be replaced with <set> and <call-class-method> should be replaced with <script>. >> >> Perhaps if that service was updated performance would improve. >> >> Adrian Crum >> Sandglass Software >> www.sandglass-software.com >> >> On 10/30/2014 5:57 AM, Jacopo Cappellato wrote: >>> On Oct 29, 2014, at 8:24 PM, Jacques Le Roux <[hidden email]> wrote: >>> >>>> Apart what I reported in the commit comment and the Jira description, I did not make other measures. Before it was blocking the JMeter test after it passed. >>> >>> I suspect that the main difference could be Minilang vs Java, not between service vs method call: this is why I think it would be interesting to test the latter (java service vs java method). >>> We should not switch to direct Java method calls so lightly, at least not before trying our best to find the root causes (in the service call chain) of the low performance. >>> This effort would (if we find the bottleneck in the service call and manage to fix it) improve the whole system in one shot rather just fixing one service. >>> Even if the difference is Minilang vs Java, it may be interesting to try to figure out what is the reason for it (I suspect it could be the larger number of reflexion) and try to fix it. >>> I am willing to help you in these tasks but in the meantime I would change you latest code to use a service call (to Java) rather than a direct method call, mostly for consistency with the rest of the codebase. >>> >>> Kind regards, >>> >>> Jacopo >>> > |
Re: svn commit: r1635192 - in /ofbiz/trunk/applications: order/servicedef/services.xml order/src/org/ofbiz/order/orde
|
Administrator
|
In reply to this post by Scott Gray-2
I tend to agree, that's why I simply bypassed the service call. Since the ProductCalculatedInfo is only for recording matter a call after could be a
better solution. For now, I tried to change things as less as possible. Jacques Le 30/10/2014 07:35, Scott Gray a écrit : > I have serious doubts there's any significant performance problems with this simple method. Given this problem occurred during load testing I would be almost certain that the problem is lock contention on the ProductCalculatedInfo records. > > It's the same old story: > - Long running order creation transaction > - Updating a record that will be touched by many order creation transactions, causing a lock on the record until the transaction is committed > - The transactions queue up, eventually they have to wait too long and the lock wait timer runs out or the transaction timeout is hit > > The service should be executed in separate transaction after the order submission transaction is committed. > > Regards > Scott > > On 30/10/2014, at 7:23 pm, Adrian Crum <[hidden email]> wrote: > >> Thanks Jacopo. >> >> I noticed the Mini-language service uses two deprecated elements: >> >> https://cwiki.apache.org/confluence/display/OFBADMIN/Mini-language+%28minilang%29+Reference >> >> <calculate> should be replaced with <set> and <call-class-method> should be replaced with <script>. >> >> Perhaps if that service was updated performance would improve. >> >> Adrian Crum >> Sandglass Software >> www.sandglass-software.com >> >> On 10/30/2014 5:57 AM, Jacopo Cappellato wrote: >>> On Oct 29, 2014, at 8:24 PM, Jacques Le Roux <[hidden email]> wrote: >>> >>>> Apart what I reported in the commit comment and the Jira description, I did not make other measures. Before it was blocking the JMeter test after it passed. >>> I suspect that the main difference could be Minilang vs Java, not between service vs method call: this is why I think it would be interesting to test the latter (java service vs java method). >>> We should not switch to direct Java method calls so lightly, at least not before trying our best to find the root causes (in the service call chain) of the low performance. >>> This effort would (if we find the bottleneck in the service call and manage to fix it) improve the whole system in one shot rather just fixing one service. >>> Even if the difference is Minilang vs Java, it may be interesting to try to figure out what is the reason for it (I suspect it could be the larger number of reflexion) and try to fix it. >>> I am willing to help you in these tasks but in the meantime I would change you latest code to use a service call (to Java) rather than a direct method call, mostly for consistency with the rest of the codebase. >>> >>> Kind regards, >>> >>> Jacopo >>> > > |
Re: svn commit: r1635192 - in /ofbiz/trunk/applications: order/servicedef/services.xml order/src/org/ofbiz/order/orde
|
|
Service call or not, the update is still happening in the same transaction at the same point in the flow. I'd strongly suggest you keep trying your load tests as I really can't see how anything has changed significantly.
Regards Scott On 30/10/2014, at 8:40 pm, Jacques Le Roux <[hidden email]> wrote: > I tend to agree, that's why I simply bypassed the service call. Since the ProductCalculatedInfo is only for recording matter a call after could be a better solution. For now, I tried to change things as less as possible. > > Jacques > > Le 30/10/2014 07:35, Scott Gray a écrit : >> I have serious doubts there's any significant performance problems with this simple method. Given this problem occurred during load testing I would be almost certain that the problem is lock contention on the ProductCalculatedInfo records. >> >> It's the same old story: >> - Long running order creation transaction >> - Updating a record that will be touched by many order creation transactions, causing a lock on the record until the transaction is committed >> - The transactions queue up, eventually they have to wait too long and the lock wait timer runs out or the transaction timeout is hit >> >> The service should be executed in separate transaction after the order submission transaction is committed. >> >> Regards >> Scott >> On 30/10/2014, at 7:23 pm, Adrian Crum <[hidden email]> wrote: >> >>> Thanks Jacopo. >>> >>> I noticed the Mini-language service uses two deprecated elements: >>> >>> https://cwiki.apache.org/confluence/display/OFBADMIN/Mini-language+%28minilang%29+Reference >>> >>> <calculate> should be replaced with <set> and <call-class-method> should be replaced with <script>. >>> >>> Perhaps if that service was updated performance would improve. >>> >>> Adrian Crum >>> Sandglass Software >>> www.sandglass-software.com >>> >>> On 10/30/2014 5:57 AM, Jacopo Cappellato wrote: >>>> On Oct 29, 2014, at 8:24 PM, Jacques Le Roux <[hidden email]> wrote: >>>> >>>>> Apart what I reported in the commit comment and the Jira description, I did not make other measures. Before it was blocking the JMeter test after it passed. >>>> I suspect that the main difference could be Minilang vs Java, not between service vs method call: this is why I think it would be interesting to test the latter (java service vs java method). >>>> We should not switch to direct Java method calls so lightly, at least not before trying our best to find the root causes (in the service call chain) of the low performance. >>>> This effort would (if we find the bottleneck in the service call and manage to fix it) improve the whole system in one shot rather just fixing one service. >>>> Even if the difference is Minilang vs Java, it may be interesting to try to figure out what is the reason for it (I suspect it could be the larger number of reflexion) and try to fix it. >>>> I am willing to help you in these tasks but in the meantime I would change you latest code to use a service call (to Java) rather than a direct method call, mostly for consistency with the rest of the codebase. >>>> >>>> Kind regards, >>>> >>>> Jacopo >>>> >> >> |
Re: svn commit: r1635192 - in /ofbiz/trunk/applications: order/servicedef/services.xml order/src/org/ofbiz/order/orde
|
Administrator
|
Yes, you are right, I need to get deeper in this, but not at the moment
Jacques Le 30/10/2014 08:55, Scott Gray a écrit : > Service call or not, the update is still happening in the same transaction at the same point in the flow. I'd strongly suggest you keep trying your load tests as I really can't see how anything has changed significantly. > > Regards > Scott > > On 30/10/2014, at 8:40 pm, Jacques Le Roux <[hidden email]> wrote: > >> I tend to agree, that's why I simply bypassed the service call. Since the ProductCalculatedInfo is only for recording matter a call after could be a better solution. For now, I tried to change things as less as possible. >> >> Jacques >> >> Le 30/10/2014 07:35, Scott Gray a écrit : >>> I have serious doubts there's any significant performance problems with this simple method. Given this problem occurred during load testing I would be almost certain that the problem is lock contention on the ProductCalculatedInfo records. >>> >>> It's the same old story: >>> - Long running order creation transaction >>> - Updating a record that will be touched by many order creation transactions, causing a lock on the record until the transaction is committed >>> - The transactions queue up, eventually they have to wait too long and the lock wait timer runs out or the transaction timeout is hit >>> >>> The service should be executed in separate transaction after the order submission transaction is committed. >>> >>> Regards >>> Scott >>> On 30/10/2014, at 7:23 pm, Adrian Crum <[hidden email]> wrote: >>> >>>> Thanks Jacopo. >>>> >>>> I noticed the Mini-language service uses two deprecated elements: >>>> >>>> https://cwiki.apache.org/confluence/display/OFBADMIN/Mini-language+%28minilang%29+Reference >>>> >>>> <calculate> should be replaced with <set> and <call-class-method> should be replaced with <script>. >>>> >>>> Perhaps if that service was updated performance would improve. >>>> >>>> Adrian Crum >>>> Sandglass Software >>>> www.sandglass-software.com >>>> >>>> On 10/30/2014 5:57 AM, Jacopo Cappellato wrote: >>>>> On Oct 29, 2014, at 8:24 PM, Jacques Le Roux <[hidden email]> wrote: >>>>> >>>>>> Apart what I reported in the commit comment and the Jira description, I did not make other measures. Before it was blocking the JMeter test after it passed. >>>>> I suspect that the main difference could be Minilang vs Java, not between service vs method call: this is why I think it would be interesting to test the latter (java service vs java method). >>>>> We should not switch to direct Java method calls so lightly, at least not before trying our best to find the root causes (in the service call chain) of the low performance. >>>>> This effort would (if we find the bottleneck in the service call and manage to fix it) improve the whole system in one shot rather just fixing one service. >>>>> Even if the difference is Minilang vs Java, it may be interesting to try to figure out what is the reason for it (I suspect it could be the larger number of reflexion) and try to fix it. >>>>> I am willing to help you in these tasks but in the meantime I would change you latest code to use a service call (to Java) rather than a direct method call, mostly for consistency with the rest of the codebase. >>>>> >>>>> Kind regards, >>>>> >>>>> Jacopo >>>>> >>> > > |
Re: svn commit: r1635192 - in /ofbiz/trunk/applications: order/servicedef/services.xml order/src/org/ofbiz/order/orde
|
Administrator
|
In reply to this post by Scott Gray-2
At revision: 1635567, I have put back the service call since it did not make any differences with the direct method call. Using the same load test I
got no timeout like before with the Minilang implementation It must be run in the same transaction (else another thread might change the context) so your last proposition seems to be the right solution. I did not implement it though since I did not need it. Jacques Le 30/10/2014 08:40, Scott Gray a écrit : > Or as an alternative it could be run in the same transaction right before committing, to reduce the amount of time the lock is held. > > Regards > Scott > > On 30/10/2014, at 7:35 pm, Scott Gray <[hidden email]> wrote: > >> I have serious doubts there's any significant performance problems with this simple method. Given this problem occurred during load testing I would be almost certain that the problem is lock contention on the ProductCalculatedInfo records. >> >> It's the same old story: >> - Long running order creation transaction >> - Updating a record that will be touched by many order creation transactions, causing a lock on the record until the transaction is committed >> - The transactions queue up, eventually they have to wait too long and the lock wait timer runs out or the transaction timeout is hit >> >> The service should be executed in separate transaction after the order submission transaction is committed. >> >> Regards >> Scott >> >> On 30/10/2014, at 7:23 pm, Adrian Crum <[hidden email]> wrote: >> >>> Thanks Jacopo. >>> >>> I noticed the Mini-language service uses two deprecated elements: >>> >>> https://cwiki.apache.org/confluence/display/OFBADMIN/Mini-language+%28minilang%29+Reference >>> >>> <calculate> should be replaced with <set> and <call-class-method> should be replaced with <script>. >>> >>> Perhaps if that service was updated performance would improve. >>> >>> Adrian Crum >>> Sandglass Software >>> www.sandglass-software.com >>> >>> On 10/30/2014 5:57 AM, Jacopo Cappellato wrote: >>>> On Oct 29, 2014, at 8:24 PM, Jacques Le Roux <[hidden email]> wrote: >>>> >>>>> Apart what I reported in the commit comment and the Jira description, I did not make other measures. Before it was blocking the JMeter test after it passed. >>>> I suspect that the main difference could be Minilang vs Java, not between service vs method call: this is why I think it would be interesting to test the latter (java service vs java method). >>>> We should not switch to direct Java method calls so lightly, at least not before trying our best to find the root causes (in the service call chain) of the low performance. >>>> This effort would (if we find the bottleneck in the service call and manage to fix it) improve the whole system in one shot rather just fixing one service. >>>> Even if the difference is Minilang vs Java, it may be interesting to try to figure out what is the reason for it (I suspect it could be the larger number of reflexion) and try to fix it. >>>> I am willing to help you in these tasks but in the meantime I would change you latest code to use a service call (to Java) rather than a direct method call, mostly for consistency with the rest of the codebase. >>>> >>>> Kind regards, >>>> >>>> Jacopo >>>> > > |
«
Return to OFBiz - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |