[ 
https://issues.apache.org/jira/browse/TORQUE-372?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17948897#comment-17948897
 ] 

Georg Kallidis commented on TORQUE-372:
---------------------------------------

You may reintroduce this setting again IMO (the variable is still there in 
options.properties, see below). The concurrency problem in this issue is about 
the lazy initialization of the mappingStrategy, another question would be, if 
the order of the setters are important ..

It is a little bit difficult to understand technically, what useSimpleMapping 
is about. If set it adds an additional condition to a "simple" record mapping.

Until now useSimpleMapping  is set to true by default, but we may better not 
enabling it by default, as it seems for most eyes to be an advanced feature. I 
think I added the additional check, that SQL joins should be empty as otherwise 
this simple mapping will result in errors.

You may rename the variable as well and explain it in more detail, what it is 
about, where it is defined in svn 
[options.ps|https://svn.apache.org/viewvc/db/torque/trunk/torque-templates/src/main/resources/org/apache/torque/templates/om/conf/options.properties?view=markup]:
{code:bash}
# In processRow method use simpleMapping (default  true)
torque.om.simpleMapping = true
{code}


> Apache Torque 6 concurrency problem
> -----------------------------------
>
>                 Key: TORQUE-372
>                 URL: https://issues.apache.org/jira/browse/TORQUE-372
>             Project: Torque
>          Issue Type: Bug
>          Components: Templates
>    Affects Versions: 6.0
>            Reporter: Harald Nuding
>            Priority: Major
>             Fix For: 7.0
>
>         Attachments: recordMapperBase-1.vm
>
>
> This was provided as e-mail from Harald Nuding 4th of Feb 2025:
> {noformat}
> We’re working on a 3pp uplift of an old project based on Torque 5.1 towards 
> the new version 6.0.
> The migration was painless so far, so thank you all for the effort you spend! 
>  
> During regression testing we run into a concurrency problem.
> The problem happens when multiple treads parallel initialize the Recordmapper 
> of the same Table:
> java.util.ConcurrentModificationException: null
>                  at java.base/java.util.ArrayList.sort(ArrayList.java:1723) 
> ~[?:?]
>                  at 
> org.apache.torque.om.mapper.MappingStrategy.finish(MappingStrategy.java:70) 
> ~[torque-runtime-6.0.jar:6.0]
>                  at 
> app.backend.torque.BaseZoneRecordMapper.processRow(BaseZoneRecordMapper.java:635)
>  ~[classes/:?]
>                  at 
> app.backend.torque.BaseZoneRecordMapper.processRow(BaseZoneRecordMapper.java:1)
>  ~[classes/:?]
>                  at 
> org.apache.torque.util.ResultsetSpliterator.tryAdvance(ResultsetSpliterator.java:120)
>  ~[torque-runtime-6.0.jar:6.0]
>                  at 
> java.base/java.util.Spliterator.forEachRemaining(Spliterator.java:332) ~[?:?]
>                  at 
> java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
>  ~[?:?]
>                  at 
> java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
>  ~[?:?]
>                  at 
> java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921)
>  ~[?:?]
>                  at 
> java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
>  ~[?:?]
>                  at 
> java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:682)
>  ~[?:?]
>                  at 
> org.apache.torque.util.BasePeerImpl.doSelect(BasePeerImpl.java:1153) 
> ~[torque-runtime-6.0.jar:6.0]
>                  at 
> org.apache.torque.util.BasePeerImpl.doSelect(BasePeerImpl.java:836) 
> ~[torque-runtime-6.0.jar:6.0]
> Drilling down the stack, we were able to identify the root cause at the 
> (singleton) initialisation of the Table 
> specific 
> Base<table name>RecordMapper
> The template definition of recordMapperBase.vm om module torque-templates 
> produces concurrency sensitive code, 
> especially the lazy initialization of the strategy member at line 172ff. 
> (introduced with TORQUE-364: init a new Strategy implementation) 
>    
> 111    ## TORQUE-364: Should this be cached per RecordMapper (Thread 
> safety/Multi query safety?)
> 112    private MappingStrategy<${dbObjectClassName}> strategy;
> ...
> 172                if (useMappingStrategy) {
> 173                    initStrategy();
> 174                }
> ...
> 194                        if (useMappingStrategy) 
> 195                        {
> 196                            strategy.addColumn(nextOffset, 
> 197                                (res, inst) -> inst.${setter}( 
> ${getter}(res, nextOffset)));
> 198                        } else
> {noformat}
> Harald proposes a solution and attached a fix fro recordeMapperBase.vm:
> {noformat}
> I like to propose a solution for this problem:
> 1)    The comment at line 111 was right 😊
> 2)    initStrategy() should return the created MappingStrategy.
> 3)    The MappingStrategy value must be used within the processRow( .. ) 
> method
> 4)    A double init of the strategy member does not do any harm, as long the 
> created MappingStrategy instance is 
> used within the processRow( .. ) method
> 5)    Synchronisation or volatile on this.strategy is not needed then
> 6)    The boolean member useMappingStrategy can be omitted 
> I’ve attached our the modified and tested recordMapperBase.vm template file.
> ...
> {noformat}
> I start to test it soon and then put it into the repo trunk with comment like 
> TORQUE-ID contributed by .. 
> Any comments welcome :)



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscr...@db.apache.org
For additional commands, e-mail: torque-dev-h...@db.apache.org

Reply via email to