Skip to content

Commit dbab3d2

Browse files
committed
[ENH]: allow deleting v0 from version file
1 parent 43ad358 commit dbab3d2

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
@@ -1138,6 +1138,7 @@ func (tc *Catalog) createSegmentImpl(txCtx context.Context, createSegment *model
11381138
Type: createSegment.Type,
11391139
Scope: createSegment.Scope,
11401140
Ts: ts,
1141+
FilePaths: createSegment.FilePaths,
11411142
}
11421143
err := tc.metaDomain.SegmentDb(txCtx).Insert(dbSegment)
11431144
if err != nil {
@@ -2000,11 +2001,11 @@ func (tc *Catalog) getNumberOfActiveVersions(versionFilePb *coordinatorpb.Collec
20002001
}
20012002

20022003
func (tc *Catalog) getOldestVersionTs(versionFilePb *coordinatorpb.CollectionVersionFile) time.Time {
2003-
if versionFilePb.GetVersionHistory() == nil || len(versionFilePb.GetVersionHistory().Versions) <= 1 {
2004+
if versionFilePb.GetVersionHistory() == nil || len(versionFilePb.GetVersionHistory().Versions) == 0 {
20042005
// Returning a zero timestamp that represents an unset value.
20052006
return time.Time{}
20062007
}
2007-
oldestVersionTs := versionFilePb.GetVersionHistory().Versions[1].CreatedAtSecs
2008+
oldestVersionTs := versionFilePb.GetVersionHistory().Versions[0].CreatedAtSecs
20082009

20092010
return time.Unix(oldestVersionTs, 0)
20102011
}
@@ -2041,7 +2042,7 @@ func (tc *Catalog) DeleteVersionEntriesForCollection(ctx context.Context, tenant
20412042
}
20422043

20432044
numActiveVersions := tc.getNumberOfActiveVersions(versionFilePb)
2044-
if numActiveVersions <= 1 {
2045+
if numActiveVersions < 1 {
20452046
// No remaining valid versions after GC.
20462047
return errors.New("no valid versions after gc")
20472048
}
@@ -2091,11 +2092,15 @@ func (tc *Catalog) DeleteCollectionVersion(ctx context.Context, req *coordinator
20912092
result := coordinatorpb.DeleteCollectionVersionResponse{
20922093
CollectionIdToSuccess: make(map[string]bool),
20932094
}
2095+
var firstErr error
20942096
for _, collectionVersionList := range req.Versions {
20952097
err := tc.DeleteVersionEntriesForCollection(ctx, collectionVersionList.TenantId, collectionVersionList.CollectionId, collectionVersionList.Versions)
20962098
result.CollectionIdToSuccess[collectionVersionList.CollectionId] = err == nil
2099+
if firstErr == nil && err != nil {
2100+
firstErr = err
2101+
}
20972102
}
2098-
return &result, nil
2103+
return &result, firstErr
20992104
}
21002105

21012106
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)