-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[ENH]: soft delete databases, add FinishDatabaseDeletion
gRPC method to hard delete databases
#4627
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: feat-gc-hard-delete-collection
Are you sure you want to change the base?
[ENH]: soft delete databases, add FinishDatabaseDeletion
gRPC method to hard delete databases
#4627
Conversation
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
738703d
to
af4051e
Compare
b099250
to
c84c9a6
Compare
af4051e
to
ed65c55
Compare
c84c9a6
to
add6683
Compare
ed65c55
to
230d5ac
Compare
add6683
to
42b9bff
Compare
230d5ac
to
cf6858c
Compare
156ccde
to
0ee676b
Compare
FinishDatabaseDeletion
gRPC method to hard delete databases
0ee676b
to
6ff85b5
Compare
return nil | ||
}) | ||
} | ||
|
||
func (tc *Catalog) GetAllDatabases(ctx context.Context, ts types.Timestamp) ([]*model.Database, error) { |
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.
this method was never used
Implement Soft Delete for Databases and Add FinishDatabaseDeletion gRPC This PR reworks database deletion semantics in the system: database deletions now perform a soft delete by marking the database as deleted, rather than hard-removing it. A new Key Changes: Affected Areas: Potential Impact: Functionality: Changes how databases are deleted throughout the system; existing delete flows perform only soft delete, requiring explicit follow-up for hard deletion via the new FinishDatabaseDeletion endpoint. Performance: Batch hard deletion is implemented efficiently (in chunks of 1000); no major perf regressions expected but GC timing may be affected. Security: Soft delete reduces risk of accidental data loss, but it's critical that the hard delete only acts on eligible databases. Scalability: Improved deletion scalability via batch hard delete and separation of soft/hard deletion, facilitating large-scale cleanup without blocking key operations. Review Focus: Testing Needed• Verify soft-deleted databases are excluded from normal queries immediately after mark/delete. Code Quality Assessmentgo/pkg/sysdb/metastore/db/dao/database.go: Major refactor, clear transaction usage for safe updates, all major code paths adapted. go/pkg/sysdb/grpc/tenant_database_service_test.go: Robust and expanded test coverage for soft/hard deletion and edge cases. go/pkg/sysdb/coordinator/table_catalog.go: All API logic correctly updated; unused methods removed. idl/chromadb/proto/coordinator.proto: Well-structured proto additions, backward compatible. rust/sysdb/src/sysdb.rs: Fully adapted client interface, with structured error handling. Best PracticesAPI Design: Testing: Transactionality: Potential Issues• If a collection reference is not cleaned up properly, hard deletion may not proceed (check for lingering collections). This summary was automatically generated by @propel-code-bot |
Description of changes
Databases are now always soft deleted instead of being hard deleted. Databases are hard deleted when the new
FinishDatabaseDeletion
method is called (the garbage collector calls this in the next PR).Test plan
How are these changes tested?
pytest
for python,yarn test
for js,cargo test
for rustUpdated existing test to cover soft delete behavior.
Documentation Changes
Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the docs section?
n/a