Skip to content
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

[BUG] Fix property failure for soft delete. #3220

Merged
merged 2 commits into from
Dec 4, 2024
Merged

[BUG] Fix property failure for soft delete. #3220

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
10fa171
[BUG] Fix property failure for soft delete.
rohitcpbot Dec 2, 2024
7296f3e
Update table_catalog.go
rohitcpbot Dec 3, 2024
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
57 changes: 23 additions & 34 deletions go/pkg/sysdb/coordinator/table_catalog.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package coordinator
import (
"context"
"fmt"
"math/rand"
"time"

"github.com/chroma-core/chroma/go/pkg/common"
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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 {
Expand All @@ -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(),
Expand Down Expand Up @@ -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
}
Expand Down
Loading