Skip to content

Commit e25f526

Browse files
authored
[CLN]: Revert 5742 (#5782)
## Description of changes _Summarize the changes made by this PR._ - Improvements & Bug fixes - Reverts https://github.com/chroma-core/chroma/pull/5742/files. We are cleaning up the schema code a little bit to have separate reconcile logic for read and write paths so this is in direction of that. - New functionality - ... ## Test plan _How are these changes tested?_ - [x] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Migration plan None ## Observability plan None ## Documentation Changes None
1 parent 285a4d8 commit e25f526

File tree

3 files changed

+15
-35
lines changed

3 files changed

+15
-35
lines changed

rust/segment/src/distributed_hnsw.rs

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -97,12 +97,9 @@ impl DistributedHNSWSegmentWriter {
9797
) -> Result<Box<DistributedHNSWSegmentWriter>, Box<DistributedHNSWSegmentFromSegmentError>>
9898
{
9999
let hnsw_configuration = collection
100-
.schema
101-
.as_ref()
102-
.map(|schema| schema.get_internal_hnsw_config_with_legacy_fallback(segment))
103-
.transpose()
100+
.config
101+
.get_hnsw_config_with_legacy_fallback(segment)
104102
.map_err(DistributedHNSWSegmentFromSegmentError::InvalidHnswConfiguration)?
105-
.flatten()
106103
.ok_or(DistributedHNSWSegmentFromSegmentError::MissingHnswConfiguration)?;
107104

108105
// TODO: this is hacky, we use the presence of files to determine if we need to load or create the index
@@ -317,12 +314,9 @@ impl DistributedHNSWSegmentReader {
317314
) -> Result<Box<DistributedHNSWSegmentReader>, Box<DistributedHNSWSegmentFromSegmentError>>
318315
{
319316
let hnsw_configuration = collection
320-
.schema
321-
.as_ref()
322-
.map(|schema| schema.get_internal_hnsw_config_with_legacy_fallback(segment))
323-
.transpose()
317+
.config
318+
.get_hnsw_config_with_legacy_fallback(segment)
324319
.map_err(DistributedHNSWSegmentFromSegmentError::InvalidHnswConfiguration)?
325-
.flatten()
326320
.ok_or(DistributedHNSWSegmentFromSegmentError::MissingHnswConfiguration)?;
327321

328322
// TODO: this is hacky, we use the presence of files to determine if we need to load or create the index

rust/segment/src/distributed_spann.rs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ use chroma_index::spann::types::{
1515
use chroma_index::IndexUuid;
1616
use chroma_index::{hnsw_provider::HnswIndexProvider, spann::types::SpannIndexWriter};
1717
use chroma_types::Collection;
18+
use chroma_types::KnnIndex;
19+
use chroma_types::Schema;
1820
use chroma_types::SchemaError;
1921
use chroma_types::SegmentUuid;
2022
use chroma_types::HNSW_PATH;
@@ -110,13 +112,14 @@ impl SpannSegmentWriter {
110112
return Err(SpannSegmentWriterError::InvalidArgument);
111113
}
112114

113-
let schema = collection.schema.as_ref().ok_or_else(|| {
114-
SpannSegmentWriterError::InvalidSchema(SchemaError::InvalidSchema {
115-
reason: "Schema is None".to_string(),
116-
})
117-
})?;
115+
let reconciled_schema = Schema::reconcile_schema_and_config(
116+
collection.schema.as_ref(),
117+
Some(&collection.config),
118+
KnnIndex::Spann,
119+
)
120+
.map_err(SpannSegmentWriterError::InvalidSchema)?;
118121

119-
let params = schema
122+
let params = reconciled_schema
120123
.get_internal_spann_config()
121124
.ok_or(SpannSegmentWriterError::MissingSpannConfiguration)?;
122125

rust/worker/src/execution/orchestration/compact.rs

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ use chroma_system::{
2929
OrchestratorContext, PanicError, TaskError, TaskMessage, TaskResult,
3030
};
3131
use chroma_types::{
32-
AttachedFunction, AttachedFunctionUuid, Chunk, Collection, CollectionUuid, KnnIndex, LogRecord,
32+
AttachedFunction, AttachedFunctionUuid, Chunk, Collection, CollectionUuid, LogRecord,
3333
NonceUuid, Schema, SchemaError, Segment, SegmentFlushInfo, SegmentType, SegmentUuid,
3434
};
3535
use opentelemetry::trace::TraceContextExt;
@@ -1124,7 +1124,7 @@ impl Handler<TaskResult<GetCollectionAndSegmentsOutput, GetCollectionAndSegments
11241124
}
11251125

11261126
// Store output collection
1127-
let mut output_collection = output.output.collection.clone();
1127+
let output_collection = output.output.collection.clone();
11281128
if self
11291129
.output_collection
11301130
.set(output_collection.clone())
@@ -1295,23 +1295,6 @@ impl Handler<TaskResult<GetCollectionAndSegmentsOutput, GetCollectionAndSegments
12951295
Some(writer) => writer,
12961296
None => return,
12971297
};
1298-
if self
1299-
.ok_or_terminate(
1300-
match vector_segment.r#type {
1301-
SegmentType::Spann => output_collection
1302-
.reconcile_schema_with_config(KnnIndex::Spann)
1303-
.map_err(CompactionError::from),
1304-
_ => output_collection
1305-
.reconcile_schema_with_config(KnnIndex::Hnsw)
1306-
.map_err(CompactionError::from),
1307-
},
1308-
ctx,
1309-
)
1310-
.await
1311-
.is_none()
1312-
{
1313-
return;
1314-
}
13151298
let (hnsw_index_uuid, vector_writer, is_vector_segment_spann) = match vector_segment.r#type
13161299
{
13171300
SegmentType::Spann => match self

0 commit comments

Comments
 (0)