-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[ENH] When the log offset is behind sysdb, this can repair it safely. #4722
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
base: main
Are you sure you want to change the base?
Conversation
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
Enable Safe Repair of Log Offset to System Database State This PR adds tooling and backend changes to safely synchronize a collection's log offset with the system database (sysdb) if it has fallen behind, preventing illegal backward movement of the offset. The changes span proto, Rust, and Go domains, including a new Rust tool for repair, a strong server-side test for log invariants, and sysdb interface updates to surface required information. Key Changes: Affected Areas: Potential Impact: Functionality: Users and operators can now repair a collection's log offset in case of desynchronization, with safeguards to prevent violating offset monotonicity. Performance: Negligible impact; the repair tool and proto expansion add minimal runtime complexity. Security: No new externally-exposed vectors; repair tool assumes trusted access and sysdb/reflection pairing. Scalability: Neutral-changes make operations safer, even at large scale, but fundamentally do not alter scaling characteristics. Review Focus: Testing Needed• Manual invocation of the new chroma-update-collection-log-offset tool against intentionally skewed log/sysdb states. Code Quality Assessmentrust/log-service/src/lib.rs: Added a well-structured async test ensuring strict invariants. Change is sound and idiomatic. rust/log-service/src/bin/chroma-update-collection-log-offset.rs: Tool is concise, idiomatic, and includes clear error-handling. go/pkg/sysdb/coordinator/table_catalog.go: Refactor maintains style and handles new log_position output safely. go/pkg/sysdb/grpc/collection_service.go: Proto method changes are correctly propagated; code is still readable. go/pkg/sysdb/coordinator/coordinator.go: Simple function signature update; transparent change. idl/chromadb/proto/coordinator.proto: Proto update is minimal, clear, backward compatible as long as clients properly ignore unknown fields. Best PracticesTesting: Backward Compatibility: Validation: Potential Issues• External consumers of CheckCollectionsResponse may break if they don't ignore the new log_position field; confirm downstream compatibility. This summary was automatically generated by @propel-code-bot |
d382541
to
dddd899
Compare
std::process::exit(13); | ||
} | ||
if collection_info.deleted[0] { | ||
eprintln!("cowardly refusing to do anything with a deleted database"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: "deleted collection"?
Description of changes
This adds a tool to make the log offset match sysdb. Then a test to make sure it cannot move things
backwards breaking invariants.
Test plan
Added a test.
pytest
for python,yarn test
for js,cargo test
for rustDocumentation Changes
N/A