Skip to content

[ES-1436915] Modify an e2e test to iterate over the rs in a separate thread#807

Merged
jayantsing-db merged 3 commits into
mainfrom
jayantsing-db/thread-safety
Apr 25, 2025
Merged

[ES-1436915] Modify an e2e test to iterate over the rs in a separate thread#807
jayantsing-db merged 3 commits into
mainfrom
jayantsing-db/thread-safety

Conversation

@jayantsing-db
Copy link
Copy Markdown
Collaborator

@jayantsing-db jayantsing-db commented Apr 25, 2025

Description

NO_CHANGELOG=true

This will help identify potential thread-safety issues, such as problems with state propagation and context management. In this scenario, customers can interact with first-class JDBC objects like Statement and ResultSet from a separate thread, while the JDBC connection object itself is managed in a different thread. The test is designed to simulate this type of multithreaded environment, ensuring that the system can handle concurrent access to these objects without issues. By doing so, it helps uncover any synchronization problems or unintended side effects that may arise when JDBC components are accessed from multiple threads.

Testing

The test is expected to fail now due to a bug in the implementation of DatabricksContextHolder.

Additional Notes to the Reviewer

This will help surface any thread-safety issues like state propagation, etc.
Customers can use the first-class objects like Statement, ResultSet in a separate thread and
the test now tries to emulate the same
@github-actions
Copy link
Copy Markdown

Please ensure that the `NEXT_CHANGELOG.md` file is updated with any relevant changes.  
If this is not necessary for your PR, include this in the PR body:

```
NO_CHANGELOG=true
```

and rerun the workflow.

@jayantsing-db jayantsing-db changed the title Modify the multi-chunk test to iterate over the rs in a separate thread [ES-1436915] Modify an e2e test to iterate over the rs in a separate thread Apr 25, 2025
@jayantsing-db jayantsing-db requested a review from Copilot April 25, 2025 08:39
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates an end-to-end test to iterate over a ResultSet in a separate thread, aiming to surface potential thread-safety issues in the use of first-class objects such as Statement and ResultSet. Key changes include updating the test method signature to throw InterruptedException, creating a separate thread to iterate the result set, and capturing any exceptions occurring in that thread using an AtomicReference.

Comments suppressed due to low confidence (1)

src/test/java/com/databricks/jdbc/integration/fakeservice/tests/MultiChunkExecutionIntegrationTests.java:43

  • [nitpick] Consider renaming 'thread' to a more descriptive name, such as 'resultProcessingThread', to clarify its role in this test.
Thread thread = new Thread(

…s/MultiChunkExecutionIntegrationTests.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@samikshya-db
Copy link
Copy Markdown
Collaborator

Thanks for adding this, J !

@jayantsing-db jayantsing-db merged commit 2fba0bb into databricks:main Apr 25, 2025
13 of 16 checks passed
@jayantsing-db jayantsing-db deleted the jayantsing-db/thread-safety branch April 25, 2025 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants