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.

Sign up for GitHub

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

Jump to bottom

SNOW-1418523 make analyzer server connection thread safe #2282

Merged
merged 12 commits into from
Sep 25, 2024
Merged

SNOW-1418523 make analyzer server connection thread safe #2282

merged 12 commits into from
Sep 25, 2024

Conversation

sfc-gh-aalam
Copy link
Contributor

@sfc-gh-aalam sfc-gh-aalam commented Sep 11, 2024 •
edited
Loading

  1. Which Jira issue is this PR addressing? Make sure that there is an accompanying issue to your PR.

    Fixes SNOW-1418523

  2. Fill out the following pre-review checklist:

    • I am adding a new automated test(s) to verify correctness of my new code
      • If this test skips Local Testing mode, I'm requesting review from @snowflakedb/local-testing
    • I am adding new logging messages
    • I am adding a new telemetry message
    • I am adding new credentials
    • I am adding a new dependency
    • If this is a new feature/behavior, I'm adding the Local Testing parity changes.

  3. Please describe how your code solves the related issue.

    [NOTE] Easier to review by selecting hide whitespace option when viewing diffs
    This PR makes the following updates:

    1. server_connection.py:
    • Adds _thread_store to create a _cursor per thread to enable concurrent querying
    • Protects _query_listener updates using _lock so all queries can be added in a thread-safe manner
    1. session.py:
    • Create per thread _analyzer to enable concurrent dataframe operations.
    • Use lock to protect generation of actions ids
    • Use lock to protect running alter query tag and updating _query_tag.
    • Protect _session_stage creation with a lock so only one session stage is created and used.

@sfc-gh-aalam sfc-gh-aalam changed the base branch from main to aalam-SNOW-1418523-make-internal-session-variables-thread-safe September 11, 2024 23:58
@sfc-gh-aalam sfc-gh-aalam added the NO-CHANGELOG-UPDATES This pull request does not need to update CHANGELOG.md label Sep 11, 2024
@sfc-gh-aalam sfc-gh-aalam marked this pull request as ready for review September 12, 2024 19:01
@sfc-gh-aalam sfc-gh-aalam requested a review from a team as a code owner September 12, 2024 19:01
@sfc-gh-aalam sfc-gh-aalam requested review from sfc-gh-aling, sfc-gh-yuwang and sfc-gh-jrose and removed request for a team September 12, 2024 19:01
@github-actions GitHub Actions
Copy link

Seems like your changes contain some Local Testing changes, please request review from @snowflakedb/local-testing

@sfc-gh-aalam sfc-gh-aalam requested a review from a team as a code owner September 12, 2024 20:21
@sfc-gh-aalam sfc-gh-aalam requested review from sfc-gh-evandenberg and sfc-gh-rdurrani and removed request for a team September 12, 2024 20:21
@github-actions GitHub Actions
Copy link

Seems like your changes contain some Local Testing changes, please request review from @snowflakedb/local-testing

@sfc-gh-aalam
Merge branch 'aalam-SNOW-1418523-make-internal-session-variables-thre…
723bdf7 
…ad-safe' into aalam-SNOW-1418523-make-analyzer-server_connection-thread-safe
@github-actions GitHub Actions
Copy link

Seems like your changes contain some Local Testing changes, please request review from @snowflakedb/local-testing

This was referenced Sep 12, 2024
Merged
Merged
@sfc-gh-aalam
Merge branch 'aalam-SNOW-1418523-make-internal-session-variables-thre…
4eef3e9 
…ad-safe' into aalam-SNOW-1418523-make-analyzer-server_connection-thread-safe
@github-actions GitHub Actions
Copy link

Seems like your changes contain some Local Testing changes, please request review from @snowflakedb/local-testing

@sfc-gh-aalam
Copy link
Contributor Author

https://ci-dev-142.int.snowflakecomputing.com/job/PythonStoredProcBuildPrecommitTest/1322/

@sfc-gh-aalam
Merge branch 'aalam-SNOW-1418523-make-internal-session-variables-thre…
4260d91 
…ad-safe' into aalam-SNOW-1418523-make-analyzer-server_connection-thread-safe
@github-actions GitHub Actions
Copy link

Seems like your changes contain some Local Testing changes, please request review from @snowflakedb/local-testing

@github-actions GitHub Actions
Copy link

Seems like your changes contain some Local Testing changes, please request review from @snowflakedb/local-testing

@github-actions GitHub Actions
Copy link

Seems like your changes contain some Local Testing changes, please request review from @snowflakedb/local-testing

@sfc-gh-aalam sfc-gh-aalam merged commit 5f140ab into aalam-SNOW-1418523-make-internal-session-variables-thread-safe Sep 25, 2024
31 of 33 checks passed
@sfc-gh-aalam sfc-gh-aalam deleted the aalam-SNOW-1418523-make-analyzer-server_connection-thread-safe branch September 25, 2024 16:50
@github-actions github-actions bot locked and limited conversation to collaborators Sep 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sfc-gh-yuwang sfc-gh-yuwang sfc-gh-yuwang approved these changes

@sfc-gh-aling sfc-gh-aling sfc-gh-aling approved these changes

@sfc-gh-jrose sfc-gh-jrose Awaiting requested review from sfc-gh-jrose sfc-gh-jrose was automatically assigned from snowflakedb/snowpark-python-api-reviewers

@sfc-gh-evandenberg sfc-gh-evandenberg Awaiting requested review from sfc-gh-evandenberg sfc-gh-evandenberg was automatically assigned from snowflakedb/snowpark-pandas-api

@sfc-gh-rdurrani sfc-gh-rdurrani Awaiting requested review from sfc-gh-rdurrani sfc-gh-rdurrani was automatically assigned from snowflakedb/snowpark-pandas-api

Assignees
No one assigned
Labels
NO-CHANGELOG-UPDATES This pull request does not need to update CHANGELOG.md
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sfc-gh-aalam @sfc-gh-aling @sfc-gh-yuwang