[
https://issues.apache.org/jira/browse/OPENJPA-878?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12679624#action_12679624
]
Jeremy Bauer commented on OPENJPA-878:
--------------------------------------
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!
> Support default query hint for query timeout
> --------------------------------------------
>
> Key: OPENJPA-878
> URL: https://issues.apache.org/jira/browse/OPENJPA-878
> Project: OpenJPA
> Issue Type: Sub-task
> Components: query
> Affects Versions: 2.0.0
> Reporter: Donald Woods
> Assignee: Donald Woods
> Fix For: 2.0.0
>
> Attachments: OPENJPA-878-20090305-draft.patch
>
>
> 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).
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
