[
https://issues.apache.org/jira/browse/OPENJPA-2517?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14561358#comment-14561358
]
Heath Thomann commented on OPENJPA-2517:
----------------------------------------
HI! I've been looking at this issue and running my findings by Rick Curtis.
I'll detail my findings, but basically OpenJPA already converts the query
timeout from milliseconds to seconds, so things are working as expected.....I
think the expectations in the test are incorrect.
I think we have a couple of issues going on:
1) We missed the fact that OpenJPA code does actually convert the timeout to
seconds (see DBDictionary.setStatementQueryTimeout) so it is not needed as is
proposed in the fix attached to this JIRA.
2) The originator (and those of us initially looking at this JIRA) missed the
fact that a 'persist' is used (rather than a query) in the provided test and as
such are expecting a query timeout to apply to EM operations.
Let me touch on each of these: First, if you look at TestQueryTimeout [1] you
can see that the query timeout is very extensively tested. So I think this
property is tested well and working well. Next, I found that OpenJPA does
convert the timeout from milliseconds to seconds, see here in
DBDictionary.setStatementQueryTimeout:
/**
* Allow subclasses to provide DB unique override implementations of
* setting query timeouts, while preserving the default timeout logic
* in the public setQueryTimeout method.
* @param stmnt
* @param timeout in milliseconds
* @throws SQLException
*/
protected void setStatementQueryTimeout(PreparedStatement stmnt,
int timeout) throws SQLException {
// JDBC uses seconds, so we'll do a simple round-down conversion here
stmnt.setQueryTimeout(timeout / 1000);
}
Given this, I decided to run the test sent into this JIRA. In doing so, it
finally hit me that the test is doing a 'persist' while some other thread has
locked the same row in the DB. So the user is expecting the query timeout to
apply to all EM operations. However, the query timeout should only apply to a
query (the spec implies this). Again, if you look at TestQueryTimeout, you'll
see a test method with comments:
/**
* Scenario being tested: 6) PU Query timeout hints do not affect EM
* operations like updating Entities returned by EM.find()/findAll()
* Expected Results: The DELAY function is being called and the update
* takes 2000+ msecs to complete.
*/
public void testQueryTimeout6() {
In other words, this method verifies that the 'find', which takes longer than
the query timeout, doesn't time out. So I believe the intentions are to not
apply the query timeout to EM find, etc.
I can understand the confusion here because the Spec is not 100% clear here. I
think a person needs to read the spec closely on this property. Let me point
out a few entries which imply the query timeout is limited to the Query
interface (and queries). First, the javax.persistence.query.timeout is defined
here:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
3.8.9 Query Hints
The following hint is defined by this specification for use in query
configuration.
javax.persistence.query.timeout // time in milliseconds
This hint may be used with the Query or TypedQuery setHint method or the
NamedQuery and
NamedNativeQuery annotations. It may also be passed as a property to the
Persistence.createEntityManagerFactory
method and used in the properties element of the persistence.
xml file.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
This appears to be referred to as the "query timeout" throughout the rest of
the spec. Next, If you look at the Query interface (section 3.8.1), it makes
reference to a "query timeout".....the other hits of "query timeout" throughout
the spec deal with the Query interface or query operations. The methods on the
EM interface talk about a "timeout", but never a "query timeout".
To me, this implies that the query.timeout only applies to the Query methods.
Finally, in working with the test of OJ2517, I added a test where a query is
used and verified correctness of the query timeout. The test works as I'd
expect for Derby and H2 (I choose H2 because that is the database in use in the
original test sent in). Let me attach my test, named
openjpa-querytimeout-working.zip in case someone wants to look at it or try it
out.
I propose this JIRA be closed if folks agree with my findings.
Thanks,
Heath Thomann
[1]
https://fisheye6.atlassian.com/browse/openjpa/branches/2.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/TestQueryTimeout.java?r=1244807
> Incorrect the time unit of query timeout value.
> -----------------------------------------------
>
> Key: OPENJPA-2517
> URL: https://issues.apache.org/jira/browse/OPENJPA-2517
> Project: OpenJPA
> Issue Type: Bug
> Components: jdbc
> Affects Versions: 2.2.0, 2.2.1, 2.2.2, 2.3.0
> Reporter: Masafumi Koba
> Assignee: Heath Thomann
> Attachments: OPENJPA-2517.patch, openjpa-querytimeout-bug.zip
>
>
> The value of the "javax.persistence.query.timeout" property have been passed
> to the java.sql.Statement.setQueryTimeout(int) in milliseconds rather than
> seconds.
> The query timeout milliseconds should be converted to seconds.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
