OpenJPA
  1. OpenJPA
  2. OPENJPA-1376

@SequenceGenerator allocationSize incorrect implementation

    Details

      Description

      OpenJpa use allocationSize parameter on NativeJDBCSeq to set the sequence cache value.

      But one "JSR 220: Enterprise JavaBeansTM,Version 3.0" , the allocationSize is : "The amount to increment by when allocating sequence
      numbers from the sequence."

      Allocating is used to limit the number of access to the sequence or table not to set the sequence cache value !

      If allocationSize=50, the sequence need to be call one time on each 50 call on AbstractJDBCSeq.next().

      1 call on AbstractJDBCSeq.next(). call the sequence or table
      2 call : return previous value + 1;
      3 call : return previous value + 2;
      ...
      51 call :re-call the sequence
      52 return previous value + 1
      ...

        Issue Links

          Activity

          Yves created issue -
          Yves made changes -
          Field Original Value New Value
          Original Estimate 8m [ 480 ] 8h [ 28800 ]
          Remaining Estimate 8m [ 480 ] 8h [ 28800 ]
          Hide
          Palmer Cox added a comment -

          The attached patch modifies the NativeJDBCSeq class to make use of the increment parameter. The modified class will only get the next value of the database sequence object once every increment number of invocations.

          The patch also modifies the meta-data parsers to use the allocationSize parameter to the SequenceGenerator's to set the increment size.

          Show
          Palmer Cox added a comment - The attached patch modifies the NativeJDBCSeq class to make use of the increment parameter. The modified class will only get the next value of the database sequence object once every increment number of invocations. The patch also modifies the meta-data parsers to use the allocationSize parameter to the SequenceGenerator's to set the increment size.
          Palmer Cox made changes -
          Attachment cache-native-seq-values.diff [ 12470539 ]
          Milosz Tylenda made changes -
          Assignee Milosz Tylenda [ milosz ]
          Milosz Tylenda made changes -
          Component/s competitive [ 12313757 ]
          Milosz Tylenda made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          Milosz Tylenda made changes -
          Summary @GeneratedValue allocationSize incorrect implementation @SequenceGenerator allocationSize incorrect implementation
          Fix Version/s 2.2.0 [ 12315910 ]
          Priority Critical [ 2 ] Major [ 3 ]
          Component/s performance [ 12312974 ]
          Hide
          Milosz Tylenda added a comment -

          Palmer, thanks for the patch. I have modified it, e.g. allocate property is used instead of increment to be more consistent with table-based sequence, but the idea is the same. Committed.

          I also got rid of the auto-configuration support which was marked deprecated from the beginning of OpenJPA (2006) which was interferring with this change.

          Since this change poses some burden during migration, I will update the manual soon.

          Show
          Milosz Tylenda added a comment - Palmer, thanks for the patch. I have modified it, e.g. allocate property is used instead of increment to be more consistent with table-based sequence, but the idea is the same. Committed. I also got rid of the auto-configuration support which was marked deprecated from the beginning of OpenJPA (2006) which was interferring with this change. Since this change poses some burden during migration, I will update the manual soon.
          Milosz Tylenda made changes -
          Affects Version/s 2.1.0 [ 12314542 ]
          Milosz Tylenda made changes -
          Status In Progress [ 3 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Hide
          Milosz Tylenda added a comment -

          Added other Fix Versions since code has been checked in.

          On branches earlier than 2.2, the behaviour is gated by the useNativeSequenceCache property of DBDictionary. Set it to true to enable this new functionality.

          Show
          Milosz Tylenda added a comment - Added other Fix Versions since code has been checked in. On branches earlier than 2.2, the behaviour is gated by the useNativeSequenceCache property of DBDictionary. Set it to true to enable this new functionality.
          Milosz Tylenda made changes -
          Fix Version/s 1.2.3 [ 12314517 ]
          Fix Version/s 1.3.0 [ 12313326 ]
          Fix Version/s 2.0.2 [ 12315257 ]
          Fix Version/s 2.1.2 [ 12317142 ]
          Hide
          Milosz Tylenda added a comment -

          Should be: Set it to FALSE to enable this new functionality.

          Show
          Milosz Tylenda added a comment - Should be: Set it to FALSE to enable this new functionality.
          Hide
          Milosz Tylenda added a comment -

          Heath, I think the following code, found in a branch, needs correction. Won't we get too many "INCREMENT BY"s ?

          if (seq.getIncrement() > 1 && useNativeSequenceCache)

          { buf.append(" INCREMENT BY ").append(seq.getIncrement()); }

          else if ((seq.getIncrement() > 1) || (seq.getAllocate() > 1))

          { buf.append(" INCREMENT BY ").append(seq.getIncrement() * seq.getAllocate()); }

          if (seq.getIncrement() > 1)

          buf.append(" INCREMENT BY ").append(seq.getIncrement());

          Show
          Milosz Tylenda added a comment - Heath, I think the following code, found in a branch, needs correction. Won't we get too many "INCREMENT BY"s ? if (seq.getIncrement() > 1 && useNativeSequenceCache) { buf.append(" INCREMENT BY ").append(seq.getIncrement()); } else if ((seq.getIncrement() > 1) || (seq.getAllocate() > 1)) { buf.append(" INCREMENT BY ").append(seq.getIncrement() * seq.getAllocate()); } if (seq.getIncrement() > 1) buf.append(" INCREMENT BY ").append(seq.getIncrement());
          Hide
          Heath Thomann added a comment -

          Hello Milosz!! Excellent catch!! Thank you! If you would please, could you double check my work here to make sure we are on the same page? That is, in 1.2.x and 1.3.x, it looks like my changes to DBDictionary are correct. However, looking at the DBDictionary changes in 2.0.x and 2.1.x, I forgot to remove this 'if' block:

          if (seq.getIncrement() > 1)

          buf.append(" INCREMENT BY ").append(seq.getIncrement());

          Sound correct or am I still missing something?

          Show
          Heath Thomann added a comment - Hello Milosz!! Excellent catch!! Thank you! If you would please, could you double check my work here to make sure we are on the same page? That is, in 1.2.x and 1.3.x, it looks like my changes to DBDictionary are correct. However, looking at the DBDictionary changes in 2.0.x and 2.1.x, I forgot to remove this 'if' block: if (seq.getIncrement() > 1) buf.append(" INCREMENT BY ").append(seq.getIncrement()); Sound correct or am I still missing something?
          Hide
          Milosz Tylenda added a comment -

          Hi Heath! It sounds correct now. I will have a look at the other files and will let you know if I find anything.

          Show
          Milosz Tylenda added a comment - Hi Heath! It sounds correct now. I will have a look at the other files and will let you know if I find anything.
          Albert Lee made changes -
          Link This issue relates to OPENJPA-2069 [ OPENJPA-2069 ]
          Hide
          Albert Lee added a comment -

          Close issue in preparation for 2.2.0 release.

          Show
          Albert Lee added a comment - Close issue in preparation for 2.2.0 release.
          Albert Lee made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Kevin Sutter made changes -
          Link This issue relates to OPENJPA-2196 [ OPENJPA-2196 ]
          Heath Thomann made changes -
          Link This issue is related to OPENJPA-2450 [ OPENJPA-2450 ]
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open In Progress In Progress
          471d 20h 51m 1 Milosz Tylenda 19/Feb/11 20:40
          In Progress In Progress Resolved Resolved
          21d 13h 16m 1 Milosz Tylenda 13/Mar/11 09:57
          Resolved Resolved Closed Closed
          326d 6h 31m 1 Albert Lee 02/Feb/12 16:28

            People

            • Assignee:
              Milosz Tylenda
              Reporter:
              Yves
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 8h
                8h
                Remaining:
                Remaining Estimate - 8h
                8h
                Logged:
                Time Spent - Not Specified
                Not Specified

                  Development