Skip to content

Commit

Permalink
[BUG] Fix property failure for soft delete. (#3220)
Browse files Browse the repository at this point in the history
[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.

## Description of changes

*Summarize the changes made by this PR.*
 - Improvements & Bug fixes
   - Fix property failure for soft delete.

## Test plan
*How are these changes tested?*

- [ ] Tests pass locally with `pytest` for python, `yarn test` for js,
`cargo test` for rust

## Documentation Changes
*Are all docstrings for user-facing APIs updated if required? Do we need
to make documentation changes in the [docs
repository](https://github.com/chroma-core/docs)?*
n/a
  • Loading branch information
rohitcpbot authored Dec 4, 2024
1 parent 0df312a commit 5861af3
Showing 1 changed file with 23 additions and 34 deletions.
57 changes: 23 additions & 34 deletions go/pkg/sysdb/coordinator/table_catalog.go
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

0 comments on commit 5861af3

Please sign in to comment.