You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Jacopo Cappellato <ja...@hotwaxmedia.com> on 2014/09/02 12:05:40 UTC

Proposal: set screen's use-transaction=false by default (instead of true as it is now)

Specifically, here is the change that I am suggesting:

Index: framework/widget/dtd/widget-screen.xsd
===================================================================
--- framework/widget/dtd/widget-screen.xsd	(revision 1621949)
+++ framework/widget/dtd/widget-screen.xsd	(working copy)
@@ -36,7 +36,7 @@
                     <xs:documentation>Transaction timeout in seconds</xs:documentation>
                 </xs:annotation>
             </xs:attribute>
-            <xs:attribute name="use-transaction" default="true">
+            <xs:attribute name="use-transaction" default="false">
                 <xs:simpleType>
                     <xs:restriction base="xs:token">
                         <xs:enumeration value="true" />


Right now a database transaction is started (if not already) every time a screen is rendered: this is costly and for most screens this is not needed.
The screens that need a transaction in place are the ones with data preparation scripts that get an EntityListIterator (without starting a transaction explicitly): without a transaction in place an error will be logged in the console with a message like:

ERROR: Cannot do a find that returns an EntityListIterator with no transaction in place. Wrap this call in a transaction.

In order to fix it we can add use-transaction="true" to the screen definition. It shouldn't be too difficult to spot most of the screens with some static code analysis and then fix the remaining ones as we use the system.

In my opinion this effort would be worth of the time spent because it will save db resources (especially in systems with high traffic, like ecommerce applications).

Jacopo

Re: Proposal: set screen's use-transaction=false by default (instead of true as it is now)

Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
Thank you Adrian,

in rev. 1622171 I have committed a test to confirm David's explanation and the test is successful.
So the current default (use-transaction="true") seems to be indeed the best one.

Jacopo


On Sep 2, 2014, at 12:43 PM, Jacopo Cappellato <ja...@hotwaxmedia.com> wrote:

> very interesting, thank you Adrian.
> I didn't consider this aspect and I will definitely think (and possibly research) more about it.
> 
> Jacopo
> 
> 
> On Sep 2, 2014, at 12:17 PM, Adrian Crum <ad...@sandglass-software.com> wrote:
> 
>> I seem to recall David saying it is done that way to improve performance.
>> 
>> Most screens will have multiple Delegator calls to find records. If you look at the GenericDelegator find methods, a transaction is created if one does not already exist, the SELECT is performed, and the transaction is committed.
>> 
>> So, by making that attribute "false" - a screen will have many individual transactions instead of just one.
>> 
>> Adrian Crum
>> Sandglass Software
>> www.sandglass-software.com
>> 
>> On 9/2/2014 11:05 AM, Jacopo Cappellato wrote:
>>> Specifically, here is the change that I am suggesting:
>>> 
>>> Index: framework/widget/dtd/widget-screen.xsd
>>> ===================================================================
>>> --- framework/widget/dtd/widget-screen.xsd	(revision 1621949)
>>> +++ framework/widget/dtd/widget-screen.xsd	(working copy)
>>> @@ -36,7 +36,7 @@
>>>                     <xs:documentation>Transaction timeout in seconds</xs:documentation>
>>>                 </xs:annotation>
>>>             </xs:attribute>
>>> -            <xs:attribute name="use-transaction" default="true">
>>> +            <xs:attribute name="use-transaction" default="false">
>>>                 <xs:simpleType>
>>>                     <xs:restriction base="xs:token">
>>>                         <xs:enumeration value="true" />
>>> 
>>> 
>>> Right now a database transaction is started (if not already) every time a screen is rendered: this is costly and for most screens this is not needed.
>>> The screens that need a transaction in place are the ones with data preparation scripts that get an EntityListIterator (without starting a transaction explicitly): without a transaction in place an error will be logged in the console with a message like:
>>> 
>>> ERROR: Cannot do a find that returns an EntityListIterator with no transaction in place. Wrap this call in a transaction.
>>> 
>>> In order to fix it we can add use-transaction="true" to the screen definition. It shouldn't be too difficult to spot most of the screens with some static code analysis and then fix the remaining ones as we use the system.
>>> 
>>> In my opinion this effort would be worth of the time spent because it will save db resources (especially in systems with high traffic, like ecommerce applications).
>>> 
>>> Jacopo
>>> 
> 


