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

The errorCode and errorMessage fields of QueryStatus enum are not thread-safe #1370

Closed
arouel opened this issue Apr 28, 2023 · 8 comments
Closed
Assignees
Labels

Comments

@arouel
Copy link
Contributor

arouel commented Apr 28, 2023

  1. What version of JDBC driver are you using?

3.13.30

  1. What operating system and processor architecture are you using?

Any OS

  1. What version of Java are you using?

Java 17

  1. What did you do?

Running many Snowflake queries concurrently.

  1. What did you expect to see?

When multiple queries fail at the same time, the errorCode and errorMessage of the QueryStatus should contain the correct error as seen in the query history, but may contain another error code and error message from a different query that failed at the same time.

  1. Can you set logging to DEBUG and collect the logs?

No

  1. What is your Snowflake account identifier, if any? (Optional)
@arouel arouel added the bug label Apr 28, 2023
@arouel arouel changed the title errorCode and errorMessage of QueryStatus enum is not thread safe The errorCode and errorMessage fields of QueryStatus enum are not thread safe Apr 28, 2023
@arouel arouel changed the title The errorCode and errorMessage fields of QueryStatus enum are not thread safe The errorCode and errorMessage fields of QueryStatus enum are not thread-safe Apr 28, 2023
@arouel arouel mentioned this issue Apr 28, 2023
8 tasks
@sfc-gh-spanaite sfc-gh-spanaite self-assigned this May 17, 2023
@sfc-gh-spanaite sfc-gh-spanaite added the status-triage Issue is under initial triage label May 17, 2023
@sfc-gh-spanaite
Copy link
Contributor

Thanks for reporting it @arouel , PR is being reviewed internally.
CC: @sfc-gh-igarish, since I see you already are looking at it.

@sfc-gh-hchaturvedi
Copy link
Collaborator

Added comments to the PR. Still in review.

@sfc-gh-igarish
Copy link
Collaborator

Ask PR filer to answer the questions in the PR.

@sfc-gh-igarish
Copy link
Collaborator

@sfc-gh-pfus Could you please check the PR and finish merging it? Josh Zana from apps team need this merge to the code line. If you are buys we can assign to other teammates.

@sfc-gh-igarish
Copy link
Collaborator

@sfc-gh-jzana The filer doesn't give the test case to repro and then test after the fix. Can we get it? Are we sharing connection between threads?

@sfc-gh-jzana
Copy link

@sfc-gh-igarish It shouldn't matter whether the connection is shared across threads., Doesn't even require multiple threads for that matter. This is because each enum value of QueryStatus is a singleton, but it tries to temporarily store per-request code and message on the singleton instance.

The test case would look something like this psuedocode:

// start two async queries
queryId1 = executeQuery(<some query that will produce an error>)
queryId2 = executeQuery(<a different query that will produce a different error>)

// Get status on each one once they are both done
queryStatus1 = getQueryStatus(queryId1)
queryStatus2 = getQueryStatus(queryId2)

You would expect that after this, queryStatus1 would have the error message appropriate to queryId1

With the current code, it will not, because the second call will overwrite the error message on the singleton instance of QueryStatus.FAILED_WITH_ERROR

@sfc-gh-pfus
Copy link
Contributor

sfc-gh-pfus commented Jan 10, 2024

The PR was merged and should be the part of the January release.

@sfc-gh-dprzybysz
Copy link
Collaborator

Fix is part of snowflake-jdbc in version 3.14.5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants