WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

Conversation

@jasonvigil
Copy link
Contributor

Description of changes

Given a default client timeout of 60s, this sets a much lower value to prevent the client from using "almost timed-out" connections from the pool.

Ref: hyperium/hyper#2136 (comment)

Test plan

ci

Migration plan

n/a

Observability plan

n/a

Documentation Changes

n/a

@jasonvigil jasonvigil requested a review from rescrv as a code owner December 9, 2025 21:56
@github-actions
Copy link

github-actions bot commented Dec 9, 2025

Reviewer Checklist

Please leverage this checklist to ensure your code review is thorough before approving

Testing, Bugs, Errors, Logs, Documentation

  • Can you think of any use case in which the code does not behave as intended? Have they been tested?
  • Can you think of any inputs or external events that could break the code? Is user input validated and safe? Have they been tested?
  • If appropriate, are there adequate property based tests?
  • If appropriate, are there adequate unit tests?
  • Should any logging, debugging, tracing information be added or removed?
  • Are error messages user-friendly?
  • Have all documentation changes needed been made?
  • Have all non-obvious changes been commented?

System Compatibility

  • Are there any potential impacts on other parts of the system or backward compatibility?
  • Does this change intersect with any items on our roadmap, and if so, is there a plan for fitting them together?

Quality

  • Is this code of a unexpectedly high quality (Readability, Modularity, Intuitiveness)

@propel-code-bot
Copy link
Contributor

propel-code-bot bot commented Dec 9, 2025

Set HTTP client pool idle timeout and bump crate versions

Introduces a 30-second pool idle timeout on the reqwest::Client used by ChromaHttpClient to avoid reusing connections that are about to be dropped by servers with 60-second idle limits. Workspace crates (chroma, chroma-api-types, chroma-error, chroma-types) and the root Cargo.toml dependency table receive synchronized version bumps to 0.8.0/0.7.0.

Key Changes

• Imported std::time::Duration and set reqwest::Client::builder().pool_idle_timeout(Duration::from_secs(30)) in rust/chroma/src/client/chroma_http_client.rs with context comments referencing upstream hyper guidance.
• Incremented chroma crate version to 0.8.0 and updated dependent workspace crate versions (chroma-api-types, chroma-error, chroma-types) to 0.7.0 across Cargo.toml files.
• Aligned root workspace dependency entries in Cargo.toml to reference the bumped crate versions.

Affected Areas

rust/chroma/src/client/chroma_http_client.rs
Cargo.toml
rust/chroma/Cargo.toml
rust/api-types/Cargo.toml
rust/error/Cargo.toml
rust/types/Cargo.toml

This summary was automatically generated by @propel-code-bot

@jasonvigil jasonvigil requested a review from HammadB December 9, 2025 22:00
Copy link
Contributor

@rescrv rescrv left a comment

Choose a reason for hiding this comment

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

You'll need to bump the root Cargo.toml as well.

Might I suggest following this and then we can cut a release: https://github.com/chroma-core/hosted-chroma/blob/main/runbooks/publish-chroma-crates.md

jasonvigil and others added 5 commits December 10, 2025 09:51
Given a default client timeout of 60s, this sets a much lower value to prevent
the client from using "almost timed-out" connections from the pool.

Ref: hyperium/hyper#2136 (comment)
Co-authored-by: propel-code-bot[bot] <203372662+propel-code-bot[bot]@users.noreply.github.com>
@jasonvigil jasonvigil force-pushed the jason/set-client-idle-timeout branch from 9c68ac0 to c01670a Compare December 10, 2025 17:58
@HammadB
Copy link
Collaborator

HammadB commented Dec 10, 2025

This is fine - I do think a proper keep alive configuration and thought is in order.

@jasonvigil jasonvigil enabled auto-merge (squash) December 10, 2025 18:08
@jasonvigil jasonvigil merged commit 0f1de70 into main Dec 10, 2025
63 checks passed
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