Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SNOW-782588: Retry result request for async query if still in progress #824

Merged
merged 1 commit into from
Jun 22, 2023

Conversation

sfc-gh-ext-simba-lb
Copy link
Contributor

@sfc-gh-ext-simba-lb sfc-gh-ext-simba-lb commented Jun 15, 2023

Description

For #768 and #795
https://github.com/snowflakedb/snowflake-sdks-drivers-issues-teamwork/issues/323

If a statement is executed and the execution is completed in 45 seconds, the server returns the result. If the statement execution takes longer to complete, the statement handle is returned.

In the current driver implementation, when making a get request to the /queries/<query-id>/result endpoint, the response will be processed even if the server returns the queryInProgressAsyncCode (333334). Since it doesn't check the response code for in progress queries, it will be marked as complete even though the query hasn't finished and the result was not returned.

This change is to retry the request to the /result endpoint if the query is still in progress so that results are retrieved for queries longer than 45 seconds. Adding a backoff up to 5 seconds similar to the pattern from the jdbc driver: https://github.com/snowflakedb/snowflake-jdbc/blob/master/src/main/java/net/snowflake/client/jdbc/SFAsyncResultSet.java#L125

Checklist

  • Code compiles correctly
  • Run make fmt to fix inconsistent formats
  • Run make lint to get lint errors and fix all of them
  • Created tests which fail without the change (if possible)
  • All tests passing
  • Extended the README / documentation, if necessary

@sfc-gh-ext-simba-lb sfc-gh-ext-simba-lb marked this pull request as ready for review June 15, 2023 17:45
@sfc-gh-ext-simba-lb sfc-gh-ext-simba-lb requested a review from a team as a code owner June 15, 2023 17:45
async.go Outdated Show resolved Hide resolved
async.go Outdated Show resolved Hide resolved
async.go Show resolved Hide resolved
async.go Outdated Show resolved Hide resolved
async.go Show resolved Hide resolved
async.go Show resolved Hide resolved
async_test.go Show resolved Hide resolved
async_test.go Show resolved Hide resolved
async_test.go Outdated Show resolved Hide resolved
@sfc-gh-ext-simba-lb sfc-gh-ext-simba-lb force-pushed the fixLongRunningAsyncQueryIssue branch 2 times, most recently from 1e3c97d to 5ddeff0 Compare June 16, 2023 19:21
async.go Show resolved Hide resolved
async.go Show resolved Hide resolved
async.go Show resolved Hide resolved
async_test.go Show resolved Hide resolved
Copy link
Collaborator

@sfc-gh-igarish sfc-gh-igarish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sfc-gh-ext-simba-lb sfc-gh-ext-simba-lb merged commit 2b61d99 into master Jun 22, 2023
21 checks passed
@sfc-gh-ext-simba-lb sfc-gh-ext-simba-lb deleted the fixLongRunningAsyncQueryIssue branch June 22, 2023 22:46
@github-actions github-actions bot locked and limited conversation to collaborators Jun 22, 2023
@sfc-gh-igarish sfc-gh-igarish added bug Erroneous or unexpected behaviour and removed bug Erroneous or unexpected behaviour labels Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants