Skip to content

Conversation

@alukach
Copy link
Contributor

@alukach alukach commented Oct 24, 2025

What I'm changing

In an effort to improve the data proxy's performance when communicating with the Source API, we can re-use our reqwest::Client.

From the reqwest::Client Docs:

An asynchronous Client to make Requests with.

...

The Client holds a connection pool internally, so it is advised that
you create one and reuse it.

You do not have to wrap the Client in an [Rc] or [Arc] to reuse it,
because it already uses an [Arc] internally.

How I did it

Rather than creating a new client on each outbound call to the Source API, we instead create it once when we initialize the SourceApi struct and utilize it to make API calls when needed.

Summary of discussion with Claude
  > Should we persists the reqwest client on the SourceApi struct for better performance? 
  
  ⏺ Great question! Let me analyze the current implementation and the potential benefits.
  
  ⏺ Read(src/apis/source/mod.rs)
    ⎿  Read 50 lines
  
  ⏺ Search(pattern: "build_req_client", path: "src/apis/source/mod.rs", output_mode: "content")
    ⎿  Found 6 lines (ctrl+o to expand)
  
  ⏺ Read(src/apis/source/mod.rs)
    ⎿  Read 15 lines
  
  ⏺ Read(src/apis/source/mod.rs)
    ⎿  Read 45 lines

  Yes, absolutely! This would be a significant performance improvement. Let me explain why:

  Current Issues

  Looking at the code, build_req_client() is called 5 times across different API methods:
  - get_account (line 255)
  - get_repository_record (line 428)
  - fetch_data_connection (line 446)
  - fetch_api_key (line 511)
  - fetch_permission (line 591)

  Each call creates a brand new reqwest::Client, which means:

  Performance Problems

  1. Lost Connection Pooling: reqwest::Client maintains an internal HTTP connection pool that reuses TCP connections. By creating a new client each time, you lose this major
  benefit - every request has to:
    - Perform a new DNS lookup
    - Establish a new TCP connection
    - Perform TLS handshake (for HTTPS)
    - Tear down the connection after use
  2. Resource Overhead: Each client initialization has CPU and memory overhead
  3. Against Best Practices: The reqwest documentation specifically recommends creating one client and reusing it

  Benefits of Persisting the Client

  1. Connection Reuse: Subsequent requests to api.source.coop can reuse existing TCP connections (~20-50ms savings per request)
  2. DNS Caching: The client caches DNS lookups internally
  3. Better Resource Management: Fewer file descriptors, more efficient connection management
  4. Thread Safe: reqwest::Client uses Arc internally, so it's cheap to clone and thread-safe - perfect for this struct that's already using Arc for caches

  Recommended Changes

╭─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
│ Ready to code?                                                                                                                                                                      │
│                                                                                                                                                                                     │
│ Here is Claude's plan:                                                                                                                                                              │
│ ╭─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮ │
│ │ Persist reqwest::Client on SourceApi                                                                                                                                            │ │
│ │                                                                                                                                                                                 │ │
│ │ Changes to make:                                                                                                                                                                │ │
│ │                                                                                                                                                                                 │ │
│ │ 1. Update SourceApi struct in src/apis/source/mod.rs:                                                                                                                           │ │
│ │   - Add client: reqwest::Client field (after proxy_url)                                                                                                                         │ │
│ │   - Add documentation explaining the client is reused for performance                                                                                                           │ │
│ │ 2. Update SourceApi::new() constructor:                                                                                                                                         │ │
│ │   - Build the reqwest::Client once during initialization                                                                                                                        │ │
│ │   - Store it in the struct instead of building it on every request                                                                                                              │ │
│ │ 3. Update all API methods:                                                                                                                                                      │ │
│ │   - Replace 5 calls to self.build_req_client() with &self.client                                                                                                                │ │
│ │   - Methods: get_account, get_repository_record, fetch_data_connection, fetch_api_key, fetch_permission                                                                         │ │
│ │ 4. Remove build_req_client() method:                                                                                                                                            │ │
│ │   - No longer needed since client is initialized once                                                                                                                           │ │
│ │                                                                                                                                                                                 │ │
│ │ Performance Impact:                                                                                                                                                             │ │
│ │ - Enables HTTP connection pooling and reuse                                                                                                                                     │ │
│ │ - Reduces latency by ~20-50ms per request (avoiding TCP handshake + TLS)                                                                                                        │ │
│ │ - Better resource utilization                                                                                                                                                   │ │
│ │ - Follows reqwest best practices                                                                                                                                                │ │
│ │                                                                                                                                                                                 │ │
│ │ Risk: Very low - reqwest::Client is designed for this pattern and the struct is already Clone-compatible                                                                        │ │
│ ╰─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ │

How to test it

PR Checklist

  • This PR has no breaking changes.
  • I have updated or added new tests to cover the changes in this PR.
  • This PR affects the Source Cooperative Frontend & API,
    and I have opened issue/PR #XXX to track the change.

Related Issues

Copy link
Contributor

@gadomski gadomski left a comment

Choose a reason for hiding this comment

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

Makes sense to me

@alukach alukach merged commit 453b2a8 into main Oct 24, 2025
2 checks passed
@alukach alukach deleted the optimization/persist-source-client branch October 24, 2025 17:00
alukach pushed a commit that referenced this pull request Oct 26, 2025
🤖 I have created a release *beep* *boop*
---


##
[1.0.3](v1.0.2...v1.0.3)
(2025-10-24)


### Performance Improvements

* persist source client across requests
([#95](#95))
([453b2a8](453b2a8))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: source-coop-release[bot] <187876225+source-coop-release[bot]@users.noreply.github.com>
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 this pull request may close these issues.

3 participants