Skip to content

[ENH]: sysdb changes to support moving collection hard deletes to garbage collector #4607

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
May 28, 2025
Merged

[ENH]: sysdb changes to support moving collection hard deletes to garbage collector #4607

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions go/cmd/coordinator/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,6 @@ func init() {
Cmd.Flags().IntVar(&conf.DBConfig.MaxOpenConns, "max-open-conns", 10, "MetaTable max open connections")
Cmd.Flags().StringVar(&conf.DBConfig.SslMode, "ssl-mode", "disable", "SSL mode for database connection")

// Soft deletes
Cmd.Flags().BoolVar(&conf.SoftDeleteEnabled, "soft-delete-enabled", true, "Enable soft deletes")
Cmd.Flags().DurationVar(&conf.SoftDeleteCleanupInterval, "soft-delete-cleanup-interval", 30*time.Second, "Soft delete cleanup interval")
Cmd.Flags().DurationVar(&conf.SoftDeleteMaxAge, "soft-delete-max-age", 72*time.Hour, "Soft delete max age")
Cmd.Flags().UintVar(&conf.SoftDeleteCleanupBatchSize, "soft-delete-cleanup-batch-size", 100, "Soft delete cleanup batch size")
// With above values, the soft delete cleaner can remove around 1000 collections in 5 minutes.

// Memberlist
Cmd.Flags().StringVar(&conf.KubernetesNamespace, "kubernetes-namespace", "chroma", "Kubernetes namespace")
Cmd.Flags().DurationVar(&conf.ReconcileInterval, "reconcile-interval", 100*time.Millisecond, "Reconcile interval")
Expand Down
1 change: 1 addition & 0 deletions go/pkg/common/errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ var (
ErrCollectionTooManyFork = errors.New("collection entry has too many forks")
ErrCollectionDeletedWithLocksHeld = errors.New("collection got deleted concurrently even though select for update locks were held. Not possible unless corruption somehow")
ErrMissingLineageFileName = errors.New("missing lineage file name in root collection entry")
ErrCollectionWasNotSoftDeleted = errors.New("collection was not soft deleted")

// Collection metadata errors
ErrUnknownCollectionMetadataType = errors.New("collection metadata value type not supported")
Expand Down
28 changes: 6 additions & 22 deletions go/pkg/sysdb/coordinator/coordinator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,30 +15,18 @@ import (
"go.uber.org/zap"
)

// DeleteMode represents whether to perform a soft or hard delete
type DeleteMode int

const (
// SoftDelete marks records as deleted but keeps them in the database
SoftDelete DeleteMode = iota
// HardDelete permanently removes records from the database
HardDelete
)

// Coordinator is the top level component.
// Currently, it only has the system catalog related APIs and will be extended to
// support other functionalities such as membership managed and propagation.
type Coordinator struct {
ctx context.Context
catalog Catalog
deleteMode DeleteMode
objectStore *s3metastore.S3MetaStore
}

func NewCoordinator(ctx context.Context, deleteMode DeleteMode, objectStore *s3metastore.S3MetaStore, versionFileEnabled bool) (*Coordinator, error) {
func NewCoordinator(ctx context.Context, objectStore *s3metastore.S3MetaStore, versionFileEnabled bool) (*Coordinator, error) {
s := &Coordinator{
ctx: ctx,
deleteMode: deleteMode,
objectStore: objectStore,
}

Expand Down Expand Up @@ -142,14 +130,11 @@ func (s *Coordinator) GetSoftDeletedCollections(ctx context.Context, collectionI
return s.catalog.GetSoftDeletedCollections(ctx, collectionID, tenantID, databaseName, limit)
}

func (s *Coordinator) DeleteCollection(ctx context.Context, deleteCollection *model.DeleteCollection) error {
if s.deleteMode == SoftDelete {
return s.catalog.DeleteCollection(ctx, deleteCollection, true)
}
return s.catalog.DeleteCollection(ctx, deleteCollection, false)
func (s *Coordinator) SoftDeleteCollection(ctx context.Context, deleteCollection *model.DeleteCollection) error {
return s.catalog.DeleteCollection(ctx, deleteCollection, true)
}

func (s *Coordinator) CleanupSoftDeletedCollection(ctx context.Context, deleteCollection *model.DeleteCollection) error {
func (s *Coordinator) FinishCollectionDeletion(ctx context.Context, deleteCollection *model.DeleteCollection) error {
return s.catalog.DeleteCollection(ctx, deleteCollection, false)
}

Expand Down Expand Up @@ -267,7 +252,6 @@ func (s *Coordinator) BatchGetCollectionVersionFilePaths(ctx context.Context, re
return s.catalog.BatchGetCollectionVersionFilePaths(ctx, req.CollectionIds)
}

// SetDeleteMode sets the delete mode for testing
func (c *Coordinator) SetDeleteMode(mode DeleteMode) {
c.deleteMode = mode
func (s *Coordinator) BatchGetCollectionSoftDeleteStatus(ctx context.Context, req *coordinatorpb.BatchGetCollectionSoftDeleteStatusRequest) (*coordinatorpb.BatchGetCollectionSoftDeleteStatusResponse, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

review: should we instead extend the GetCollection method to allow returning soft deleted collections, and then get the statuses with multiple calls?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer a single call to reduce the roundtrips. GetCollection should not return soft deleted collections for its current usages.

return s.catalog.BatchGetCollectionSoftDeleteStatus(ctx, req.CollectionIds)
}
83 changes: 15 additions & 68 deletions go/pkg/sysdb/coordinator/coordinator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func (suite *APIsTestSuite) SetupTest() {
collection.Name = "collection_" + suite.T().Name() + strconv.Itoa(index)
}
ctx := context.Background()
c, err := NewCoordinator(ctx, SoftDelete, suite.s3MetaStore, true)
c, err := NewCoordinator(ctx, suite.s3MetaStore, true)
if err != nil {
suite.T().Fatalf("error creating coordinator: %v", err)
}
Expand Down Expand Up @@ -111,7 +111,7 @@ func (suite *APIsTestSuite) TearDownTest() {
func testCollection(t *rapid.T) {
dbcore.ConfigDatabaseForTesting()
ctx := context.Background()
c, err := NewCoordinator(ctx, HardDelete, nil, false)
c, err := NewCoordinator(ctx, nil, false)
if err != nil {
t.Fatalf("error creating coordinator: %v", err)
}
Expand Down Expand Up @@ -164,7 +164,7 @@ func testCollection(t *rapid.T) {
func testSegment(t *rapid.T) {
dbcore.ConfigDatabaseForTesting()
ctx := context.Background()
c, err := NewCoordinator(ctx, HardDelete, nil, false)
c, err := NewCoordinator(ctx, nil, false)
if err != nil {
t.Fatalf("error creating coordinator: %v", err)
}
Expand Down Expand Up @@ -486,7 +486,7 @@ func (suite *APIsTestSuite) TestCreateGetDeleteCollections() {
DatabaseName: suite.databaseName,
TenantID: suite.tenantName,
}
err = suite.coordinator.DeleteCollection(ctx, deleteCollection)
err = suite.coordinator.SoftDeleteCollection(ctx, deleteCollection)
suite.NoError(err)

results, err = suite.coordinator.GetCollections(ctx, types.NilUniqueID(), nil, suite.tenantName, suite.databaseName, nil, nil)
Expand All @@ -508,7 +508,7 @@ func (suite *APIsTestSuite) TestCreateGetDeleteCollections() {
suite.Empty(byIDResult)

// Duplicate delete throws an exception
err = suite.coordinator.DeleteCollection(ctx, deleteCollection)
err = suite.coordinator.SoftDeleteCollection(ctx, deleteCollection)
suite.Error(err)

// Re-create the deleted collection
Expand Down Expand Up @@ -560,7 +560,7 @@ func (suite *APIsTestSuite) TestCreateGetDeleteCollections() {
DatabaseName: suite.databaseName,
TenantID: suite.tenantName,
}
err = suite.coordinator.DeleteCollection(ctx, deleteCollection)
err = suite.coordinator.SoftDeleteCollection(ctx, deleteCollection)
suite.NoError(err)

// Verify collection and segment were deleted
Expand All @@ -572,68 +572,11 @@ func (suite *APIsTestSuite) TestCreateGetDeleteCollections() {
segments, err = suite.coordinator.GetSegments(ctx, segment.ID, nil, nil, createCollection.ID)
suite.NoError(err)
suite.NotEmpty(segments)
suite.coordinator.deleteMode = HardDelete
err = suite.coordinator.DeleteCollection(ctx, deleteCollection)
err = suite.coordinator.FinishCollectionDeletion(ctx, deleteCollection)
suite.NoError(err)
segments, err = suite.coordinator.GetSegments(ctx, segment.ID, nil, nil, createCollection.ID)
suite.NoError(err)
suite.Empty(segments)

// Check for forward and backward compatibility with soft and hard delete.
// 1. Create a collection with soft delete enabled.
// 2. Delete the collection (i.e. it will be marked as is_deleted)
// 3. Disable soft delete.
// 4. Query for the deleted collection. It should not be found.
// 5. Enable soft delete.
// 6. Query for the deleted collection. It should be found.

suite.coordinator.deleteMode = SoftDelete
collectionId := types.NewUniqueID()
suite.coordinator.CreateCollection(ctx, &model.CreateCollection{
ID: collectionId,
Name: "test_coll_fwd_bkwd_compat",
TenantID: suite.tenantName,
DatabaseName: suite.databaseName,
})
suite.coordinator.DeleteCollection(ctx, &model.DeleteCollection{
ID: collectionId,
DatabaseName: suite.databaseName,
TenantID: suite.tenantName,
})
collection, err := suite.coordinator.GetCollections(ctx, collectionId, nil, suite.tenantName, suite.databaseName, nil, nil)
suite.NoError(err)
// Check that the collection is deleted
suite.Empty(collection)
// Toggle the mode.
suite.coordinator.deleteMode = HardDelete
collection, err = suite.coordinator.GetCollections(ctx, collectionId, nil, suite.tenantName, suite.databaseName, nil, nil)
suite.NoError(err)
// Check that the collection is still deleted
suite.Empty(collection)
// Create a collection and delete while being in HardDelete mode.
anotherCollectionId := types.NewUniqueID()
suite.coordinator.CreateCollection(ctx, &model.CreateCollection{
ID: anotherCollectionId,
Name: "test_coll_fwd_bkwd_compat",
TenantID: suite.tenantName,
DatabaseName: suite.databaseName,
})
suite.coordinator.DeleteCollection(ctx, &model.DeleteCollection{
ID: anotherCollectionId,
DatabaseName: suite.databaseName,
TenantID: suite.tenantName,
})

// Toggle the mode.
suite.coordinator.deleteMode = SoftDelete
collection, err = suite.coordinator.GetCollections(ctx, collectionId, nil, suite.tenantName, suite.databaseName, nil, nil)
suite.NoError(err)
// Check that the collection is still deleted
suite.Empty(collection)
collection, err = suite.coordinator.GetCollections(ctx, anotherCollectionId, nil, suite.tenantName, suite.databaseName, nil, nil)
suite.NoError(err)
// Check that another collection is still deleted
suite.Empty(collection)
}

func (suite *APIsTestSuite) TestCollectionSize() {
Expand Down Expand Up @@ -1310,7 +1253,6 @@ func (suite *APIsTestSuite) TestSoftAndHardDeleteCollection() {
ctx := context.Background()

// Test Hard Delete scenario
suite.coordinator.deleteMode = HardDelete
// Create test collection
testCollection2 := &model.CreateCollection{
ID: types.NewUniqueID(),
Expand All @@ -1324,7 +1266,13 @@ func (suite *APIsTestSuite) TestSoftAndHardDeleteCollection() {
suite.NoError(err)

// Hard delete the collection
err = suite.coordinator.DeleteCollection(ctx, &model.DeleteCollection{
err = suite.coordinator.SoftDeleteCollection(ctx, &model.DeleteCollection{
ID: testCollection2.ID,
TenantID: testCollection2.TenantID,
DatabaseName: testCollection2.DatabaseName,
})
suite.NoError(err)
err = suite.coordinator.FinishCollectionDeletion(ctx, &model.DeleteCollection{
ID: testCollection2.ID,
TenantID: testCollection2.TenantID,
DatabaseName: testCollection2.DatabaseName,
Expand All @@ -1343,7 +1291,6 @@ func (suite *APIsTestSuite) TestSoftAndHardDeleteCollection() {
suite.Empty(softDeletedResults)

// Test Soft Delete scenario
suite.coordinator.deleteMode = SoftDelete
// Create a test collection
testCollection := &model.CreateCollection{
ID: types.NewUniqueID(),
Expand All @@ -1357,7 +1304,7 @@ func (suite *APIsTestSuite) TestSoftAndHardDeleteCollection() {
suite.NoError(err)

// Soft delete the collection
err = suite.coordinator.DeleteCollection(ctx, &model.DeleteCollection{
err = suite.coordinator.SoftDeleteCollection(ctx, &model.DeleteCollection{
ID: testCollection.ID,
TenantID: testCollection.TenantID,
DatabaseName: testCollection.DatabaseName,
Expand Down
Loading
Loading