Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.0-M2
    • Fix Version/s: 2.0.0-M2
    • Component/s: query
    • Labels:
      None

      Description

      Support default query hint for query timeout as defined in section 3.6.4 of the spec.
      A new hint can be supplied for Java SE and Java EE environments -
      javax.persistence.query.timeout // query timeout in seconds
      Can be used in the following:
      Methods - Query.setHint()
      Annotations (via QueryHint) - NamedQuery, NativeNamedQuery
      Properties - Persistence.createEntityManagerFactory, persistence.xml
      The following methods can return a javax.persistence.QueryTimeoutException: getResultList(), getSingleResult(), executeUpdate().
      If a QTE is thrown, the current transaction (if active) should not be marked for rollback.
      If the DB query timeout causes a rollback, then a PersistenceException should be thrown instead (see 3.6.1).

      1. OPENJPA-878-20090305-draft.patch
        71 kB
        Donald Woods
      2. OPENJPA-878-20090306.patch
        71 kB
        Donald Woods
      3. OPENJPA-878-20090310-eclipse.patch
        78 kB
        Donald Woods
      4. OPENJPA-878-20090310.patch
        165 kB
        Donald Woods
      5. OPENJPA-878-20090311-eclipse.patch
        163 kB
        Donald Woods
      6. OPENJPA-878-20090311.patch
        165 kB
        Donald Woods
      7. OPENJPA-878-docs-20090311.patch
        1 kB
        Donald Woods
      8. OPENJPA-878-20090311.patch
        78 kB
        Donald Woods
      9. OPENJPA-878-20090312-minimal.patch
        57 kB
        Donald Woods
      10. OPENJPA-878-20090313.patch
        79 kB
        Donald Woods
      11. OPENJPA-878-DBDictionary.patch
        2 kB
        Donald Woods
      12. OPENJPA-878-TestQueryTimeout.patch
        6 kB
        Donald Woods
      13. OPENJPA-878-serialVersionUID.patch
        4 kB
        Donald Woods
      14. OpenJPA-878-db2.zip
        4 kB
        Donald Woods
      15. OpenJPA-878-db2.tgz
        5 kB
        Donald Woods

        Issue Links

          Activity

          Hide
          Donald Woods added a comment -

          DB2 for Linux setup scripts for testing.

          Show
          Donald Woods added a comment - DB2 for Linux setup scripts for testing.
          Hide
          Donald Woods added a comment -

          Remaining suggestion to fix serialVersionUID usage has been opened as OPENJPA-1000.

          Show
          Donald Woods added a comment - Remaining suggestion to fix serialVersionUID usage has been opened as OPENJPA-1000 .
          Hide
          Donald Woods added a comment -

          Code to setup DB2 for setQueryTimeout testing with TestQueryTimeout.java junit code.
          First, create a DB called OPENJPA and make sure you have uid=db2admin setup.
          Then, run DelaySetup.bat (which uses uid=db2admin) to create the jar with the delay() function, register the jar in DB2 and create a Delay function on the OPENJPA db.
          I've been testing this with DB2 v9.5 on Windows, but it should work for 9.1 and v8.

          Show
          Donald Woods added a comment - Code to setup DB2 for setQueryTimeout testing with TestQueryTimeout.java junit code. First, create a DB called OPENJPA and make sure you have uid=db2admin setup. Then, run DelaySetup.bat (which uses uid=db2admin) to create the jar with the delay() function, register the jar in DB2 and create a Delay function on the OPENJPA db. I've been testing this with DB2 v9.5 on Windows, but it should work for 9.1 and v8.
          Hide
          Jeremy Bauer added a comment -

          Committed OPENJPA-878-TestQueryTimeout.patch for Donald under revision 755818.
          Committed OPENJPA-878-DBDictionary.patch for Donald under revision 755819.

          The issue regarding the use of serialVersionUID will be resolved under another JIRA.

          Show
          Jeremy Bauer added a comment - Committed OPENJPA-878 -TestQueryTimeout.patch for Donald under revision 755818. Committed OPENJPA-878 -DBDictionary.patch for Donald under revision 755819. The issue regarding the use of serialVersionUID will be resolved under another JIRA.
          Hide
          Donald Woods added a comment -

          The following requested updates are ready for your review -
          OPENJPA-878-DBDictionary.patch - changes requested by Milosz
          OPENJPA-878-TestQueryTimeout.patch - changes requested by Jeremy
          OPENJPA-878-serialVersionUID.patch - changes requested by Pinaki

          Show
          Donald Woods added a comment - The following requested updates are ready for your review - OPENJPA-878 -DBDictionary.patch - changes requested by Milosz OPENJPA-878 -TestQueryTimeout.patch - changes requested by Jeremy OPENJPA-878 -serialVersionUID.patch - changes requested by Pinaki
          Hide
          Donald Woods added a comment -

          Patch (OPENJPA-878-serialVersionUID.patch) that removes serialVersionUID additions, as requested by Pinaki.

          Show
          Donald Woods added a comment - Patch ( OPENJPA-878 -serialVersionUID.patch) that removes serialVersionUID additions, as requested by Pinaki.
          Hide
          Donald Woods added a comment -

          Attached patch (OPENJPA-878-DBDictionary.patch) to address Milosz's request to remove the "this." usage from DBDictionary.
          Attached patch (OPENJPA-878-TestQueryTimeout.patch) to address Jeremy's intermittent test failures on Windows.

          Show
          Donald Woods added a comment - Attached patch ( OPENJPA-878 -DBDictionary.patch) to address Milosz's request to remove the "this." usage from DBDictionary. Attached patch ( OPENJPA-878 -TestQueryTimeout.patch) to address Jeremy's intermittent test failures on Windows.
          Hide
          Jeremy Bauer added a comment -

          Committed OPENJPA-878-20090313.patch (with updates) under revisions: 755113, 755114, 755115, 755116, 755117 for Donald.

          Comments:

          • Cleaned up unused localizers per suggestion from Milosz
          • Did not remove usage of 'this' in new dictionary methods per suggestion from Milosz. I'll catch that on the next update.
          • Tagged two methods in TestQueryTimeout with AllowFailure(true) because they fail intermittently on my local build (but ran fine on a separate build server). I've sent the surefire report to Donald for investigation. Failures look to be test procedure related.
          Show
          Jeremy Bauer added a comment - Committed OPENJPA-878 -20090313.patch (with updates) under revisions: 755113, 755114, 755115, 755116, 755117 for Donald. Comments: Cleaned up unused localizers per suggestion from Milosz Did not remove usage of 'this' in new dictionary methods per suggestion from Milosz. I'll catch that on the next update. Tagged two methods in TestQueryTimeout with AllowFailure(true) because they fail intermittently on my local build (but ran fine on a separate build server). I've sent the surefire report to Donald for investigation. Failures look to be test procedure related.
          Hide
          Milosz Tylenda added a comment -

          Jeremy, as for performing formatting changes I noticed two little things in the patch:

          • in DBDictionary, from what I have seen, we usually don't use "this" in front of variable unless necessary,
          • probably there are some unused Localizers.
          Show
          Milosz Tylenda added a comment - Jeremy, as for performing formatting changes I noticed two little things in the patch: in DBDictionary, from what I have seen, we usually don't use "this" in front of variable unless necessary, probably there are some unused Localizers.
          Hide
          Jeremy Bauer added a comment -

          Comments on OPENJPA-878-20090313.patch:

          Pinaki's suggestion was very good, IMHO. This version looks much cleaner and allows for database specific behavior. I was going to suggest eliminating the near duplicate set methods in DBDictionary with a single method that takes both timeout values as int params - but passing in the fetch config or jdbc config will allow simpler setting of future timeout values (if any should get added), without needing to modify all the setTimeouts method calls in the code. Unless there is an objection, I'll commit this patch (with formatting updates) on 3/16. That should provide ample time for others to review.

          Show
          Jeremy Bauer added a comment - Comments on OPENJPA-878 -20090313.patch: Pinaki's suggestion was very good, IMHO. This version looks much cleaner and allows for database specific behavior. I was going to suggest eliminating the near duplicate set methods in DBDictionary with a single method that takes both timeout values as int params - but passing in the fetch config or jdbc config will allow simpler setting of future timeout values (if any should get added), without needing to modify all the setTimeouts method calls in the code. Unless there is an objection, I'll commit this patch (with formatting updates) on 3/16. That should provide ample time for others to review.
          Hide
          Donald Woods added a comment - - edited

          Updated patch (OPENJPA-878-20090313.patch) that addresses Pinaki's request to centralize the set query/lock timeout code into the DBDictionary.

          Show
          Donald Woods added a comment - - edited Updated patch ( OPENJPA-878 -20090313.patch) that addresses Pinaki's request to centralize the set query/lock timeout code into the DBDictionary.
          Hide
          Pinaki Poddar added a comment -

          The very fact that following code (or similar variants) appears 10+ times in the patch points that a better solution/refactoring is required.
          It is not about that the patch is getting it wrong (in fact, the patch shows thoroughness) but the way existing things are.

          Given that
          1) the research finds that time outs may better be handled database server side
          2) JDBC does not support a lockTimeOut and we are emulating it by query time out
          3) databases employ specific techniques for server side time outs
          4) the current timeout requires us to duplicate same code block +10 times

          I suggest
          a) move the duplicated code to DBDictionary – this default implement will do what we do today i.e. emulate locktimeout via querytimeout.
          this will remove/minimize duplication of code and will localize the timeout function at one place
          b) incrementally add specific database time out functions for server side time out

          + protected void setTimeout(PreparedStatement stmnt, JDBCStore store,
          + JDBCFetchConfiguration fetch)
          + throws SQLException {
          + if (store.getDBDictionary().supportsQueryTimeout) {
          + int timeout = fetch.getQueryTimeout();
          + if (timeout >= 0) {
          + if (timeout > 0 && timeout < 1000)

          { + timeout = 1000; + Log log = store.getConfiguration().getLog(JDBCConfiguration.LOG_JDBC); + if (log.isWarnEnabled()) + log.warn(_loc.get("millis-query-timeout")); + }

          + stmnt.setQueryTimeout(timeout / 1000);
          + }
          + }
          + }
          +

          Show
          Pinaki Poddar added a comment - The very fact that following code (or similar variants) appears 10+ times in the patch points that a better solution/refactoring is required. It is not about that the patch is getting it wrong (in fact, the patch shows thoroughness) but the way existing things are. Given that 1) the research finds that time outs may better be handled database server side 2) JDBC does not support a lockTimeOut and we are emulating it by query time out 3) databases employ specific techniques for server side time outs 4) the current timeout requires us to duplicate same code block +10 times I suggest a) move the duplicated code to DBDictionary – this default implement will do what we do today i.e. emulate locktimeout via querytimeout. this will remove/minimize duplication of code and will localize the timeout function at one place b) incrementally add specific database time out functions for server side time out + protected void setTimeout(PreparedStatement stmnt, JDBCStore store, + JDBCFetchConfiguration fetch) + throws SQLException { + if (store.getDBDictionary().supportsQueryTimeout) { + int timeout = fetch.getQueryTimeout(); + if (timeout >= 0) { + if (timeout > 0 && timeout < 1000) { + timeout = 1000; + Log log = store.getConfiguration().getLog(JDBCConfiguration.LOG_JDBC); + if (log.isWarnEnabled()) + log.warn(_loc.get("millis-query-timeout")); + } + stmnt.setQueryTimeout(timeout / 1000); + } + } + } +
          Hide
          Donald Woods added a comment -

          Updated patch (OPENJPA-878-20090312-minimal.patch) that addresses Jeremy's #1 and #4 concerns by removing setTimeout() on seq/schema/counts/DBDictionary/... and only implements it on SelectImpl and SQLStoreQuery, so the TestQueryTimeout tests will still pass for the Spec required EM.find()/executeUpdate(), getResultList() and getSingleResult() operations.
          Includes base QueryTimeoutException support with required updates to sql-error-codes.xml, StoreException, QueryException and DBDictionary/DerbyDictionary.
          Includes doc updates to include new Query/Lock timeout hint definitions.
          Still uses max() if a LockTimeout is specified, until OPENJPA-957 is resolved.

          Show
          Donald Woods added a comment - Updated patch ( OPENJPA-878 -20090312-minimal.patch) that addresses Jeremy's #1 and #4 concerns by removing setTimeout() on seq/schema/counts/DBDictionary/... and only implements it on SelectImpl and SQLStoreQuery, so the TestQueryTimeout tests will still pass for the Spec required EM.find()/executeUpdate(), getResultList() and getSingleResult() operations. Includes base QueryTimeoutException support with required updates to sql-error-codes.xml, StoreException, QueryException and DBDictionary/DerbyDictionary. Includes doc updates to include new Query/Lock timeout hint definitions. Still uses max() if a LockTimeout is specified, until OPENJPA-957 is resolved.
          Hide
          Donald Woods added a comment -

          I'll take a look at #1/#2 to see what can be done, but there were several abstract interfaces being used that didn't extend a common parent (and I was afraid of breaking APIs again.)
          For #3, I'll add a comment in OPENJPA-957, to update the docs if we don't end up switching to use the db server lock timeout support.
          For #4, my thoughts were that if someone provides a query.timeout value/hint, then we should try to honor it everywhere, as the usage of setQueryTimeout() is to prevent JDBC client apps from waiting forever on blocked or missed server responses due to network or db problems.
          For the FIXME, I noticed in Eclipse there were tons of existing FIXME/TODO comment tags, so I also used it to denote pending work items....

          Thanks for the review.

          Show
          Donald Woods added a comment - I'll take a look at #1/#2 to see what can be done, but there were several abstract interfaces being used that didn't extend a common parent (and I was afraid of breaking APIs again.) For #3, I'll add a comment in OPENJPA-957 , to update the docs if we don't end up switching to use the db server lock timeout support. For #4, my thoughts were that if someone provides a query.timeout value/hint, then we should try to honor it everywhere, as the usage of setQueryTimeout() is to prevent JDBC client apps from waiting forever on blocked or missed server responses due to network or db problems. For the FIXME, I noticed in Eclipse there were tons of existing FIXME/TODO comment tags, so I also used it to denote pending work items.... Thanks for the review.
          Hide
          Jeremy Bauer added a comment -

          Couple of comments on OPENJPA-878-20090311.patch (7:12pm).

          1) There is quite a bit of duplication of the setTimeout method (likely caused by refactoring to eliminate the need for interface changes per previous comments) across multiple subclasses. Is it possible to implement the method in the abstract parent class (for store query, seq, and strats) to eliminate much of the duplication?

          or

          2) Similar to connection, could a decorator pattern be applied to prepared statement to decorate the ps with query and lock timeout hints vs. calling it explicitly after a statement is constructed? This may be difficult depending on the availability of the configuration. Something to think about though.

          3) The code uses 'max' to calculate the timeout value when query & lock timeout are used in combination. Given the way lock timeouts are currently implemented (with query timeout), that seems like the safest behavior. (Other opinions?) Until lock timeouts are handled by the DB, I think this behavior should be documented. A sentence in the lock and query timeout docs would be sufficient.

          4) Should query timeout be applied to sequence queries? I'm on the fence. Any thoughts on this?

          Is anyone opposed to including the FIXME comments when these changes are committed? They reference another JIRA so they aren't simply 'dangling' out there. Is there a convention for commenting on future work within code? Should those comments only exist within a JIRA?

          Show
          Jeremy Bauer added a comment - Couple of comments on OPENJPA-878 -20090311.patch (7:12pm). 1) There is quite a bit of duplication of the setTimeout method (likely caused by refactoring to eliminate the need for interface changes per previous comments) across multiple subclasses. Is it possible to implement the method in the abstract parent class (for store query, seq, and strats) to eliminate much of the duplication? or 2) Similar to connection, could a decorator pattern be applied to prepared statement to decorate the ps with query and lock timeout hints vs. calling it explicitly after a statement is constructed? This may be difficult depending on the availability of the configuration. Something to think about though. 3) The code uses 'max' to calculate the timeout value when query & lock timeout are used in combination. Given the way lock timeouts are currently implemented (with query timeout), that seems like the safest behavior. (Other opinions?) Until lock timeouts are handled by the DB, I think this behavior should be documented. A sentence in the lock and query timeout docs would be sufficient. 4) Should query timeout be applied to sequence queries? I'm on the fence. Any thoughts on this? Is anyone opposed to including the FIXME comments when these changes are committed? They reference another JIRA so they aren't simply 'dangling' out there. Is there a convention for commenting on future work within code? Should those comments only exist within a JIRA?
          Hide
          Donald Woods added a comment -

          Updated the OPENJPA-878-20090311.patch by using "svn diff -x --ignore-eol-style". Pinaki, thanks for the pointer.

          Show
          Donald Woods added a comment - Updated the OPENJPA-878 -20090311.patch by using "svn diff -x --ignore-eol-style". Pinaki, thanks for the pointer.
          Hide
          Pinaki Poddar added a comment -

          The key changes are not very visible in OPENJPA-878-20090311.patch because often they are masked by large diffs created by EOL/white space character mismatch.
          So
          1. Create a patch that does have the real changes (e.g. something like svn diff -x --ignore-all-space)
          2. Submit separate patch for source and test branches. Actually, the tests can be directly checked-in.

          Show
          Pinaki Poddar added a comment - The key changes are not very visible in OPENJPA-878 -20090311.patch because often they are masked by large diffs created by EOL/white space character mismatch. So 1. Create a patch that does have the real changes (e.g. something like svn diff -x --ignore-all-space) 2. Submit separate patch for source and test branches. Actually, the tests can be directly checked-in.
          Hide
          Donald Woods added a comment -

          Patch to add definitions for javax.persistence.lock.timeout and javax.persistence.query.timeout to the 10.1.7 Query Hints section.

          Show
          Donald Woods added a comment - Patch to add definitions for javax.persistence.lock.timeout and javax.persistence.query.timeout to the 10.1.7 Query Hints section.
          Hide
          Donald Woods added a comment -

          Updated Eclipse patch that can be applied to existing trunk subprojects.

          Show
          Donald Woods added a comment - Updated Eclipse patch that can be applied to existing trunk subprojects.
          Hide
          Donald Woods added a comment -

          Updated patch against Rev752306 which fixes the existing org.apache.openjpa.conf.TestQueryHints junit test, which was using getQueryTimeout() for lockTimeout comparison.

          Show
          Donald Woods added a comment - Updated patch against Rev752306 which fixes the existing org.apache.openjpa.conf.TestQueryHints junit test, which was using getQueryTimeout() for lockTimeout comparison.
          Hide
          Donald Woods added a comment -

          Updated patches that do not change the method signatures.
          I've attached both a svn diff and Subclipse version of the patch, as the svn diff version includes replacing Windows EOL on 2 files.

          Show
          Donald Woods added a comment - Updated patches that do not change the method signatures. I've attached both a svn diff and Subclipse version of the patch, as the svn diff version includes replacing Windows EOL on 2 files.
          Hide
          Donald Woods added a comment -

          Updated patch that only runs the junit tests only against Derby for now.
          Additional JIRAs 963 and 964 opened against I5 to complete test coverage items.
          EntityManagerImpl.java changes backed out.
          JPA 2.0 spec comments removed from FetchConfiguration.java and OpenJPAConfiguration.java.
          Usage of QueryTimeoutException.isFatal() not addressed, as the code matches what was done for lock timeouts.
          Method signature changes not addressed, as these were required to enable access to the queryTimeout value.
          Only reformatted TestQueryTimeout.java to 80 columns (using the 'where required' option in Eclipse.)

          Show
          Donald Woods added a comment - Updated patch that only runs the junit tests only against Derby for now. Additional JIRAs 963 and 964 opened against I5 to complete test coverage items. EntityManagerImpl.java changes backed out. JPA 2.0 spec comments removed from FetchConfiguration.java and OpenJPAConfiguration.java. Usage of QueryTimeoutException.isFatal() not addressed, as the code matches what was done for lock timeouts. Method signature changes not addressed, as these were required to enable access to the queryTimeout value. Only reformatted TestQueryTimeout.java to 80 columns (using the 'where required' option in Eclipse.)
          Hide
          Donald Woods added a comment -

          Thanks for the review.
          Yep, figured someone would catch the formatting, as I hate the 20+ year old standard of 80 column widths, given most everyone has hi-res wide screen displays now... I can fix this in my Eclipse settings for the next patch.

          For the other DBs, I'd like to open a subtask for I5. I would also like to include additional tests for setting the property via Persistence.createEntityManagerFactory and persistence.xml in I5.

          The method signature changes were the only way to get the queryTimeout value down into the prepareStatement() and prepareCall() methods, as they currently only have access to the Connection, which doesn't have handles to the FetchConfiguration or OpenJPAConfiguration classes.

          Adding queryTimeout support to TableJDBCSeq.java would involve either more method signature changes and/or creating a FetchConfiguration just to hold the QueryTimeout value if it is > 0, as neither the TableJDBCSeq nor TableSchemaFactory classes have a FetchConfiguration passed to them today (but they do have access to the OpenJPAConfiguration data.)

          Show
          Donald Woods added a comment - Thanks for the review. Yep, figured someone would catch the formatting, as I hate the 20+ year old standard of 80 column widths, given most everyone has hi-res wide screen displays now... I can fix this in my Eclipse settings for the next patch. For the other DBs, I'd like to open a subtask for I5. I would also like to include additional tests for setting the property via Persistence.createEntityManagerFactory and persistence.xml in I5. The method signature changes were the only way to get the queryTimeout value down into the prepareStatement() and prepareCall() methods, as they currently only have access to the Connection, which doesn't have handles to the FetchConfiguration or OpenJPAConfiguration classes. Adding queryTimeout support to TableJDBCSeq.java would involve either more method signature changes and/or creating a FetchConfiguration just to hold the QueryTimeout value if it is > 0, as neither the TableJDBCSeq nor TableSchemaFactory classes have a FetchConfiguration passed to them today (but they do have access to the OpenJPAConfiguration data.)
          Hide
          Jeremy Bauer added a comment -

          Comments on OPENJPA-878-20090305-draft.patch:

          • General: OpenJPA uses an 80 column, 4 space indent formatting scheme some lines were considerably longer than 80 chars.
          • General: Looks like there is a lot of work to identify the query timeout condition for all the supported DBs... We could consider making that a separate work item.
          • FetchConfiguration.java and OpenJPAConfiguration.java are in the kernel module. The kernel can be & is used by multiple ORM providers/solutions so kernel javadoc should not be specific to JPA. I recommend removing the JPA 2.0 spec ... line.
          • SQLBuffer.java - I could be mistaken, but I don't think removing a public method or changing a method signature is a good idea. Fay is more knowledgeable on the external uses of this class so it would be be good run these changes by her.
          • It looked as if OpenJPA was only passing the fetch config when dealing with LRS. Are there any additional side effects from passing the fetch config? Does any behavior get triggered that should be LRS only by passing the fetch config on the prepare?
          • JDBCStoreQuery.java - ditto on the method signature changes
          • TableJDBCSeq.java - I'd like to see some feedback from other folks regarding the use of query timeout during this sensitive operation. The sequence operation is part of the full operation so I think it makes sense to try to honor the timeout, if possible. Although a separate work item, I think lock timeout should also be considered.

          SQLStoreQuery.java - ditto on the method signature changes

          TableSchemaFactory.java - IMHO, I don't think we should honor timeouts in the schema factory. That's startup behavior vs. runtime behavior and I think this hint would be more expected to apply only to runtime behavior. It would be good to have other opinions on this as well.

          EntityManagerImpl.java - The changes to the clear method were part of Dianne's patch, but shouldn't have been in the patch.

          TestQueryTimeout .java - I noticed that the ability to retry an operation after a query timeout is database specific. Could/should this fact get carried into QueryTimeoutException.isFatal()?

          Very thorough job on the tests!

          Show
          Jeremy Bauer added a comment - Comments on OPENJPA-878 -20090305-draft.patch: General: OpenJPA uses an 80 column, 4 space indent formatting scheme some lines were considerably longer than 80 chars. General: Looks like there is a lot of work to identify the query timeout condition for all the supported DBs... We could consider making that a separate work item. FetchConfiguration.java and OpenJPAConfiguration.java are in the kernel module. The kernel can be & is used by multiple ORM providers/solutions so kernel javadoc should not be specific to JPA. I recommend removing the JPA 2.0 spec ... line. SQLBuffer.java - I could be mistaken, but I don't think removing a public method or changing a method signature is a good idea. Fay is more knowledgeable on the external uses of this class so it would be be good run these changes by her. It looked as if OpenJPA was only passing the fetch config when dealing with LRS. Are there any additional side effects from passing the fetch config? Does any behavior get triggered that should be LRS only by passing the fetch config on the prepare? JDBCStoreQuery.java - ditto on the method signature changes TableJDBCSeq.java - I'd like to see some feedback from other folks regarding the use of query timeout during this sensitive operation. The sequence operation is part of the full operation so I think it makes sense to try to honor the timeout, if possible. Although a separate work item, I think lock timeout should also be considered. SQLStoreQuery.java - ditto on the method signature changes TableSchemaFactory.java - IMHO, I don't think we should honor timeouts in the schema factory. That's startup behavior vs. runtime behavior and I think this hint would be more expected to apply only to runtime behavior. It would be good to have other opinions on this as well. EntityManagerImpl.java - The changes to the clear method were part of Dianne's patch, but shouldn't have been in the patch. TestQueryTimeout .java - I noticed that the ability to retry an operation after a query timeout is database specific. Could/should this fact get carried into QueryTimeoutException.isFatal()? Very thorough job on the tests!
          Hide
          Donald Woods added a comment -

          Draft patch that implements Query timeout support.
          So far, only tested on Derby. Also, still have a couple cleanup TODOs, but wanted to post it to get some early feedback.

          Show
          Donald Woods added a comment - Draft patch that implements Query timeout support. So far, only tested on Derby. Also, still have a couple cleanup TODOs, but wanted to post it to get some early feedback.

            People

            • Assignee:
              Donald Woods
              Reporter:
              Donald Woods
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development