[jira] [Commented] (PHOENIX-2285) phoenix.query.timeoutMs doesn't allow callers to set the timeout to less than 1 second
[ https://issues.apache.org/jira/browse/PHOENIX-2285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14941724#comment-14941724 ] Jan Fernando commented on PHOENIX-2285: --- [~jamestaylor] I committed the patch to master, all 4.5 and and all 4.x branches. Is everything okay with the Jenkins builds? There don't seem to be test failures and the build seems to have been red before I committed. Let me know if this related to me commit and I should take action. > phoenix.query.timeoutMs doesn't allow callers to set the timeout to less than > 1 second > -- > > Key: PHOENIX-2285 > URL: https://issues.apache.org/jira/browse/PHOENIX-2285 > Project: Phoenix > Issue Type: Bug >Affects Versions: 4.5.2 >Reporter: Jan Fernando >Assignee: Jan Fernando > Attachments: PHOENIX-2285-v1.txt, PHOENIX-2285-v2.txt > > > When creating a Phoenix JDBC connection I have a use case where I want to > override the default value of phoenix.query.timeoutMs to a value of 200 ms. > Currently if you set phoenix.query.timeoutMs to less than 1000 ms, the > timeout gets rounded up to 1000ms. This is because in > PhoenixStatement.getDefaultQueryTimeout() we convert the value of > phoenix.query.timeoutMs to seconds in order to be compliant with JDBC. In > BaseResultIterators we then convert it back to millis. As a result of the > conversion we loose the millisecond fidelity. > A possible solution is to store the timeout value stored on the > PhoenixStatement in both seconds and milliseconds. Then, in > BaseResultIterators when we read the value from the statement we can check if > the value exists in millisecond fidelity and if so use that value. Otherwise > we would use the value in second granularity and convert. > This would allow Phoenix to remain JDBC compatible with second level > granularity for setting query timeouts on statements, but allow millisecond > granularity of timeouts by explicitly setting phoenix.query.timeoutMs on > connection properties. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (PHOENIX-2285) phoenix.query.timeoutMs doesn't allow callers to set the timeout to less than 1 second
[ https://issues.apache.org/jira/browse/PHOENIX-2285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14941672#comment-14941672 ] Hudson commented on PHOENIX-2285: - FAILURE: Integrated in Phoenix-master #907 (See [https://builds.apache.org/job/Phoenix-master/907/]) PHOENIX-2285 Changes to store the query timeout in milliseconds, to (jfernando: rev ff35d9536cc5657c63e42fdecad5828850f575fa) * phoenix-core/src/main/java/org/apache/phoenix/iterate/BaseResultIterators.java * phoenix-core/src/test/java/org/apache/phoenix/jdbc/PhoenixPreparedStatementTest.java * phoenix-core/src/it/java/org/apache/phoenix/iterate/PhoenixQueryTimeoutIT.java * phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java > phoenix.query.timeoutMs doesn't allow callers to set the timeout to less than > 1 second > -- > > Key: PHOENIX-2285 > URL: https://issues.apache.org/jira/browse/PHOENIX-2285 > Project: Phoenix > Issue Type: Bug >Affects Versions: 4.5.2 >Reporter: Jan Fernando >Assignee: Jan Fernando > Attachments: PHOENIX-2285-v1.txt, PHOENIX-2285-v2.txt > > > When creating a Phoenix JDBC connection I have a use case where I want to > override the default value of phoenix.query.timeoutMs to a value of 200 ms. > Currently if you set phoenix.query.timeoutMs to less than 1000 ms, the > timeout gets rounded up to 1000ms. This is because in > PhoenixStatement.getDefaultQueryTimeout() we convert the value of > phoenix.query.timeoutMs to seconds in order to be compliant with JDBC. In > BaseResultIterators we then convert it back to millis. As a result of the > conversion we loose the millisecond fidelity. > A possible solution is to store the timeout value stored on the > PhoenixStatement in both seconds and milliseconds. Then, in > BaseResultIterators when we read the value from the statement we can check if > the value exists in millisecond fidelity and if so use that value. Otherwise > we would use the value in second granularity and convert. > This would allow Phoenix to remain JDBC compatible with second level > granularity for setting query timeouts on statements, but allow millisecond > granularity of timeouts by explicitly setting phoenix.query.timeoutMs on > connection properties. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (PHOENIX-2285) phoenix.query.timeoutMs doesn't allow callers to set the timeout to less than 1 second
[ https://issues.apache.org/jira/browse/PHOENIX-2285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14940685#comment-14940685 ] Jan Fernando commented on PHOENIX-2285: --- I'll get the repo setup and tackle this tomorrow. > phoenix.query.timeoutMs doesn't allow callers to set the timeout to less than > 1 second > -- > > Key: PHOENIX-2285 > URL: https://issues.apache.org/jira/browse/PHOENIX-2285 > Project: Phoenix > Issue Type: Bug >Affects Versions: 4.5.2 >Reporter: Jan Fernando >Assignee: Jan Fernando > Attachments: PHOENIX-2285-v1.txt, PHOENIX-2285-v2.txt > > > When creating a Phoenix JDBC connection I have a use case where I want to > override the default value of phoenix.query.timeoutMs to a value of 200 ms. > Currently if you set phoenix.query.timeoutMs to less than 1000 ms, the > timeout gets rounded up to 1000ms. This is because in > PhoenixStatement.getDefaultQueryTimeout() we convert the value of > phoenix.query.timeoutMs to seconds in order to be compliant with JDBC. In > BaseResultIterators we then convert it back to millis. As a result of the > conversion we loose the millisecond fidelity. > A possible solution is to store the timeout value stored on the > PhoenixStatement in both seconds and milliseconds. Then, in > BaseResultIterators when we read the value from the statement we can check if > the value exists in millisecond fidelity and if so use that value. Otherwise > we would use the value in second granularity and convert. > This would allow Phoenix to remain JDBC compatible with second level > granularity for setting query timeouts on statements, but allow millisecond > granularity of timeouts by explicitly setting phoenix.query.timeoutMs on > connection properties. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (PHOENIX-2285) phoenix.query.timeoutMs doesn't allow callers to set the timeout to less than 1 second
[ https://issues.apache.org/jira/browse/PHOENIX-2285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14937948#comment-14937948 ] James Taylor commented on PHOENIX-2285: --- +1. Looks great - thanks for the contribution, [~jfernando_sfdc]. How about testing out your new commit bit and checking this into master, 4.x, and 4.5 branches? > phoenix.query.timeoutMs doesn't allow callers to set the timeout to less than > 1 second > -- > > Key: PHOENIX-2285 > URL: https://issues.apache.org/jira/browse/PHOENIX-2285 > Project: Phoenix > Issue Type: Bug >Affects Versions: 4.5.2 >Reporter: Jan Fernando >Assignee: Jan Fernando > Attachments: PHOENIX-2285-v1.txt, PHOENIX-2285-v2.txt > > > When creating a Phoenix JDBC connection I have a use case where I want to > override the default value of phoenix.query.timeoutMs to a value of 200 ms. > Currently if you set phoenix.query.timeoutMs to less than 1000 ms, the > timeout gets rounded up to 1000ms. This is because in > PhoenixStatement.getDefaultQueryTimeout() we convert the value of > phoenix.query.timeoutMs to seconds in order to be compliant with JDBC. In > BaseResultIterators we then convert it back to millis. As a result of the > conversion we loose the millisecond fidelity. > A possible solution is to store the timeout value stored on the > PhoenixStatement in both seconds and milliseconds. Then, in > BaseResultIterators when we read the value from the statement we can check if > the value exists in millisecond fidelity and if so use that value. Otherwise > we would use the value in second granularity and convert. > This would allow Phoenix to remain JDBC compatible with second level > granularity for setting query timeouts on statements, but allow millisecond > granularity of timeouts by explicitly setting phoenix.query.timeoutMs on > connection properties. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (PHOENIX-2285) phoenix.query.timeoutMs doesn't allow callers to set the timeout to less than 1 second
[ https://issues.apache.org/jira/browse/PHOENIX-2285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14933522#comment-14933522 ] Jan Fernando commented on PHOENIX-2285: --- [~jamestaylor] I uploaded a second path file the unnecessary throws clause removed. Let me know if that works. Thanks! > phoenix.query.timeoutMs doesn't allow callers to set the timeout to less than > 1 second > -- > > Key: PHOENIX-2285 > URL: https://issues.apache.org/jira/browse/PHOENIX-2285 > Project: Phoenix > Issue Type: Bug >Affects Versions: 4.5.2 >Reporter: Jan Fernando >Assignee: Jan Fernando > Attachments: PHOENIX-2285-v1.txt, PHOENIX-2285-v2.txt > > > When creating a Phoenix JDBC connection I have a use case where I want to > override the default value of phoenix.query.timeoutMs to a value of 200 ms. > Currently if you set phoenix.query.timeoutMs to less than 1000 ms, the > timeout gets rounded up to 1000ms. This is because in > PhoenixStatement.getDefaultQueryTimeout() we convert the value of > phoenix.query.timeoutMs to seconds in order to be compliant with JDBC. In > BaseResultIterators we then convert it back to millis. As a result of the > conversion we loose the millisecond fidelity. > A possible solution is to store the timeout value stored on the > PhoenixStatement in both seconds and milliseconds. Then, in > BaseResultIterators when we read the value from the statement we can check if > the value exists in millisecond fidelity and if so use that value. Otherwise > we would use the value in second granularity and convert. > This would allow Phoenix to remain JDBC compatible with second level > granularity for setting query timeouts on statements, but allow millisecond > granularity of timeouts by explicitly setting phoenix.query.timeoutMs on > connection properties. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (PHOENIX-2285) phoenix.query.timeoutMs doesn't allow callers to set the timeout to less than 1 second
[
https://issues.apache.org/jira/browse/PHOENIX-2285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14908918#comment-14908918
]
James Taylor commented on PHOENIX-2285:
---
+1. Looks great, [~jfernando_sfdc]. One minor nit, remove the throw
SQLException here:
{code}
+public int getQueryTimeoutInMillis() throws SQLException {
+return queryTimeoutMillis;
+}
{code}
> phoenix.query.timeoutMs doesn't allow callers to set the timeout to less than
> 1 second
> --
>
> Key: PHOENIX-2285
> URL: https://issues.apache.org/jira/browse/PHOENIX-2285
> Project: Phoenix
> Issue Type: Bug
>Affects Versions: 4.5.2
>Reporter: Jan Fernando
>Assignee: Jan Fernando
> Attachments: PHOENIX-2285-v1.txt
>
>
> When creating a Phoenix JDBC connection I have a use case where I want to
> override the default value of phoenix.query.timeoutMs to a value of 200 ms.
> Currently if you set phoenix.query.timeoutMs to less than 1000 ms, the
> timeout gets rounded up to 1000ms. This is because in
> PhoenixStatement.getDefaultQueryTimeout() we convert the value of
> phoenix.query.timeoutMs to seconds in order to be compliant with JDBC. In
> BaseResultIterators we then convert it back to millis. As a result of the
> conversion we loose the millisecond fidelity.
> A possible solution is to store the timeout value stored on the
> PhoenixStatement in both seconds and milliseconds. Then, in
> BaseResultIterators when we read the value from the statement we can check if
> the value exists in millisecond fidelity and if so use that value. Otherwise
> we would use the value in second granularity and convert.
> This would allow Phoenix to remain JDBC compatible with second level
> granularity for setting query timeouts on statements, but allow millisecond
> granularity of timeouts by explicitly setting phoenix.query.timeoutMs on
> connection properties.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (PHOENIX-2285) phoenix.query.timeoutMs doesn't allow callers to set the timeout to less than 1 second
[ https://issues.apache.org/jira/browse/PHOENIX-2285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14907067#comment-14907067 ] Jan Fernando commented on PHOENIX-2285: --- [~jamestaylor] I've attached a potential patch that will honor the phoenix.query.timeMs value to millisecond level granularity. The main changes are in PhoenixStatement, where I made changes to store the timeout value in milliseconds and not seconds. To honor the JDBC contract for setting query timeouts I now do the conversion on the set and get methods instead of at initialization time. BaseResultIterators no longer needs to convert the query time out to millis and just uses the millisecond value stored on the statement. In terms of testing, I couldn't find any tests specific to the behavior of phoenix.query.timeMs and so went ahead added both unit and IT tests that are in the proposed patch. Let me know what you think. cc [~samarthjain] > phoenix.query.timeoutMs doesn't allow callers to set the timeout to less than > 1 second > -- > > Key: PHOENIX-2285 > URL: https://issues.apache.org/jira/browse/PHOENIX-2285 > Project: Phoenix > Issue Type: Bug >Affects Versions: 4.5.2 >Reporter: Jan Fernando >Assignee: Jan Fernando > Attachments: PHOENIX-2285-v1.txt > > > When creating a Phoenix JDBC connection I have a use case where I want to > override the default value of phoenix.query.timeoutMs to a value of 200 ms. > Currently if you set phoenix.query.timeoutMs to less than 1000 ms, the > timeout gets rounded up to 1000ms. This is because in > PhoenixStatement.getDefaultQueryTimeout() we convert the value of > phoenix.query.timeoutMs to seconds in order to be compliant with JDBC. In > BaseResultIterators we then convert it back to millis. As a result of the > conversion we loose the millisecond fidelity. > A possible solution is to store the timeout value stored on the > PhoenixStatement in both seconds and milliseconds. Then, in > BaseResultIterators when we read the value from the statement we can check if > the value exists in millisecond fidelity and if so use that value. Otherwise > we would use the value in second granularity and convert. > This would allow Phoenix to remain JDBC compatible with second level > granularity for setting query timeouts on statements, but allow millisecond > granularity of timeouts by explicitly setting phoenix.query.timeoutMs on > connection properties. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
