From 10fa171871afcce85c972b2c4ee2e53ec17c301f Mon Sep 17 00:00:00 2001 From: rohitcpbot Date: Mon, 2 Dec 2024 05:09:17 -0800 Subject: [PATCH 1/2] [BUG] Fix property failure for soft delete. Renaming soft deleted collection at delete time since modify and create can try to re-create a collection with the same name. --- go/pkg/sysdb/coordinator/table_catalog.go | 57 +++++++++-------------- 1 file changed, 23 insertions(+), 34 deletions(-) diff --git a/go/pkg/sysdb/coordinator/table_catalog.go b/go/pkg/sysdb/coordinator/table_catalog.go index c0e27445e35..f4c231e011b 100644 --- a/go/pkg/sysdb/coordinator/table_catalog.go +++ b/go/pkg/sysdb/coordinator/table_catalog.go @@ -3,6 +3,7 @@ package coordinator import ( "context" "fmt" + "math/rand" "time" "github.com/chroma-core/chroma/go/pkg/common" @@ -244,20 +245,6 @@ func (tc *Catalog) createCollectionImpl(txCtx context.Context, createCollection } else { return nil, false, common.ErrCollectionUniqueConstraintViolation } - } else { - // If collection is soft-deleted, then new collection will throw an error since name should be unique, so we need to rename it. - isSoftDeleted, sdCollectionID, err := tc.metaDomain.CollectionDb(txCtx).CheckCollectionIsSoftDeleted(createCollection.Name, tenantID, databaseName) - if err != nil { - return nil, false, fmt.Errorf("failed to create collection: %w", err) - } - if isSoftDeleted { - log.Info("new collection create request with same name as collection that was soft deleted", zap.Any("collection", createCollection)) - // Rename the soft deleted collection to a new name with timestamp - err = tc.renameSoftDeletedCollection(txCtx, sdCollectionID, createCollection.Name, tenantID, databaseName) - if err != nil { - return nil, false, fmt.Errorf("failed to create collection: %w", err) - } - } } dbCollection := &dbmodel.Collection{ @@ -389,25 +376,6 @@ func (tc *Catalog) hardDeleteCollection(ctx context.Context, deleteCollection *m }) } -func (tc *Catalog) renameSoftDeletedCollection(ctx context.Context, collectionID string, collectionName string, tenantID string, databaseName string) error { - log.Info("Renaming soft deleted collection", zap.String("collectionID", collectionID), zap.String("collectionName", collectionName), zap.Any("tenantID", tenantID), zap.String("databaseName", databaseName)) - // Generate new name with timestamp - newName := fmt.Sprintf("deleted_%s_%d", collectionName, time.Now().Unix()) - - dbCollection := &dbmodel.Collection{ - ID: collectionID, - Name: &newName, - IsDeleted: true, - } // Not updating the timestamp or updated_at. - - err := tc.metaDomain.CollectionDb(ctx).Update(dbCollection) - if err != nil { - log.Error("rename soft deleted collection failed", zap.Error(err)) - return fmt.Errorf("collection rename failed due to update error: %w", err) - } - return nil -} - func (tc *Catalog) softDeleteCollection(ctx context.Context, deleteCollection *model.DeleteCollection) error { log.Info("Soft deleting collection", zap.Any("softDeleteCollection", deleteCollection)) return tc.txImpl.Transaction(ctx, func(txCtx context.Context) error { @@ -420,8 +388,13 @@ func (tc *Catalog) softDeleteCollection(ctx context.Context, deleteCollection *m return common.ErrCollectionDeleteNonExistingCollection } + // Generate new name with timestamp and random number + oldName := *collections[0].Collection.Name + newName := fmt.Sprintf("deleted_%s_%d_%d", oldName, time.Now().Unix(), rand.Intn(1000)) + dbCollection := &dbmodel.Collection{ ID: deleteCollection.ID.String(), + Name: &newName, IsDeleted: true, Ts: deleteCollection.Ts, UpdatedAt: time.Now(), @@ -461,13 +434,29 @@ func (tc *Catalog) UpdateCollection(ctx context.Context, updateCollection *model var result *model.Collection err := tc.txImpl.Transaction(ctx, func(txCtx context.Context) error { + // Check if collection exists + collections, err := tc.metaDomain.CollectionDb(txCtx).GetCollections( + types.FromUniqueID(updateCollection.ID), + nil, + updateCollection.TenantID, + updateCollection.DatabaseName, + nil, + nil, + ) + if err != nil { + return err + } + if len(collections) == 0 { + return common.ErrCollectionNotFound + } + dbCollection := &dbmodel.Collection{ ID: updateCollection.ID.String(), Name: updateCollection.Name, Dimension: updateCollection.Dimension, Ts: ts, } - err := tc.metaDomain.CollectionDb(txCtx).Update(dbCollection) + err = tc.metaDomain.CollectionDb(txCtx).Update(dbCollection) if err != nil { return err } From 7296f3effb8850f63fabece16939bd323e146a15 Mon Sep 17 00:00:00 2001 From: Rohit P Date: Tue, 3 Dec 2024 10:59:24 -0800 Subject: [PATCH 2/2] Update table_catalog.go --- go/pkg/sysdb/coordinator/table_catalog.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/pkg/sysdb/coordinator/table_catalog.go b/go/pkg/sysdb/coordinator/table_catalog.go index f4c231e011b..4691cae7ed7 100644 --- a/go/pkg/sysdb/coordinator/table_catalog.go +++ b/go/pkg/sysdb/coordinator/table_catalog.go @@ -390,7 +390,7 @@ func (tc *Catalog) softDeleteCollection(ctx context.Context, deleteCollection *m // Generate new name with timestamp and random number oldName := *collections[0].Collection.Name - newName := fmt.Sprintf("deleted_%s_%d_%d", oldName, time.Now().Unix(), rand.Intn(1000)) + newName := fmt.Sprintf("_deleted_%s_%d_%d", oldName, time.Now().Unix(), rand.Intn(1000)) dbCollection := &dbmodel.Collection{ ID: deleteCollection.ID.String(),