Re: Proposal: set screen's use-transaction=false by default (instead of true as it is now)

Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
very interesting, thank you Adrian.
I didn't consider this aspect and I will definitely think (and possibly research) more about it.

Jacopo


On Sep 2, 2014, at 12:17 PM, Adrian Crum <ad...@sandglass-software.com> wrote:

> I seem to recall David saying it is done that way to improve performance.
> 
> Most screens will have multiple Delegator calls to find records. If you look at the GenericDelegator find methods, a transaction is created if one does not already exist, the SELECT is performed, and the transaction is committed.
> 
> So, by making that attribute "false" - a screen will have many individual transactions instead of just one.
> 
> Adrian Crum
> Sandglass Software
> www.sandglass-software.com
> 
> On 9/2/2014 11:05 AM, Jacopo Cappellato wrote:
>> Specifically, here is the change that I am suggesting:
>> 
>> Index: framework/widget/dtd/widget-screen.xsd
>> ===================================================================
>> --- framework/widget/dtd/widget-screen.xsd	(revision 1621949)
>> +++ framework/widget/dtd/widget-screen.xsd	(working copy)
>> @@ -36,7 +36,7 @@
>>                      <xs:documentation>Transaction timeout in seconds</xs:documentation>
>>                  </xs:annotation>
>>              </xs:attribute>
>> -            <xs:attribute name="use-transaction" default="true">
>> +            <xs:attribute name="use-transaction" default="false">
>>                  <xs:simpleType>
>>                      <xs:restriction base="xs:token">
>>                          <xs:enumeration value="true" />
>> 
>> 
>> Right now a database transaction is started (if not already) every time a screen is rendered: this is costly and for most screens this is not needed.
>> The screens that need a transaction in place are the ones with data preparation scripts that get an EntityListIterator (without starting a transaction explicitly): without a transaction in place an error will be logged in the console with a message like:
>> 
>> ERROR: Cannot do a find that returns an EntityListIterator with no transaction in place. Wrap this call in a transaction.
>> 
>> In order to fix it we can add use-transaction="true" to the screen definition. It shouldn't be too difficult to spot most of the screens with some static code analysis and then fix the remaining ones as we use the system.
>> 
>> In my opinion this effort would be worth of the time spent because it will save db resources (especially in systems with high traffic, like ecommerce applications).
>> 
>> Jacopo
>> 


Re: Proposal: set screen's use-transaction=false by default (instead of true as it is now)

Posted by Adrian Crum <ad...@sandglass-software.com>.
I seem to recall David saying it is done that way to improve performance.

Most screens will have multiple Delegator calls to find records. If you 
look at the GenericDelegator find methods, a transaction is created if 
one does not already exist, the SELECT is performed, and the transaction 
is committed.

So, by making that attribute "false" - a screen will have many 
individual transactions instead of just one.

Adrian Crum
Sandglass Software
www.sandglass-software.com

On 9/2/2014 11:05 AM, Jacopo Cappellato wrote:
> Specifically, here is the change that I am suggesting:
>
> Index: framework/widget/dtd/widget-screen.xsd
> ===================================================================
> --- framework/widget/dtd/widget-screen.xsd	(revision 1621949)
> +++ framework/widget/dtd/widget-screen.xsd	(working copy)
> @@ -36,7 +36,7 @@
>                       <xs:documentation>Transaction timeout in seconds</xs:documentation>
>                   </xs:annotation>
>               </xs:attribute>
> -            <xs:attribute name="use-transaction" default="true">
> +            <xs:attribute name="use-transaction" default="false">
>                   <xs:simpleType>
>                       <xs:restriction base="xs:token">
>                           <xs:enumeration value="true" />
>
>
> Right now a database transaction is started (if not already) every time a screen is rendered: this is costly and for most screens this is not needed.
> The screens that need a transaction in place are the ones with data preparation scripts that get an EntityListIterator (without starting a transaction explicitly): without a transaction in place an error will be logged in the console with a message like:
>
> ERROR: Cannot do a find that returns an EntityListIterator with no transaction in place. Wrap this call in a transaction.
>
> In order to fix it we can add use-transaction="true" to the screen definition. It shouldn't be too difficult to spot most of the screens with some static code analysis and then fix the remaining ones as we use the system.
>
> In my opinion this effort would be worth of the time spent because it will save db resources (especially in systems with high traffic, like ecommerce applications).
>
> Jacopo
>