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

Brett Bergquist commented on DERBY-6975:
----------------------------------------

Looking at the code, it seems to me that the "break" in question is for the 
"switch" case, so it should continue with the next iteration of the "for" loop 
anyways and have the same affect as the "continue" change in the patch.   
Unless I am seeing something wrong?



> Bug in SequenceUpdater.getCurrentValueAndAdvance - break used instead of 
> continue
> ---------------------------------------------------------------------------------
>
>                 Key: DERBY-6975
>                 URL: https://issues.apache.org/jira/browse/DERBY-6975
>             Project: Derby
>          Issue Type: Bug
>          Components: SQL
>    Affects Versions: 10.14.1.0
>            Reporter: David Sitsky
>         Attachments: fix.patch
>
>
> I have an application that sporadically gets this exception when inserting a 
> lot of rows using a sequence generator.
> {noformat}
> Caused by: ERROR 40XL1: A lock could not be obtained within the time 
> requested 
>         at 
> org.apache.derby.iapi.error.StandardException.newException(StandardException.java)
>  
>         at 
> org.apache.derby.iapi.error.StandardException.newException(StandardException.java)
>  
>         at 
> org.apache.derby.impl.sql.catalog.SequenceUpdater.tooMuchContentionException(SequenceUpdater.java)
>  
>         at 
> org.apache.derby.impl.sql.catalog.SequenceUpdater.getCurrentValueAndAdvance(SequenceUpdater.java)
>  
>         at 
> org.apache.derby.impl.sql.catalog.DataDictionaryImpl.getCurrentValueAndAdvance(DataDictionaryImpl.java)
>  
>         at 
> org.apache.derby.impl.sql.execute.BaseActivation.getCurrentValueAndAdvance(BaseActivation.java)
>  
>         at 
> org.apache.derby.impl.sql.execute.InsertResultSet.getSetAutoincrementValue(InsertResultSet.java)
>  
>         at 
> org.apache.derby.impl.sql.execute.BaseActivation.getSetAutoincrementValue(BaseActivation.java)
>  
>         at 
> org.apache.derby.exe.ac560740aax015fx6bc1x68cax000002339e626a.e0(ac560740aax015fx6bc1x68cax000002339e626a.java)
>  
>         at 
> org.apache.derby.impl.services.reflect.DirectCall.invoke(DirectCall.java) 
>         at 
> org.apache.derby.impl.sql.execute.RowResultSet.getNextRowCore(RowResultSet.java)
>  
>         at 
> org.apache.derby.impl.sql.execute.NormalizeResultSet.getNextRowCore(NormalizeResultSet.java)
>  
>         at 
> org.apache.derby.impl.sql.execute.DMLWriteResultSet.getNextRowCore(DMLWriteResultSet.java)
>  
>         at 
> org.apache.derby.impl.sql.execute.InsertResultSet.getNextRowCore(InsertResultSet.java)
>  
>         at 
> org.apache.derby.impl.sql.execute.InsertResultSet.open(InsertResultSet.java) 
>         at 
> org.apache.derby.impl.sql.GenericPreparedStatement.executeStmt(GenericPreparedStatement.java)
>  
>         at 
> org.apache.derby.impl.sql.GenericPreparedStatement.execute(GenericPreparedStatement.java)
>  
>         ... 25 more 
> {noformat}
> When I look at the code in question, it comes down to this method:
> {code}
>     /**
>      * <p>
>      * Get the next sequence number managed by this generator and advance the 
> number. Could raise an
>      * exception if the legal range is exhausted and wrap-around is not 
> allowed.
>      * Only one thread at a time is allowed through here. We do not want a 
> race between the
>      * two calls to the sequence generator: getCurrentValueAndAdvance() and 
> allocateNewRange().
>      * </p>
>      *
>      * @param returnValue This value is stuffed with the new sequence number.
>      */
>     public synchronized void getCurrentValueAndAdvance
>         ( NumberDataValue returnValue ) throws StandardException
>     {
>         //
>         // We may have to try to get a value from the Sequence Generator 
> twice.
>         // The first attempt may fail because we need to pre-allocate a new 
> chunk
>         // of values.
>         //
>         for ( int i = 0; i < 2; i++ )
>         {
>             //
>             // We try to get a sequence number. The SequenceGenerator method 
> is synchronized
>             // so only one writer should be in there at a time. Lock 
> contention is possible if
>             // someone has selected from SYSSEQUENCES contrary to our advice. 
> In that case,
>             // we raise a TOO MUCH CONTENTION exception.
>             //
>             long[] cvaa = _sequenceGenerator.getCurrentValueAndAdvance();
>             
>             int status = (int) cvaa[ SequenceGenerator.CVAA_STATUS ];
>             long currentValue = cvaa[ SequenceGenerator.CVAA_CURRENT_VALUE ];
>             long lastAllocatedValue = cvaa[ 
> SequenceGenerator.CVAA_LAST_ALLOCATED_VALUE ];
>             long numberOfValuesAllocated = cvaa[ 
> SequenceGenerator.CVAA_NUMBER_OF_VALUES_ALLOCATED ];
>             
>             switch ( status )
>             {
>             case SequenceGenerator.RET_OK:
>                 returnValue.setValue( currentValue );
>                 return;
>                 
>             case SequenceGenerator.RET_MARK_EXHAUSTED:
>                 updateCurrentValueOnDisk( currentValue, null );
>                 returnValue.setValue( currentValue );
>                 return;
>                 
>             case SequenceGenerator.RET_ALLOCATE_NEW_VALUES:
>                 
>                 if ( updateCurrentValueOnDisk( currentValue, 
> lastAllocatedValue ) )
>                 {
>                     _sequenceGenerator.allocateNewRange( currentValue, 
> numberOfValuesAllocated );
>                 }
>                 break;
>             
>             default:
>                 throw unimplementedFeature();
>             }
>         }
>         //
>         // If we get here, then we failed to allocate a new sequence number 
> range.
>         //
>         throw tooMuchContentionException();
>     }
> {code}
> If I understand the intent of this code, in the 
> SequenceGenerator.RET_ALLOCATE_NEW_VALUES case, we allocate a new sequence 
> generator range and then we are meant to re-execute the loop again to use it, 
> however since "break" is used we break out of the for loop and throw the 
> exception.
> It seems if we replace "break" with 'continue" this will work as expected?



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to