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

[Fix] Make UserAgent's otherInfo thread-safe #357

Merged
merged 6 commits into from
Oct 4, 2024
Merged

Conversation

mgyucht
Copy link
Contributor

@mgyucht mgyucht commented Oct 4, 2024

Changes

UserAgent.asString() is called by ApiClient every time a request is made. UserAgent contains an otherInfo list of additional metadata that users can add to the user agent as needed. If users concurrently modify this otherInfo value while iterating through it when making a request, ConcurrentModificationException is thrown.

To prevent this, we add synchronized blocks around modifying and reading from otherInfo. Since the time spent constructing the user agent is generally fast, this should not significantly affect performance.

I also added a readme describing the new benchmark module with a small contribution guide, and I renamed the existing load test to align with that contribution guide.

Tests

Added a UserAgentLoadTest to validate that you can safely add to otherInfo and make requests.

Added a WorkspaceClientLoadIT to exercise the SDK end-to-end with the SCIM Me request, concurrently.

Comment on lines +29 to +33
public void testConcurrentConfigBasicAuthAttrs(
@EnvOrSkip("DATABRICKS_HOST") String host,
@EnvOrSkip("DATABRICKS_CLIENT_ID") String clientId,
@EnvOrSkip("DATABRICKS_CLIENT_SECRET") String clientSecret)
throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

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

Much better! :)

@mgyucht mgyucht added this pull request to the merge queue Oct 4, 2024
Merged via the queue into main with commit 67ee6f9 Oct 4, 2024
10 checks passed
@mgyucht mgyucht deleted the thread-safe-user-agent branch October 4, 2024 12:28
rauchy added a commit that referenced this pull request Oct 7, 2024
### Bug Fixes

 * Make `UserAgent`'s `otherInfo` thread-safe ([#357](#357)).
@rauchy rauchy mentioned this pull request Oct 7, 2024
rauchy added a commit that referenced this pull request Oct 7, 2024
 * Make `UserAgent`'s `otherInfo` thread-safe ([#357](#357)).
github-merge-queue bot pushed a commit that referenced this pull request Oct 8, 2024
### Bug Fixes

* Make `UserAgent`'s `otherInfo` thread-safe
([#357](#357)).

Co-authored-by: Omer Lachish <rauchy@users.noreply.github.com>
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.

2 participants