Skip to content

Commit 98433a2

Browse files
committed
[ENH]: allow deleting v0 from version file
1 parent aa00a67 commit 98433a2

File tree

5 files changed

+24
-8
lines changed

5 files changed

+24
-8
lines changed

go/pkg/sysdb/coordinator/table_catalog.go

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1108,6 +1108,7 @@ func (tc *Catalog) createSegmentImpl(txCtx context.Context, createSegment *model
11081108
Type: createSegment.Type,
11091109
Scope: createSegment.Scope,
11101110
Ts: ts,
1111+
FilePaths: createSegment.FilePaths,
11111112
}
11121113
err := tc.metaDomain.SegmentDb(txCtx).Insert(dbSegment)
11131114
if err != nil {
@@ -1964,11 +1965,11 @@ func (tc *Catalog) getNumberOfActiveVersions(versionFilePb *coordinatorpb.Collec
19641965
}
19651966

19661967
func (tc *Catalog) getOldestVersionTs(versionFilePb *coordinatorpb.CollectionVersionFile) time.Time {
1967-
if versionFilePb.GetVersionHistory() == nil || len(versionFilePb.GetVersionHistory().Versions) <= 1 {
1968+
if versionFilePb.GetVersionHistory() == nil || len(versionFilePb.GetVersionHistory().Versions) == 0 {
19681969
// Returning a zero timestamp that represents an unset value.
19691970
return time.Time{}
19701971
}
1971-
oldestVersionTs := versionFilePb.GetVersionHistory().Versions[1].CreatedAtSecs
1972+
oldestVersionTs := versionFilePb.GetVersionHistory().Versions[0].CreatedAtSecs
19721973

19731974
return time.Unix(oldestVersionTs, 0)
19741975
}
@@ -2004,7 +2005,7 @@ func (tc *Catalog) DeleteVersionEntriesForCollection(ctx context.Context, tenant
20042005
}
20052006

20062007
numActiveVersions := tc.getNumberOfActiveVersions(versionFilePb)
2007-
if numActiveVersions <= 1 {
2008+
if numActiveVersions < 1 {
20082009
// No remaining valid versions after GC.
20092010
return errors.New("no valid versions after gc")
20102011
}
@@ -2054,11 +2055,15 @@ func (tc *Catalog) DeleteCollectionVersion(ctx context.Context, req *coordinator
20542055
result := coordinatorpb.DeleteCollectionVersionResponse{
20552056
CollectionIdToSuccess: make(map[string]bool),
20562057
}
2058+
var firstErr error
20572059
for _, collectionVersionList := range req.Versions {
20582060
err := tc.DeleteVersionEntriesForCollection(ctx, collectionVersionList.TenantId, collectionVersionList.CollectionId, collectionVersionList.Versions)
20592061
result.CollectionIdToSuccess[collectionVersionList.CollectionId] = err == nil
2062+
if firstErr == nil && err != nil {
2063+
firstErr = err
2064+
}
20602065
}
2061-
return &result, nil
2066+
return &result, firstErr
20622067
}
20632068

20642069
func (tc *Catalog) BatchGetCollectionVersionFilePaths(ctx context.Context, collectionIds []string) (*coordinatorpb.BatchGetCollectionVersionFilePathsResponse, error) {

go/pkg/sysdb/coordinator/table_catalog_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -759,7 +759,7 @@ func TestCatalog_DeleteCollectionVersion_CollectionNotFound(t *testing.T) {
759759
resp, err := catalog.DeleteCollectionVersion(context.Background(), req)
760760

761761
// Verify results
762-
assert.NoError(t, err)
762+
assert.Error(t, err)
763763
assert.NotNil(t, resp)
764764
assert.False(t, resp.CollectionIdToSuccess[collectionID])
765765

go/pkg/sysdb/grpc/proto_model_convert.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package grpc
22

33
import (
4+
"time"
5+
46
"github.com/chroma-core/chroma/go/pkg/common"
57
"github.com/chroma-core/chroma/go/pkg/proto/coordinatorpb"
68
"github.com/chroma-core/chroma/go/pkg/sysdb/coordinator/model"
@@ -131,6 +133,7 @@ func convertToCreateCollectionModel(req *coordinatorpb.CreateCollectionRequest)
131133
GetOrCreate: req.GetGetOrCreate(),
132134
TenantID: req.GetTenant(),
133135
DatabaseName: req.GetDatabase(),
136+
Ts: time.Now().Unix(),
134137
}, nil
135138
}
136139

rust/garbage_collector/src/operators/delete_versions_at_sysdb.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ pub struct DeleteVersionsAtSysDbOutput {
3838

3939
#[derive(Error, Debug)]
4040
pub enum DeleteVersionsAtSysDbError {
41+
#[error("Unknown error occurred when deleting versions at sysdb")]
42+
UnknownError,
4143
#[error("Error deleting versions in sysdb: {0}")]
4244
SysDBError(String),
4345
#[error("Error deleting version file {path}: {message}")]
@@ -157,7 +159,13 @@ impl Operator<DeleteVersionsAtSysDbInput, DeleteVersionsAtSysDbOutput>
157159
.delete_collection_version(vec![input.versions_to_delete.clone()])
158160
.await
159161
{
160-
Ok(_) => {
162+
Ok(results) => {
163+
for (_, was_successful) in results {
164+
if !was_successful {
165+
return Err(DeleteVersionsAtSysDbError::UnknownError);
166+
}
167+
}
168+
161169
tracing::info!(
162170
versions = ?input.versions_to_delete.versions,
163171
"Successfully deleted versions from SysDB"

rust/sysdb/src/sysdb.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1331,14 +1331,14 @@ impl ChromaError for MarkVersionForDeletionError {
13311331

13321332
#[derive(Error, Debug)]
13331333
pub enum DeleteCollectionVersionError {
1334-
#[error("Failed to delete version")]
1334+
#[error("Failed to delete version: {0}")]
13351335
FailedToDeleteVersion(#[from] tonic::Status),
13361336
}
13371337

13381338
impl ChromaError for DeleteCollectionVersionError {
13391339
fn code(&self) -> ErrorCodes {
13401340
match self {
1341-
DeleteCollectionVersionError::FailedToDeleteVersion(_) => ErrorCodes::Internal,
1341+
DeleteCollectionVersionError::FailedToDeleteVersion(e) => e.code().into(),
13421342
}
13431343
}
13441344
}

0 commit comments

Comments
 (0)