[HOTFIX] applying PR #5369 to rc/2025-08-29 #5379
Conversation
## Description of changes This PR will make the push_logs call initialize the log. Existing logs are unaffected. If a log doesn't exist, a not_found error will be returned or the request will be empty, as appropriate. This deletes the paths that push/pull logs in the go log service. ## Test plan CI ## Migration plan This is part of the migration to the rust log service. We need to plan for how to roll it out such that the rust log service rolls first. ## Observability plan Watch staging. ## Documentation Changes N/A
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
|
Deprecate Go Log Service and Remove Legacy LogService Paths This PR fully removes support for the legacy Go-based log service ('logservice') from the Chroma codebase, completing the transition to the Rust-based log service. All gRPC endpoints and test paths in the Go log server related to log mutation (PushLogs, PullLogs, ScoutLogs, etc.) now return explicit errors instructing clients that these APIs are no longer available and have migrated to Rust. Python distributed failover and migration tests that relied on the old Go log service are deleted. Configuration and deployment (Tiltfile, config YAMLs) are updated to point exclusively to the Rust log service, and any alternate or fallback host logic is eliminated. Property-based tests, integration tests, and utility code are refactored to align with the new Rust-only log protocol, ensuring the system no longer attempts to communicate with the deprecated Go log service. Key Changes• All log mutation-related Affected Areas• Go log service server implementation (go/pkg/log/server) This summary was automatically generated by @propel-code-bot |
|
4 Jobs Failed: PR checks / all-required-pr-checks-passedStep "Decide whether the needed jobs succeeded or failed" from job "all-required-pr-checks-passed" is failing. The last 20 log lines are: PR checks / Python tests / test-cluster-rust-frontend (3.9, chromadb/test/property/test_add.py)Step "Test" from job "Python tests / test-cluster-rust-frontend (3.9, chromadb/test/property/test_add.py)" is failing. The last 20 log lines are: PR checks / Python tests / test-rust-bindings (3.9, chromadb/test/property --ignore-glob chromadb/test/property/test_cross_v...Step "Test" from job "Python tests / test-rust-bindings (3.9, chromadb/test/property --ignore-glob chromadb/test/property/test_cross_v..." is failing. The last 20 log lines are: 1 job failed running on non-Blacksmith runners. Summary: 1 successful workflow, 1 failed workflow
Last updated: 2025-08-29 18:00:39 UTC |
![propel-code-bot[bot]](https://avatars.githubusercontent.com/in/1179004?s=60&v=4){kind=small}
| return Err(Status::not_found(format!( | ||
| "collection {collection_id} not found" |
There was a problem hiding this comment.
[BestPractice]
The error handling strategy has changed from forwarding to proxy services to returning NOT_FOUND errors. This is a significant behavioral change that could break existing clients expecting the previous forwarding behavior. Consider:
- Documenting this breaking change in migration guides
- Adding proper error codes that clearly indicate the service migration
- Ensuring all callers can handle NOT_FOUND appropriately
return Err(Status::not_found(format!(
"collection {collection_id} not found (migrated from legacy log service)"
)));
| let fragments = match reader.scan(offset, Limits::UNLIMITED).await { | ||
| Ok(fragments) => fragments, | ||
| Err(Error::UninitializedLog) => vec![], | ||
| Err(err) => return Err(err), |
There was a problem hiding this comment.
[BestPractice]
The copy function now silently handles UninitializedLog errors by returning an empty fragments vector. This could mask legitimate errors. Consider logging this condition:
let fragments = match reader.scan(offset, Limits::UNLIMITED).await {
Ok(fragments) => fragments,
Err(Error::UninitializedLog) => {
tracing::debug!("Source log is uninitialized, copying empty log");
vec![]
},
Err(err) => return Err(err),
};
This PR cherry-picks the commit a5d9241 onto rc/2025-08-29. If there are unresolved conflicts, please resolve them manually.