mirror of
https://github.com/milvus-io/milvus.git
synced 2026-07-21 10:15:43 +00:00
feat: support drop field via AlterCollectionSchema (#48988)
## Summary - Support dropping fields from collection schema via `AlterCollectionSchema` RPC with `DropRequest` - Support dropping function fields (cascade delete output fields and indexes) - Support disabling dynamic field via `AlterCollectionSchema` (reuse existing `AlterCollection` logic) - C++ segcore defense-in-depth: skip dropped fields during segment loading issue: #48983 design doc: docs/design-docs/design_docs/20260413-drop-collection-field-design.md ## Test plan - [x] Unit tests for rootcoord drop field/function logic - [x] Unit tests for proxy task validation (drop field constraints) - [x] Unit tests for datacoord index service (dropped field handling) - [x] Unit tests for querynode segment loader (dropped field skipping) - [x] C++ unit tests for SegmentLoadInfo ComputeDiff with dropped fields - [ ] E2E tests: drop scalar field, drop vector field, drop field with index, drop function, disable dynamic field (deferred to QA pending pymilvus RC) 🤖 Generated with [Claude Code](https://claude.ai/code) --------- Signed-off-by: sijie-ni-0214 <sijie.ni@zilliz.com>
This commit is contained in:
@@ -2,8 +2,8 @@
|
||||
|
||||
**Branch**: `feat/drop-collection-field`
|
||||
**Author**: sijie-ni-0214
|
||||
**Date**: April 2026
|
||||
**Scope**: 30 files, +2,182/-600 lines
|
||||
**Date**: May 2026
|
||||
**Scope**: 40 files, +3,852/-1,067 lines
|
||||
|
||||
---
|
||||
|
||||
@@ -21,7 +21,7 @@ This feature enables users to dynamically drop fields and functions from existin
|
||||
2. **Backward compatibility**: Existing segments with dropped-field data must remain loadable and queryable
|
||||
3. **Cascade cleanup**: Indexes on dropped fields must be automatically removed
|
||||
4. **Field ID safety**: Dropped field IDs must never be reused to prevent data corruption
|
||||
5. **Concurrency safety**: Reuse the existing `AlterCollectionSchema` mutual exclusion and schema version consistency gateway
|
||||
5. **Concurrency safety**: Use the existing schema DDL execution path
|
||||
6. **Unified API**: Integrate into the existing `AlterCollectionSchema` RPC rather than introducing a separate RPC
|
||||
|
||||
### 1.3 Design Principles
|
||||
@@ -45,8 +45,6 @@ This feature enables users to dynamically drop fields and functions from existin
|
||||
v
|
||||
+------------------------------------------------------------------------------+
|
||||
| PROXY |
|
||||
| * Mutual exclusion: alterSchemaInFlight (one schema change per collection) |
|
||||
| * Schema version consistency check (all segments aligned) |
|
||||
| * Validate drop constraints (not PK, not partition key, not last vector...) |
|
||||
| * Forward to RootCoord via MixCoord |
|
||||
+------------------------------------------------------------------------------+
|
||||
@@ -77,7 +75,7 @@ This feature enables users to dynamically drop fields and functions from existin
|
||||
|
||||
| Component | Responsibility |
|
||||
|-----------|----------------|
|
||||
| **Proxy** | Request validation, concurrency control, schema version consistency gate |
|
||||
| **Proxy** | Request validation and DDL scheduling |
|
||||
| **RootCoord** | Schema construction, field ID management, WAL broadcast, cascade index deletion |
|
||||
| **QueryNode (Go)** | Filter dropped-field binlogs and indexes during segment loading |
|
||||
| **Segcore (C++)** | Schema-driven field filtering in `ComputeDiff*`, `LoadFieldData`, column group loading |
|
||||
@@ -89,7 +87,6 @@ The drop field feature reuses the existing `AlterCollectionSchema` infrastructur
|
||||
|
||||
- **Streaming/WAL layer**: No new message types; uses existing `AlterCollectionMessage`
|
||||
- **DataNode**: No backfill; dropped-field binlogs are left in place
|
||||
- **Schema version consistency**: Reuses the existing gateway from add-field (DataCoord reports consistent/total segment counts)
|
||||
|
||||
---
|
||||
|
||||
@@ -133,10 +130,10 @@ message AlterCollectionSchemaRequest {
|
||||
#### AlterCollectionMessage Extension
|
||||
|
||||
```protobuf
|
||||
// messages.proto - AlterCollectionMessageBody.Updates
|
||||
message Updates {
|
||||
// ... existing fields (schema, properties, etc.)
|
||||
repeated int64 dropped_field_ids = 9; // Field IDs removed from schema,
|
||||
// messages.proto - AlterCollectionMessageHeader
|
||||
message AlterCollectionMessageHeader {
|
||||
// ... existing fields (db_id, collection_id, update_mask, etc.)
|
||||
repeated int64 dropped_field_ids = 6; // Field IDs removed from schema,
|
||||
// used to cascade-delete indexes
|
||||
// in ack callback
|
||||
}
|
||||
@@ -146,10 +143,10 @@ message Updates {
|
||||
|
||||
| Operation | UpdateMask Fields |
|
||||
|-----------|-------------------|
|
||||
| **Add** (field/function) | `schema` only |
|
||||
| **Add** (field/function) | `schema` + `properties` |
|
||||
| **Drop** (field/function) | `schema` + `properties` |
|
||||
|
||||
Drop requires updating `properties` because it must persist `max_field_id` to prevent field ID reuse.
|
||||
Schema mutations update `properties` because they maintain `max_field_id` to prevent field ID reuse.
|
||||
|
||||
### 3.3 Client SDK (pymilvus)
|
||||
|
||||
@@ -178,33 +175,27 @@ def alter_collection_schema(
|
||||
|
||||
**Files**: `internal/proxy/impl.go`, `internal/proxy/task.go`
|
||||
|
||||
#### Concurrency Control (Inherited from Add-Field)
|
||||
#### Scheduling and Validation
|
||||
|
||||
Drop operations share the same concurrency safeguards as add operations:
|
||||
Drop operations use the standard Proxy DDL path:
|
||||
|
||||
1. **`alterSchemaInFlight`** (`sync.Map`): Ensures only one `AlterCollectionSchema` request per collection at a time. Key is `dbName/collectionName`.
|
||||
1. **Current schema validation**: Proxy calls `DescribeCollection` and passes the current schema into `alterCollectionSchemaTask` for local validation.
|
||||
|
||||
2. **Schema Version Consistency Gate**: Before any schema change, the proxy queries DataCoord for segment schema version statistics:
|
||||
```
|
||||
consistentSegments == totalSegments --> proceed
|
||||
consistentSegments != totalSegments --> reject (previous schema change still propagating)
|
||||
```
|
||||
|
||||
3. **DDL Queue**: Tasks execute serially within the DDL queue, preventing race conditions.
|
||||
2. **DDL Queue**: The task is enqueued on `ddQueue`.
|
||||
|
||||
#### PreExecute: Drop Validation
|
||||
|
||||
```go
|
||||
func (t *alterCollectionSchemaTask) preExecuteDrop(ctx context.Context) error {
|
||||
dropReq := t.req.GetAction().GetOp().(*milvuspb.AlterSchemaAction_DropRequest).DropRequest
|
||||
dropReq := t.GetAction().GetDropRequest()
|
||||
|
||||
switch id := dropReq.GetIdentifier().(type) {
|
||||
case *milvuspb.AlterCollectionSchemaRequest_DropRequest_FunctionName:
|
||||
return validateDropFunction(t.oldSchema, id.FunctionName)
|
||||
case *milvuspb.AlterCollectionSchemaRequest_DropRequest_FieldId:
|
||||
// Resolve field ID to field name, then validate
|
||||
fieldName := resolveFieldName(t.oldSchema, id.FieldId)
|
||||
return validateDropField(t.oldSchema, fieldName)
|
||||
// Resolve field ID across top-level fields and struct array fields.
|
||||
// Struct sub-field drops are rejected.
|
||||
return validateDropFieldByID(t.oldSchema, id.FieldId)
|
||||
case *milvuspb.AlterCollectionSchemaRequest_DropRequest_FieldName:
|
||||
return validateDropField(t.oldSchema, id.FieldName)
|
||||
}
|
||||
@@ -215,16 +206,16 @@ func (t *alterCollectionSchemaTask) preExecuteDrop(ctx context.Context) error {
|
||||
|
||||
| Constraint | Error Message |
|
||||
|-----------|---------------|
|
||||
| Field name is empty | `field name cannot be empty` |
|
||||
| Field name is empty | `field name is empty` |
|
||||
| Field not found in schema | `field not found: {name}` |
|
||||
| System field (`$rowid`, `$timestamp`, `$meta`, `$namespace`) | `cannot drop system field: {name}` |
|
||||
| Primary key field | `cannot drop primary key field: {name}` |
|
||||
| Partition key field | `cannot drop partition key field: {name}` |
|
||||
| Clustering key field | `cannot drop clustering key field: {name}` |
|
||||
| Dynamic field (`IsDynamic=true`) | `cannot drop dynamic field directly, use AlterCollection to disable` |
|
||||
| Dynamic field (`IsDynamic=true`) | `cannot drop dynamic field: {name}, use AlterCollection to disable dynamic schema instead` |
|
||||
| Last vector field in schema | `cannot drop the last vector field: {name}` |
|
||||
| Field is function input | `field is referenced by function {fn} as input` |
|
||||
| Field is function output | `field is referenced by function {fn} as output, drop the function instead` |
|
||||
| Field is function output | `field is referenced by function {fn} as output, drop function first` |
|
||||
| Target is a sub-field of a struct array field | `cannot drop sub-field of struct array field: {struct}.{sub}` |
|
||||
| Dropping whole struct array field would leave no vector | `cannot drop struct array field {name}: it would leave no vector field in the collection` |
|
||||
|
||||
@@ -451,7 +442,7 @@ func broadcastDisableDynamicField(ctx context.Context, coll *model.Collection) e
|
||||
}
|
||||
```
|
||||
|
||||
**Proxy-side gate coverage**: Enabling/disabling the dynamic field mutates the schema and bumps `SchemaVersion + 1`, so it must respect the same concurrency invariants as `AlterCollectionSchema` (see §6.1). The `AlterCollection` handler inspects `request.Properties` and, when `dynamicfield.enabled` is present, routes the request through the same `alterSchemaInFlight` + `checkSchemaVersionConsistency` gates as `AlterCollectionSchema`, sharing the same `sync.Map` so the two handlers are mutually exclusive per collection. Requests that only alter unrelated properties (e.g. `collection.ttl.seconds`, `mmap.enabled`) bypass the gate and keep their existing lightweight path.
|
||||
Requests that alter unrelated properties (e.g., `collection.ttl.seconds`, `mmap.enabled`) keep the existing property-only path.
|
||||
|
||||
### 4.6 Segcore (C++) Layer
|
||||
|
||||
@@ -519,29 +510,29 @@ if err != nil {
|
||||
| **Index cascade** | Drops indexes on the field | Drops indexes on all output fields |
|
||||
| **Validation** | Cannot drop if referenced by a function | No restriction (output fields cascade) |
|
||||
|
||||
### Difference from DropCollectionFunction (Existing API)
|
||||
### Drop Function Cascade Contract
|
||||
|
||||
| Aspect | DropCollectionFunction (existing) | AlterCollectionSchema Drop Function (this feature) |
|
||||
|--------|----------------------------------|-----------------------------------------------------|
|
||||
| Output fields | Preserved, marked `IsFunctionOutput=false` | **Cascade deleted** from schema |
|
||||
| Indexes | Not touched | Cascade deleted |
|
||||
| Field ID protection | No `max_field_id` update | `max_field_id` updated |
|
||||
Dropping a function through `AlterCollectionSchema.DropRequest` applies the same schema-removal contract to the function and its output fields:
|
||||
|
||||
- The target function is removed from `schema.Functions`.
|
||||
- Every output field of the function is removed from `schema.Fields`.
|
||||
- Input fields are preserved.
|
||||
- Indexes on output fields are cascade-deleted through `DroppedFieldIds`.
|
||||
- `max_field_id` remains pinned to the historical high-water mark.
|
||||
|
||||
---
|
||||
|
||||
## 6. Concurrency and Consistency
|
||||
|
||||
### 6.1 Three Layers of Protection
|
||||
### 6.1 Schema Mutation Ordering
|
||||
|
||||
Drop operations inherit the same concurrency model as add operations through the unified `AlterCollectionSchema` entry point:
|
||||
Drop operations use the existing schema DDL ordering:
|
||||
|
||||
1. **`alterSchemaInFlight` Mutex** (`sync.Map`): Only one schema modification per collection at a time. A second request immediately receives an error response.
|
||||
1. **Proxy DDL queue**: `AlterCollectionSchema` tasks are enqueued on `ddQueue`.
|
||||
|
||||
2. **DDL Queue Serial Execution**: All DDL tasks (including schema changes) execute serially within the proxy's DDL queue.
|
||||
2. **Collection broadcast lock**: RootCoord broadcasts the schema mutation under the collection resource key.
|
||||
|
||||
3. **Schema Version Consistency Gate**: The proxy checks DataCoord's segment statistics before allowing any schema change. If a previous change (e.g., add-field backfill) hasn't propagated to all segments, the new request is rejected.
|
||||
|
||||
**Coverage across RPC entry points**: Because the proxy rootcoord lock serializes concurrent *writes* but not *writer-vs-in-flight-backfill*, layers (1) and (3) must guard every RPC entry that mutates the schema. In this PR, both `AlterCollectionSchema` and the dynamic-field branch of `AlterCollection` go through the same two gates and share the same `alterSchemaInFlight` `sync.Map`, so any two schema mutations targeting the same collection — regardless of which RPC they arrived on — are mutually exclusive, and neither can begin until the previous mutation's backfill has reached 100% of segments.
|
||||
3. **Schema-drop readiness**: Field/function drops and dynamic-field disable call `waitUntilSchemaDropReady` before broadcasting the schema change.
|
||||
|
||||
### 6.2 Ack Callback Idempotency
|
||||
|
||||
@@ -591,7 +582,7 @@ Requests that do not reference the dropped field pass through unaffected — pla
|
||||
| 2. Cross-version (narrow) | stale (N) | new schema (N+1), segment reloaded without the field | plan compiles against schema N, reaches querynode; segcore's `chunk_data_impl`/`chunk_array_view_impl` check `field_data_ready_bitset_`, the bit is 0 for the dropped field, `AssertInfo` raises `SegcoreError` — surfaced to the client as an error status |
|
||||
| 3. No race | stale or new | still has the field | executes normally |
|
||||
|
||||
Cases 1 and 2 both return a clear error to the client; **neither aborts or corrupts state**. Case 2 is bounded in time by the proxy cache invalidation latency (rootcoord broadcast → per-proxy cache invalidate, milliseconds to seconds in practice), and further bounded by the schema-version-consistency gate in §6.1: a second schema mutation cannot begin until the previous one has propagated, so the cross-version window is always tied to a single in-flight mutation and never accumulates.
|
||||
Cases 1 and 2 both return a clear error to the client; **neither aborts or corrupts state**. Case 2 is bounded in time by the proxy cache invalidation latency (rootcoord broadcast → per-proxy cache invalidate, milliseconds to seconds in practice).
|
||||
|
||||
SDKs cache collection schema in a process-wide `GlobalCache`. `add_collection_field` and `add_collection_function` previously did not invalidate this cache, relying on `@retry_on_schema_mismatch` on `insert_rows` / `upsert_rows` to recover from staleness on the write path.
|
||||
|
||||
@@ -612,12 +603,6 @@ Client Proxy RootCoord WAL/Streaming QueryNod
|
||||
| (DropRequest: field_name="vec2") | | |
|
||||
|------------------>| | | |
|
||||
| | | | |
|
||||
| | alterSchemaInFlight.LoadOrStore | |
|
||||
| | (check no concurrent schema op) | |
|
||||
| | | | |
|
||||
| | checkSchemaVersionConsistency | |
|
||||
| | (all segments aligned?) | |
|
||||
| | | | |
|
||||
| | validateDropField | |
|
||||
| | (not PK, not last vector, etc.) | |
|
||||
| | | | |
|
||||
@@ -690,7 +675,7 @@ Client Proxy RootCoord WAL/Streaming QueryNod
|
||||
**Decision**: Use `AlterCollectionSchema` with `DropRequest` instead of a dedicated `DropCollectionField` RPC.
|
||||
|
||||
**Rationale**:
|
||||
- Inherits all existing concurrency controls (mutual exclusion, version consistency gate)
|
||||
- Reuses the existing schema evolution entry point
|
||||
- Single API surface for all schema evolution operations
|
||||
- Consistent with the Add/Drop symmetry pattern
|
||||
- Simplifies client SDK and documentation
|
||||
@@ -738,7 +723,7 @@ Client Proxy RootCoord WAL/Streaming QueryNod
|
||||
| 10 | Drop BM25 function | Function + output fields + indexes all removed, input preserved |
|
||||
| 11 | Drop function input field | Rejected (must drop function first) |
|
||||
| 12 | Field ID reuse prevention | Drop + Add same-name different-type, search old data no crash |
|
||||
| 13 | Add + Drop serial interaction | Schema version consistency gate works |
|
||||
| 13 | Add + Drop serial interaction | Schema remains valid after sequential operations |
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -367,6 +367,11 @@ class Schema {
|
||||
return fields_;
|
||||
}
|
||||
|
||||
bool
|
||||
has_field(FieldId field_id) const {
|
||||
return fields_.count(field_id) > 0;
|
||||
}
|
||||
|
||||
const std::unordered_map<FieldId, FieldMeta>
|
||||
get_field_metas(const std::vector<FieldId>& field_ids) {
|
||||
std::unordered_map<FieldId, FieldMeta> field_metas;
|
||||
|
||||
@@ -1000,9 +1000,20 @@ ChunkedSegmentSealedImpl::load_column_group_data_internal(
|
||||
std::vector<FieldId> milvus_field_ids;
|
||||
milvus_field_ids.reserve(field_id_list.size());
|
||||
for (int i = 0; i < field_id_list.size(); ++i) {
|
||||
milvus_field_ids.emplace_back(field_id_list.Get(i));
|
||||
merged_in_load_list = merged_in_load_list ||
|
||||
schema_->ShouldLoadField(milvus_field_ids[i]);
|
||||
auto fid = FieldId(field_id_list.Get(i));
|
||||
// Defense-in-depth for legacy binlog meta (child_field_ids empty):
|
||||
// dropped fields are normally filtered at ComputeDiffBinlogs level,
|
||||
// but legacy data reads field IDs from parquet which may still
|
||||
// contain dropped fields.
|
||||
if (!schema_->has_field(fid)) {
|
||||
continue;
|
||||
}
|
||||
milvus_field_ids.emplace_back(fid);
|
||||
merged_in_load_list =
|
||||
merged_in_load_list || schema_->ShouldLoadField(fid);
|
||||
}
|
||||
if (milvus_field_ids.empty()) {
|
||||
continue;
|
||||
}
|
||||
|
||||
auto mmap_dir_path =
|
||||
|
||||
@@ -779,6 +779,15 @@ SegmentGrowingImpl::load_field_data_internal(const LoadFieldDataInfo& infos) {
|
||||
auto reserved_offset = PreInsert(num_rows);
|
||||
for (auto& [id, info] : infos.field_infos) {
|
||||
auto field_id = FieldId(id);
|
||||
|
||||
// Skip fields that have been dropped from schema (except system fields)
|
||||
if (!SystemProperty::Instance().IsSystem(field_id) &&
|
||||
!schema_->has_field(field_id)) {
|
||||
LOG_INFO("growing segment skips dropped field {} during load",
|
||||
field_id.get());
|
||||
continue;
|
||||
}
|
||||
|
||||
auto insert_files = info.insert_files;
|
||||
storage::SortByPath(insert_files);
|
||||
|
||||
@@ -851,6 +860,14 @@ SegmentGrowingImpl::load_field_data_common(
|
||||
return;
|
||||
}
|
||||
|
||||
// Skip if field has been dropped from schema
|
||||
if (!schema_->has_field(field_id)) {
|
||||
LOG_INFO(
|
||||
"growing segment skips dropped field {} in load_field_data_common",
|
||||
field_id.get());
|
||||
return;
|
||||
}
|
||||
|
||||
auto field_meta = (*schema_)[field_id];
|
||||
|
||||
if (!indexing_record_.HasRawData(field_id)) {
|
||||
@@ -1018,6 +1035,16 @@ SegmentGrowingImpl::load_column_group_data_internal(
|
||||
->metadata()
|
||||
->Get(milvus_storage::ARROW_FIELD_ID_KEY)
|
||||
->data());
|
||||
|
||||
// Skip if field has been dropped from schema
|
||||
if (!schema_->has_field(FieldId(field_id))) {
|
||||
LOG_INFO(
|
||||
"growing segment skips dropped field {} in column "
|
||||
"group",
|
||||
field_id);
|
||||
continue;
|
||||
}
|
||||
|
||||
for (auto& field : schema_->get_fields()) {
|
||||
if (field.second.get_id().get() != field_id) {
|
||||
continue;
|
||||
|
||||
@@ -252,9 +252,15 @@ SegmentLoadInfo::ComputeDiffIndexes(LoadDiff& diff, SegmentLoadInfo& new_info) {
|
||||
|
||||
std::set<int64_t> new_index_ids;
|
||||
// Find indexes to load/replace: indexes in new_info but not in current
|
||||
// Use converted_field_index_cache_ from new_info
|
||||
// Only consider fields that exist in the current schema (skip dropped fields)
|
||||
for (const auto& [field_id, load_index_infos] :
|
||||
new_info.converted_field_index_cache_) {
|
||||
// Schema-driven: skip indexes for fields not in current schema.
|
||||
// Use new_info's schema (the latest) since this->schema_ may still
|
||||
// contain dropped fields from a prior load.
|
||||
if (!new_info.HasFieldInSchema(field_id)) {
|
||||
continue;
|
||||
}
|
||||
for (const auto& load_index_info : load_index_infos) {
|
||||
new_index_ids.insert(load_index_info.index_id);
|
||||
if (current_index_ids.find(load_index_info.index_id) ==
|
||||
@@ -504,9 +510,8 @@ SegmentLoadInfo::ComputeDiffColumnGroups(LoadDiff& diff,
|
||||
// Field at same position — check if needs lazification
|
||||
// (transitioning from no-raw-data-index to raw-data-index)
|
||||
if (!prefer_field_data &&
|
||||
new_info.field_index_has_raw_data_.count(
|
||||
FieldId(field_id)) > 0 &&
|
||||
field_index_has_raw_data_.count(FieldId(field_id)) == 0) {
|
||||
new_info.field_index_has_raw_data_.count(fid) > 0 &&
|
||||
field_index_has_raw_data_.count(fid) == 0) {
|
||||
lazy_replace_fields.emplace_back(field_id);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3284,3 +3284,166 @@ TEST_F(SegmentLoadInfoTest,
|
||||
EXPECT_TRUE(saw_user_group);
|
||||
EXPECT_TRUE(diff.column_groups_to_load.empty());
|
||||
}
|
||||
|
||||
// ==================== Drop Field Tests ====================
|
||||
// These tests verify that ComputeDiff correctly handles fields that exist
|
||||
// in the old schema but have been dropped from the new schema.
|
||||
|
||||
TEST_F(SegmentLoadInfoTest, ComputeDiffDefaultFieldsDroppedField) {
|
||||
// Bug: ComputeDiffDefaultFields iterates schema_->get_fields() (old schema)
|
||||
// which still includes the dropped field. If the field has no data source
|
||||
// in either old or new, it gets added to fields_to_fill_default.
|
||||
//
|
||||
// Setup: old schema has field 110 (extra_field3). New schema drops it.
|
||||
// Old_info has no data for field 110 (never loaded).
|
||||
// Expected: field 110 should NOT appear in fields_to_fill_default.
|
||||
|
||||
auto new_schema = std::make_shared<Schema>();
|
||||
new_schema->AddDebugField("pk", DataType::INT64);
|
||||
new_schema->AddDebugField(
|
||||
"vec", DataType::VECTOR_FLOAT, 128, knowhere::metric::L2);
|
||||
new_schema->AddDebugField("json_field", DataType::JSON);
|
||||
new_schema->AddDebugField("bm25_field", DataType::JSON);
|
||||
new_schema->AddDebugField("group_field", DataType::INT64);
|
||||
new_schema->AddDebugField("child_field1", DataType::FLOAT);
|
||||
new_schema->AddDebugField("child_field2", DataType::FLOAT);
|
||||
new_schema->AddDebugField("child_field3", DataType::FLOAT);
|
||||
new_schema->AddDebugField("extra_field1", DataType::INT64);
|
||||
new_schema->AddDebugField("extra_field2", DataType::FLOAT);
|
||||
// field 110 (extra_field3) intentionally NOT added — it is dropped
|
||||
new_schema->set_primary_field_id(FieldId(100));
|
||||
|
||||
// Current: only has pk binlog, all other fields were default-filled
|
||||
proto::segcore::SegmentLoadInfo current_proto;
|
||||
current_proto.set_segmentid(100);
|
||||
current_proto.set_num_of_rows(1000);
|
||||
auto* binlog = current_proto.add_binlog_paths();
|
||||
binlog->set_fieldid(100);
|
||||
auto* log = binlog->add_binlogs();
|
||||
log->set_log_path("/path/to/pk_binlog");
|
||||
log->set_entries_num(1000);
|
||||
|
||||
// New: same, no data for field 110
|
||||
proto::segcore::SegmentLoadInfo new_proto;
|
||||
new_proto.set_segmentid(100);
|
||||
new_proto.set_num_of_rows(1000);
|
||||
auto* new_binlog = new_proto.add_binlog_paths();
|
||||
new_binlog->set_fieldid(100);
|
||||
auto* new_log = new_binlog->add_binlogs();
|
||||
new_log->set_log_path("/path/to/pk_binlog");
|
||||
new_log->set_entries_num(1000);
|
||||
|
||||
SegmentLoadInfo current_info(current_proto, schema_); // old schema has 110
|
||||
SegmentLoadInfo new_info(new_proto, new_schema); // new schema no 110
|
||||
auto diff = current_info.ComputeDiff(new_info);
|
||||
|
||||
// Field 110 is dropped — it should NOT be in fields_to_fill_default
|
||||
for (const auto& fid : diff.fields_to_fill_default) {
|
||||
EXPECT_NE(fid.get(), 110)
|
||||
<< "Dropped field 110 should not be in fields_to_fill_default";
|
||||
}
|
||||
}
|
||||
|
||||
TEST_F(SegmentLoadInfoTest, ComputeDiffTextIndexesDroppedField) {
|
||||
// Bug: ComputeDiffTextIndexes iterates schema_->get_fields() (old schema)
|
||||
// and checks enable_match(). If the dropped field had enable_match, it
|
||||
// gets added to text_indexes_to_create.
|
||||
//
|
||||
// Setup: old schema has text_field (102) with enable_match=true.
|
||||
// New schema drops field 102.
|
||||
// Expected: field 102 should NOT appear in text_indexes_to_create.
|
||||
|
||||
auto old_schema = CreateSchemaWithTextMatchField();
|
||||
// 100=pk, 101=vec, 102=text_field(enable_match), 103=plain_varchar
|
||||
|
||||
// New schema: drops field 102 (text_field)
|
||||
auto new_schema = std::make_shared<Schema>();
|
||||
new_schema->AddDebugField("pk", DataType::INT64);
|
||||
new_schema->AddDebugField(
|
||||
"vec", DataType::VECTOR_FLOAT, 128, knowhere::metric::L2);
|
||||
// Skip field 102 (dropped)
|
||||
new_schema->AddDebugField("plain_varchar", DataType::VARCHAR);
|
||||
new_schema->set_primary_field_id(FieldId(100));
|
||||
|
||||
proto::segcore::SegmentLoadInfo current_proto;
|
||||
current_proto.set_segmentid(100);
|
||||
current_proto.set_num_of_rows(1000);
|
||||
|
||||
proto::segcore::SegmentLoadInfo new_proto;
|
||||
new_proto.set_segmentid(100);
|
||||
new_proto.set_num_of_rows(1000);
|
||||
|
||||
SegmentLoadInfo current_info(current_proto, old_schema);
|
||||
SegmentLoadInfo new_info(new_proto, new_schema);
|
||||
auto diff = current_info.ComputeDiff(new_info);
|
||||
|
||||
// Field 102 is dropped — it should NOT be in text_indexes_to_create
|
||||
EXPECT_TRUE(diff.text_indexes_to_create.count(FieldId(102)) == 0)
|
||||
<< "Dropped field 102 should not be in text_indexes_to_create";
|
||||
}
|
||||
|
||||
// NOTE: ComputeDiffIndexes also has a schema filter for dropped fields, but
|
||||
// it cannot be unit-tested because BuildCache() requires schema to contain all
|
||||
// fields referenced by index_infos in the proto (it accesses schema[field_id]).
|
||||
// The filter serves as defense-in-depth; Go layer filters dropped-field indexes
|
||||
// before constructing the proto.
|
||||
|
||||
TEST_F(SegmentLoadInfoTest, ComputeDiffBinlogsDroppedField) {
|
||||
// Verify that ComputeDiffBinlogs skips binlogs for dropped fields.
|
||||
//
|
||||
// Setup: new_info has binlog for field 102 (legacy format).
|
||||
// New schema drops field 102.
|
||||
// The binlog should NOT appear in binlogs_to_load.
|
||||
|
||||
auto new_schema = std::make_shared<Schema>();
|
||||
new_schema->AddDebugField("pk", DataType::INT64);
|
||||
new_schema->AddDebugField(
|
||||
"vec", DataType::VECTOR_FLOAT, 128, knowhere::metric::L2);
|
||||
// field 102 (json_field) intentionally NOT added — dropped
|
||||
new_schema->set_primary_field_id(FieldId(100));
|
||||
|
||||
// Current: binlog for field 101 only
|
||||
proto::segcore::SegmentLoadInfo current_proto;
|
||||
current_proto.set_segmentid(100);
|
||||
current_proto.set_num_of_rows(1000);
|
||||
auto* cur_binlog = current_proto.add_binlog_paths();
|
||||
cur_binlog->set_fieldid(101);
|
||||
auto* cur_log = cur_binlog->add_binlogs();
|
||||
cur_log->set_log_path("/path/to/binlog_101");
|
||||
cur_log->set_entries_num(1000);
|
||||
|
||||
// New: binlogs for fields 101 and 102 (legacy format)
|
||||
proto::segcore::SegmentLoadInfo new_proto;
|
||||
new_proto.set_segmentid(100);
|
||||
new_proto.set_num_of_rows(1000);
|
||||
auto* new_binlog1 = new_proto.add_binlog_paths();
|
||||
new_binlog1->set_fieldid(101);
|
||||
auto* new_log1 = new_binlog1->add_binlogs();
|
||||
new_log1->set_log_path("/path/to/binlog_101");
|
||||
new_log1->set_entries_num(1000);
|
||||
auto* new_binlog2 = new_proto.add_binlog_paths();
|
||||
new_binlog2->set_fieldid(102);
|
||||
auto* new_log2 = new_binlog2->add_binlogs();
|
||||
new_log2->set_log_path("/path/to/binlog_102");
|
||||
new_log2->set_entries_num(1000);
|
||||
|
||||
SegmentLoadInfo current_info(current_proto, schema_);
|
||||
SegmentLoadInfo new_info(new_proto, new_schema);
|
||||
auto diff = current_info.ComputeDiff(new_info);
|
||||
|
||||
// Field 102 is dropped — should NOT appear in binlogs_to_load
|
||||
for (const auto& [field_ids, binlog] : diff.binlogs_to_load) {
|
||||
for (const auto& fid : field_ids) {
|
||||
EXPECT_NE(fid.get(), 102)
|
||||
<< "Dropped field 102 should not be in binlogs_to_load";
|
||||
}
|
||||
}
|
||||
// Field 102 should be in field_data_to_drop (was filtered from new)
|
||||
// but since it wasn't in current either, it won't be there.
|
||||
// Just verify no crash and dropped field is excluded from load.
|
||||
}
|
||||
|
||||
// NOTE: ComputeDiffColumnGroups also has a schema filter for dropped fields,
|
||||
// but it cannot be unit-tested because GetColumnGroups() requires Loon FFI
|
||||
// (real manifest file access). The filter uses the same pattern
|
||||
// (new_info.schema_->has_field()) as ComputeDiffBinlogs tested above.
|
||||
|
||||
@@ -465,11 +465,14 @@ GroupChunkTranslator::load_group_chunk(
|
||||
continue;
|
||||
}
|
||||
auto it = field_metas_.find(fid);
|
||||
AssertInfo(
|
||||
it != field_metas_.end(),
|
||||
"[StorageV2] translator {} field id {} not found in field_metas",
|
||||
key_,
|
||||
fid.get());
|
||||
if (it == field_metas_.end()) {
|
||||
// Skip fields not in field_metas (e.g., dropped fields)
|
||||
LOG_INFO(
|
||||
"[StorageV2] translator {} skips field {} not in field_metas",
|
||||
key_,
|
||||
fid.get());
|
||||
continue;
|
||||
}
|
||||
const auto& field_meta = it->second;
|
||||
|
||||
// Merge array vectors from all tables for this field
|
||||
|
||||
@@ -1042,8 +1042,13 @@ func (s *Server) DropIndex(ctx context.Context, req *indexpb.DropIndexRequest) (
|
||||
for _, index := range indexes {
|
||||
field := typeutil.GetField(schema, index.FieldID)
|
||||
if field == nil {
|
||||
log.Warn("field not found", zap.String("indexName", req.IndexName), zap.Int64("collectionID", req.GetCollectionID()), zap.Int64("fieldID", index.FieldID))
|
||||
return merr.Status(merr.WrapErrFieldNotFound(index.FieldID)), nil
|
||||
// Field already dropped from schema (cascade drop from DropCollectionField),
|
||||
// skip validation and proceed with index cleanup
|
||||
log.Info("field already dropped from schema, proceeding with index drop",
|
||||
zap.String("indexName", req.IndexName),
|
||||
zap.Int64("collectionID", req.GetCollectionID()),
|
||||
zap.Int64("fieldID", index.FieldID))
|
||||
continue
|
||||
}
|
||||
if typeutil.IsVectorType(field.GetDataType()) {
|
||||
log.Warn("vector index cannot be dropped on loaded collection", zap.String("indexName", req.IndexName), zap.Int64("collectionID", req.GetCollectionID()), zap.Int64("fieldID", index.FieldID))
|
||||
|
||||
@@ -2544,6 +2544,88 @@ func TestServer_DropIndex(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
func TestServer_DropIndex_DroppedField(t *testing.T) {
|
||||
initStreamingSystem(t)
|
||||
var (
|
||||
collID = UniqueID(1)
|
||||
fieldID = UniqueID(10)
|
||||
indexID = UniqueID(100)
|
||||
indexName = "idx_dropped_field"
|
||||
createTS = uint64(1000)
|
||||
ctx = context.Background()
|
||||
)
|
||||
|
||||
catalog := catalogmocks.NewDataCoordCatalog(t)
|
||||
catalog.On("AlterIndexes", mock.Anything, mock.Anything).Return(nil)
|
||||
|
||||
// Broker returns a schema that does NOT contain fieldID=10 (simulating a dropped field)
|
||||
b := broker.NewMockBroker(t)
|
||||
b.EXPECT().DescribeCollectionInternal(mock.Anything, mock.Anything).Return(&milvuspb.DescribeCollectionResponse{
|
||||
Status: merr.Status(nil),
|
||||
DbName: "test_db",
|
||||
CollectionName: "test_collection",
|
||||
Schema: &schemapb.CollectionSchema{
|
||||
Name: "test_collection",
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: 100, Name: "pk", DataType: schemapb.DataType_Int64, IsPrimaryKey: true},
|
||||
{FieldID: 101, Name: "vec", DataType: schemapb.DataType_FloatVector},
|
||||
// fieldID=10 is intentionally absent — it has been dropped
|
||||
},
|
||||
},
|
||||
}, nil)
|
||||
|
||||
s := &Server{
|
||||
meta: &meta{
|
||||
catalog: catalog,
|
||||
indexMeta: &indexMeta{
|
||||
catalog: catalog,
|
||||
indexes: map[UniqueID]map[UniqueID]*model.Index{
|
||||
collID: {
|
||||
indexID: {
|
||||
CollectionID: collID,
|
||||
FieldID: fieldID, // points to dropped field
|
||||
IndexID: indexID,
|
||||
IndexName: indexName,
|
||||
IsDeleted: false,
|
||||
CreateTime: createTS,
|
||||
TypeParams: []*commonpb.KeyValuePair{
|
||||
{Key: common.DimKey, Value: "128"},
|
||||
},
|
||||
IndexParams: []*commonpb.KeyValuePair{
|
||||
{Key: common.IndexTypeKey, Value: "IVF_FLAT"},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
segmentIndexes: typeutil.NewConcurrentMap[UniqueID, *typeutil.ConcurrentMap[UniqueID, *model.SegmentIndex]](),
|
||||
},
|
||||
segments: NewSegmentsInfo(),
|
||||
},
|
||||
broker: b,
|
||||
allocator: newMockAllocator(t),
|
||||
notifyIndexChan: make(chan UniqueID, 1),
|
||||
}
|
||||
|
||||
// Collection is loaded
|
||||
mixCoord := mocks.NewMixCoord(t)
|
||||
mixCoord.EXPECT().ShowLoadCollections(mock.Anything, mock.Anything).Return(&querypb.ShowCollectionsResponse{
|
||||
Status: merr.Success(),
|
||||
CollectionIDs: []int64{collID},
|
||||
}, nil)
|
||||
s.mixCoord = mixCoord
|
||||
|
||||
RegisterDDLCallbacks(s)
|
||||
s.stateCode.Store(commonpb.StateCode_Healthy)
|
||||
|
||||
// DropIndex should succeed: field not in schema triggers continue, not error
|
||||
resp, err := s.DropIndex(ctx, &indexpb.DropIndexRequest{
|
||||
CollectionID: collID,
|
||||
IndexName: indexName,
|
||||
})
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, commonpb.ErrorCode_Success, resp.GetErrorCode())
|
||||
}
|
||||
|
||||
func TestServer_GetIndexInfos(t *testing.T) {
|
||||
var (
|
||||
collID = UniqueID(1)
|
||||
|
||||
@@ -732,6 +732,11 @@ func (s *Server) AddCollectionStructField(ctx context.Context, request *milvuspb
|
||||
return s.proxy.AddCollectionStructField(ctx, request)
|
||||
}
|
||||
|
||||
// AlterCollectionSchema alters the collection schema (add/drop fields)
|
||||
func (s *Server) AlterCollectionSchema(ctx context.Context, request *milvuspb.AlterCollectionSchemaRequest) (*milvuspb.AlterCollectionSchemaResponse, error) {
|
||||
return s.proxy.AlterCollectionSchema(ctx, request)
|
||||
}
|
||||
|
||||
// GetCollectionStatistics notifies Proxy to get a collection's Statistics
|
||||
func (s *Server) GetCollectionStatistics(ctx context.Context, request *milvuspb.GetCollectionStatisticsRequest) (*milvuspb.GetCollectionStatisticsResponse, error) {
|
||||
return s.proxy.GetCollectionStatistics(ctx, request)
|
||||
@@ -749,10 +754,6 @@ func (s *Server) AlterCollectionField(ctx context.Context, request *milvuspb.Alt
|
||||
return s.proxy.AlterCollectionField(ctx, request)
|
||||
}
|
||||
|
||||
func (s *Server) AlterCollectionSchema(ctx context.Context, request *milvuspb.AlterCollectionSchemaRequest) (*milvuspb.AlterCollectionSchemaResponse, error) {
|
||||
return s.proxy.AlterCollectionSchema(ctx, request)
|
||||
}
|
||||
|
||||
func (s *Server) AddCollectionFunction(ctx context.Context, request *milvuspb.AddCollectionFunctionRequest) (*commonpb.Status, error) {
|
||||
return s.proxy.AddCollectionFunction(ctx, request)
|
||||
}
|
||||
|
||||
@@ -710,6 +710,52 @@ func (_c *MockBalancer_UpdateReplicateConfiguration_Call) RunAndReturn(run func(
|
||||
return _c
|
||||
}
|
||||
|
||||
// WaitUntilSchemaDropReady provides a mock function with given fields: ctx
|
||||
func (_m *MockBalancer) WaitUntilSchemaDropReady(ctx context.Context) error {
|
||||
ret := _m.Called(ctx)
|
||||
|
||||
if len(ret) == 0 {
|
||||
panic("no return value specified for WaitUntilSchemaDropReady")
|
||||
}
|
||||
|
||||
var r0 error
|
||||
if rf, ok := ret.Get(0).(func(context.Context) error); ok {
|
||||
r0 = rf(ctx)
|
||||
} else {
|
||||
r0 = ret.Error(0)
|
||||
}
|
||||
|
||||
return r0
|
||||
}
|
||||
|
||||
// MockBalancer_WaitUntilSchemaDropReady_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'WaitUntilSchemaDropReady'
|
||||
type MockBalancer_WaitUntilSchemaDropReady_Call struct {
|
||||
*mock.Call
|
||||
}
|
||||
|
||||
// WaitUntilSchemaDropReady is a helper method to define mock.On call
|
||||
// - ctx context.Context
|
||||
func (_e *MockBalancer_Expecter) WaitUntilSchemaDropReady(ctx interface{}) *MockBalancer_WaitUntilSchemaDropReady_Call {
|
||||
return &MockBalancer_WaitUntilSchemaDropReady_Call{Call: _e.mock.On("WaitUntilSchemaDropReady", ctx)}
|
||||
}
|
||||
|
||||
func (_c *MockBalancer_WaitUntilSchemaDropReady_Call) Run(run func(ctx context.Context)) *MockBalancer_WaitUntilSchemaDropReady_Call {
|
||||
_c.Call.Run(func(args mock.Arguments) {
|
||||
run(args[0].(context.Context))
|
||||
})
|
||||
return _c
|
||||
}
|
||||
|
||||
func (_c *MockBalancer_WaitUntilSchemaDropReady_Call) Return(_a0 error) *MockBalancer_WaitUntilSchemaDropReady_Call {
|
||||
_c.Call.Return(_a0)
|
||||
return _c
|
||||
}
|
||||
|
||||
func (_c *MockBalancer_WaitUntilSchemaDropReady_Call) RunAndReturn(run func(context.Context) error) *MockBalancer_WaitUntilSchemaDropReady_Call {
|
||||
_c.Call.Return(run)
|
||||
return _c
|
||||
}
|
||||
|
||||
// WaitUntilWALbasedDDLReady provides a mock function with given fields: ctx
|
||||
func (_m *MockBalancer) WaitUntilWALbasedDDLReady(ctx context.Context) error {
|
||||
ret := _m.Called(ctx)
|
||||
@@ -808,7 +854,8 @@ func (_c *MockBalancer_WatchChannelAssignments_Call) RunAndReturn(run func(conte
|
||||
func NewMockBalancer(t interface {
|
||||
mock.TestingT
|
||||
Cleanup(func())
|
||||
}) *MockBalancer {
|
||||
},
|
||||
) *MockBalancer {
|
||||
mock := &MockBalancer{}
|
||||
mock.Mock.Test(t)
|
||||
|
||||
|
||||
@@ -1393,6 +1393,7 @@ func (node *Proxy) AlterCollection(ctx context.Context, request *milvuspb.AlterC
|
||||
|
||||
ctx, sp := otel.Tracer(typeutil.ProxyRole).Start(ctx, "Proxy-AlterCollection")
|
||||
defer sp.End()
|
||||
|
||||
method := "AlterCollection"
|
||||
tr := timerecord.NewTimeRecorder(method)
|
||||
|
||||
|
||||
+213
-11
@@ -1057,10 +1057,19 @@ func (t *alterCollectionSchemaTask) PreExecute(ctx context.Context) error {
|
||||
if action == nil {
|
||||
return merr.WrapErrParameterInvalidMsg("action is nil in alter schema task")
|
||||
}
|
||||
addRequest := action.GetAddRequest()
|
||||
if addRequest == nil {
|
||||
return merr.WrapErrParameterInvalidMsg("add_request is nil, only add operation is supported for now")
|
||||
|
||||
switch action.GetOp().(type) {
|
||||
case *milvuspb.AlterCollectionSchemaRequest_Action_AddRequest:
|
||||
return t.preExecuteAdd(ctx)
|
||||
case *milvuspb.AlterCollectionSchemaRequest_Action_DropRequest:
|
||||
return t.preExecuteDrop(ctx)
|
||||
default:
|
||||
return merr.WrapErrParameterInvalidMsg("unknown action type in alter collection schema request")
|
||||
}
|
||||
}
|
||||
|
||||
func (t *alterCollectionSchemaTask) preExecuteAdd(ctx context.Context) error {
|
||||
addRequest := t.GetAction().GetAddRequest()
|
||||
|
||||
fieldInfos := addRequest.GetFieldInfos()
|
||||
funcSchemas := addRequest.GetFuncSchema()
|
||||
@@ -1127,18 +1136,54 @@ func (t *alterCollectionSchemaTask) PreExecute(ctx context.Context) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
func (t *alterCollectionSchemaTask) Execute(ctx context.Context) error {
|
||||
action := t.GetAction()
|
||||
if action != nil {
|
||||
addRequest := action.GetAddRequest()
|
||||
if addRequest != nil {
|
||||
for _, fieldInfo := range addRequest.GetFieldInfos() {
|
||||
if fieldInfo != nil && fieldInfo.GetFieldSchema() != nil {
|
||||
fieldInfo.GetFieldSchema().IsFunctionOutput = true
|
||||
func (t *alterCollectionSchemaTask) preExecuteDrop(ctx context.Context) error {
|
||||
dropReq := t.GetAction().GetDropRequest()
|
||||
|
||||
switch id := dropReq.GetIdentifier().(type) {
|
||||
case *milvuspb.AlterCollectionSchemaRequest_DropRequest_FunctionName:
|
||||
return validateDropFunction(t.oldSchema, id.FunctionName)
|
||||
|
||||
case *milvuspb.AlterCollectionSchemaRequest_DropRequest_FieldId:
|
||||
for _, f := range t.oldSchema.Fields {
|
||||
if f.FieldID == id.FieldId {
|
||||
return validateDropField(t.oldSchema, f.Name)
|
||||
}
|
||||
}
|
||||
// Struct array field and its sub-fields share the FieldID namespace but
|
||||
// live outside schema.Fields. Route struct-level drops to the normal
|
||||
// by-name path; sub-field drops are not supported in this change.
|
||||
for _, sf := range t.oldSchema.StructArrayFields {
|
||||
if sf.FieldID == id.FieldId {
|
||||
return validateDropField(t.oldSchema, sf.Name)
|
||||
}
|
||||
for _, sub := range sf.Fields {
|
||||
if sub.FieldID == id.FieldId {
|
||||
return merr.WrapErrParameterInvalidMsg(
|
||||
"cannot drop sub-field of struct array field: %s.%s", sf.Name, sub.Name)
|
||||
}
|
||||
}
|
||||
}
|
||||
return merr.WrapErrParameterInvalidMsg("field not found with id: %d", id.FieldId)
|
||||
|
||||
case *milvuspb.AlterCollectionSchemaRequest_DropRequest_FieldName:
|
||||
return validateDropField(t.oldSchema, id.FieldName)
|
||||
|
||||
default:
|
||||
return merr.WrapErrParameterInvalidMsg("drop request must specify field_name, field_id, or function_name")
|
||||
}
|
||||
}
|
||||
|
||||
func (t *alterCollectionSchemaTask) Execute(ctx context.Context) error {
|
||||
// For AddRequest, mark all new fields as function output before sending to RootCoord.
|
||||
// DropRequest needs no extra processing here; RootCoord handles it entirely.
|
||||
if addRequest := t.GetAction().GetAddRequest(); addRequest != nil {
|
||||
for _, fieldInfo := range addRequest.GetFieldInfos() {
|
||||
if fieldInfo != nil && fieldInfo.GetFieldSchema() != nil {
|
||||
fieldInfo.GetFieldSchema().IsFunctionOutput = true
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
var err error
|
||||
t.AlterCollectionSchemaResponse, err = t.mixCoord.AlterCollectionSchema(ctx, t.AlterCollectionSchemaRequest)
|
||||
return merr.CheckRPCCall(t.GetAlterStatus(), err)
|
||||
@@ -1148,6 +1193,163 @@ func (t *alterCollectionSchemaTask) PostExecute(ctx context.Context) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// validateDropField validates that a field can be safely dropped from the schema.
|
||||
func validateDropField(schema *schemapb.CollectionSchema, fieldName string) error {
|
||||
if fieldName == "" {
|
||||
return merr.WrapErrParameterInvalidMsg("field name is empty")
|
||||
}
|
||||
|
||||
// Struct array fields share the name namespace with top-level fields but
|
||||
// live in schema.StructArrayFields. Dropping the whole struct is allowed;
|
||||
// dropping a single sub-field is not (no symmetric add-sub-field support).
|
||||
for _, sf := range schema.StructArrayFields {
|
||||
if sf.Name == fieldName {
|
||||
return validateDropStructArrayField(schema, sf)
|
||||
}
|
||||
for _, sub := range sf.Fields {
|
||||
if sub.Name == fieldName {
|
||||
return merr.WrapErrParameterInvalidMsg(
|
||||
"cannot drop sub-field of struct array field: %s.%s", sf.Name, fieldName)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Find the target field in top-level Fields.
|
||||
var targetField *schemapb.FieldSchema
|
||||
for _, field := range schema.Fields {
|
||||
if field.Name == fieldName {
|
||||
targetField = field
|
||||
break
|
||||
}
|
||||
}
|
||||
if targetField == nil {
|
||||
return merr.WrapErrParameterInvalidMsg("field not found: %s", fieldName)
|
||||
}
|
||||
|
||||
// Check: cannot drop system fields
|
||||
if funcutil.SliceContain([]string{common.RowIDFieldName, common.TimeStampFieldName, common.MetaFieldName, common.NamespaceFieldName}, fieldName) {
|
||||
return merr.WrapErrParameterInvalidMsg("cannot drop system field: %s", fieldName)
|
||||
}
|
||||
|
||||
// Check: cannot drop primary key field
|
||||
if targetField.IsPrimaryKey {
|
||||
return merr.WrapErrParameterInvalidMsg("cannot drop primary key field: %s", fieldName)
|
||||
}
|
||||
|
||||
// Check: cannot drop partition key field
|
||||
if targetField.IsPartitionKey {
|
||||
return merr.WrapErrParameterInvalidMsg("cannot drop partition key field: %s", fieldName)
|
||||
}
|
||||
|
||||
// Check: cannot drop clustering key field
|
||||
if targetField.GetIsClusteringKey() {
|
||||
return merr.WrapErrParameterInvalidMsg("cannot drop clustering key field: %s", fieldName)
|
||||
}
|
||||
|
||||
// Check: cannot drop dynamic field; use AlterCollection to disable dynamic schema instead.
|
||||
if targetField.IsDynamic {
|
||||
return merr.WrapErrParameterInvalidMsg("cannot drop dynamic field: %s, use AlterCollection to disable dynamic schema instead", fieldName)
|
||||
}
|
||||
|
||||
// Check: cannot drop the last vector field. GetVectorFieldSchemas counts
|
||||
// vectors in both top-level Fields and StructArrayField sub-fields.
|
||||
if typeutil.IsVectorType(targetField.DataType) {
|
||||
if len(typeutil.GetVectorFieldSchemas(schema)) <= 1 {
|
||||
return merr.WrapErrParameterInvalidMsg("cannot drop the last vector field: %s", fieldName)
|
||||
}
|
||||
}
|
||||
|
||||
// Check: field must not be referenced by any function
|
||||
for _, fn := range schema.Functions {
|
||||
for _, inputName := range fn.InputFieldNames {
|
||||
if inputName == fieldName {
|
||||
return merr.WrapErrParameterInvalidMsg("field is referenced by function %s as input, drop function first", fn.Name)
|
||||
}
|
||||
}
|
||||
for _, outputName := range fn.OutputFieldNames {
|
||||
if outputName == fieldName {
|
||||
return merr.WrapErrParameterInvalidMsg("field is referenced by function %s as output, drop function first", fn.Name)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func validateDropStructArrayField(schema *schemapb.CollectionSchema, sf *schemapb.StructArrayFieldSchema) error {
|
||||
removedVectors := 0
|
||||
for _, sub := range sf.GetFields() {
|
||||
if sub.GetIsPrimaryKey() {
|
||||
return merr.WrapErrParameterInvalidMsg("cannot drop struct array field %s: sub-field %s is primary key", sf.GetName(), sub.GetName())
|
||||
}
|
||||
if sub.GetIsPartitionKey() {
|
||||
return merr.WrapErrParameterInvalidMsg("cannot drop struct array field %s: sub-field %s is partition key", sf.GetName(), sub.GetName())
|
||||
}
|
||||
if sub.GetIsClusteringKey() {
|
||||
return merr.WrapErrParameterInvalidMsg("cannot drop struct array field %s: sub-field %s is clustering key", sf.GetName(), sub.GetName())
|
||||
}
|
||||
if sub.GetIsDynamic() {
|
||||
return merr.WrapErrParameterInvalidMsg("cannot drop struct array field %s: sub-field %s is dynamic", sf.GetName(), sub.GetName())
|
||||
}
|
||||
for _, fn := range schema.GetFunctions() {
|
||||
for _, inputName := range fn.GetInputFieldNames() {
|
||||
if inputName == sub.GetName() {
|
||||
return merr.WrapErrParameterInvalidMsg("cannot drop struct array field %s: sub-field %s is referenced by function %s as input", sf.GetName(), sub.GetName(), fn.GetName())
|
||||
}
|
||||
}
|
||||
for _, outputName := range fn.GetOutputFieldNames() {
|
||||
if outputName == sub.GetName() {
|
||||
return merr.WrapErrParameterInvalidMsg("cannot drop struct array field %s: sub-field %s is referenced by function %s as output", sf.GetName(), sub.GetName(), fn.GetName())
|
||||
}
|
||||
}
|
||||
}
|
||||
if typeutil.IsVectorType(sub.GetDataType()) {
|
||||
removedVectors++
|
||||
}
|
||||
}
|
||||
if removedVectors >= len(typeutil.GetVectorFieldSchemas(schema)) {
|
||||
return merr.WrapErrParameterInvalidMsg(
|
||||
"cannot drop struct array field %s: it would leave no vector field in the collection",
|
||||
sf.GetName())
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// validateDropFunction checks that the function exists, and that the cascade
|
||||
// removal of its output fields would not leave the collection without any
|
||||
// vector field. Without this check, a user told to "drop function first" by
|
||||
// validateDropField could end up with an unsearchable collection.
|
||||
func validateDropFunction(schema *schemapb.CollectionSchema, functionName string) error {
|
||||
if functionName == "" {
|
||||
return merr.WrapErrParameterInvalidMsg("function name is empty")
|
||||
}
|
||||
|
||||
var targetFunc *schemapb.FunctionSchema
|
||||
for _, fn := range schema.Functions {
|
||||
if fn.Name == functionName {
|
||||
targetFunc = fn
|
||||
break
|
||||
}
|
||||
}
|
||||
if targetFunc == nil {
|
||||
return merr.WrapErrParameterInvalidMsg("function not found: %s", functionName)
|
||||
}
|
||||
|
||||
// Cascade removes the function's output fields; refuse if it removes all vectors.
|
||||
removedVectors := 0
|
||||
for _, name := range targetFunc.OutputFieldNames {
|
||||
if f := typeutil.GetFieldByName(schema, name); f != nil && typeutil.IsVectorType(f.DataType) {
|
||||
removedVectors++
|
||||
}
|
||||
}
|
||||
if removedVectors >= len(typeutil.GetVectorFieldSchemas(schema)) {
|
||||
return merr.WrapErrParameterInvalidMsg(
|
||||
"cannot drop function %s: it would leave no vector field in the collection",
|
||||
functionName)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
type dropCollectionTask struct {
|
||||
baseTask
|
||||
Condition
|
||||
|
||||
@@ -7616,3 +7616,641 @@ func TestAlterCollection_RejectExternalTupleMutation(t *testing.T) {
|
||||
assert.Contains(t, err.Error(), "immutable")
|
||||
})
|
||||
}
|
||||
|
||||
func testDropFieldBaseSchema() *schemapb.CollectionSchema {
|
||||
return &schemapb.CollectionSchema{
|
||||
Name: "test_collection",
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: 100, Name: "id", DataType: schemapb.DataType_Int64, IsPrimaryKey: true},
|
||||
{FieldID: 101, Name: "vector", DataType: schemapb.DataType_FloatVector, TypeParams: []*commonpb.KeyValuePair{{Key: "dim", Value: "128"}}},
|
||||
{FieldID: 102, Name: "scalar_field", DataType: schemapb.DataType_VarChar},
|
||||
{FieldID: 103, Name: "partition_key", DataType: schemapb.DataType_Int64, IsPartitionKey: true},
|
||||
{FieldID: 104, Name: "clustering_key", DataType: schemapb.DataType_Int64, IsClusteringKey: true},
|
||||
{FieldID: 105, Name: "dynamic_field", DataType: schemapb.DataType_JSON, IsDynamic: true},
|
||||
},
|
||||
Functions: []*schemapb.FunctionSchema{
|
||||
{Name: "test_func", InputFieldNames: []string{"scalar_field"}, OutputFieldNames: []string{"vector"}},
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidateDropField(t *testing.T) {
|
||||
baseSchema := testDropFieldBaseSchema()
|
||||
|
||||
t.Run("empty field name", func(t *testing.T) {
|
||||
err := validateDropField(baseSchema, "")
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "field name is empty")
|
||||
})
|
||||
|
||||
t.Run("field not found", func(t *testing.T) {
|
||||
err := validateDropField(baseSchema, "nonexistent_field")
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "field not found")
|
||||
})
|
||||
|
||||
t.Run("cannot drop primary key", func(t *testing.T) {
|
||||
err := validateDropField(baseSchema, "id")
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "cannot drop primary key field")
|
||||
})
|
||||
|
||||
t.Run("cannot drop the last vector field", func(t *testing.T) {
|
||||
err := validateDropField(baseSchema, "vector")
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "cannot drop the last vector field")
|
||||
})
|
||||
|
||||
t.Run("can drop vector field when multiple exist", func(t *testing.T) {
|
||||
multiVecSchema := &schemapb.CollectionSchema{
|
||||
Name: "test_collection",
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: 100, Name: "id", DataType: schemapb.DataType_Int64, IsPrimaryKey: true},
|
||||
{FieldID: 101, Name: "vector1", DataType: schemapb.DataType_FloatVector, TypeParams: []*commonpb.KeyValuePair{{Key: "dim", Value: "128"}}},
|
||||
{FieldID: 102, Name: "vector2", DataType: schemapb.DataType_FloatVector, TypeParams: []*commonpb.KeyValuePair{{Key: "dim", Value: "64"}}},
|
||||
},
|
||||
}
|
||||
err := validateDropField(multiVecSchema, "vector2")
|
||||
assert.NoError(t, err)
|
||||
})
|
||||
|
||||
t.Run("cannot drop partition key", func(t *testing.T) {
|
||||
err := validateDropField(baseSchema, "partition_key")
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "cannot drop partition key field")
|
||||
})
|
||||
|
||||
t.Run("cannot drop clustering key", func(t *testing.T) {
|
||||
err := validateDropField(baseSchema, "clustering_key")
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "cannot drop clustering key field")
|
||||
})
|
||||
|
||||
t.Run("cannot drop dynamic field", func(t *testing.T) {
|
||||
err := validateDropField(baseSchema, "dynamic_field")
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "cannot drop dynamic field")
|
||||
})
|
||||
|
||||
t.Run("field referenced by function - input", func(t *testing.T) {
|
||||
err := validateDropField(baseSchema, "scalar_field")
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "is referenced by function test_func as input")
|
||||
})
|
||||
|
||||
t.Run("field referenced by function - output", func(t *testing.T) {
|
||||
// Need two vector fields so the "last vector field" check passes,
|
||||
// allowing us to reach the output field reference check.
|
||||
schema := &schemapb.CollectionSchema{
|
||||
Name: "test_collection",
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: 100, Name: "id", DataType: schemapb.DataType_Int64, IsPrimaryKey: true},
|
||||
{FieldID: 101, Name: "vec_output", DataType: schemapb.DataType_FloatVector, TypeParams: []*commonpb.KeyValuePair{{Key: "dim", Value: "128"}}},
|
||||
{FieldID: 102, Name: "vec_other", DataType: schemapb.DataType_FloatVector, TypeParams: []*commonpb.KeyValuePair{{Key: "dim", Value: "64"}}},
|
||||
{FieldID: 103, Name: "text", DataType: schemapb.DataType_VarChar},
|
||||
},
|
||||
Functions: []*schemapb.FunctionSchema{
|
||||
{Name: "embed_func", InputFieldNames: []string{"text"}, OutputFieldNames: []string{"vec_output"}},
|
||||
},
|
||||
}
|
||||
err := validateDropField(schema, "vec_output")
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "is referenced by function embed_func as output")
|
||||
})
|
||||
|
||||
t.Run("success - field can be dropped", func(t *testing.T) {
|
||||
schemaNoFunc := &schemapb.CollectionSchema{
|
||||
Name: "test_collection",
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: 100, Name: "id", DataType: schemapb.DataType_Int64, IsPrimaryKey: true},
|
||||
{FieldID: 101, Name: "vector", DataType: schemapb.DataType_FloatVector, TypeParams: []*commonpb.KeyValuePair{{Key: "dim", Value: "128"}}},
|
||||
{FieldID: 102, Name: "droppable_field", DataType: schemapb.DataType_VarChar},
|
||||
},
|
||||
}
|
||||
err := validateDropField(schemaNoFunc, "droppable_field")
|
||||
assert.NoError(t, err)
|
||||
})
|
||||
|
||||
t.Run("can drop vector when struct array field also has vector", func(t *testing.T) {
|
||||
// Top-level vector is the only Fields-level vector, but the struct
|
||||
// array field also contains a vector, so total vector count == 2 and
|
||||
// the drop should succeed.
|
||||
schema := &schemapb.CollectionSchema{
|
||||
Name: "test_collection",
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: 100, Name: "id", DataType: schemapb.DataType_Int64, IsPrimaryKey: true},
|
||||
{FieldID: 101, Name: "vec", DataType: schemapb.DataType_FloatVector, TypeParams: []*commonpb.KeyValuePair{{Key: "dim", Value: "128"}}},
|
||||
},
|
||||
StructArrayFields: []*schemapb.StructArrayFieldSchema{
|
||||
{
|
||||
FieldID: 102, Name: "paragraphs",
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: 103, Name: "para_embed", DataType: schemapb.DataType_ArrayOfVector},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
err := validateDropField(schema, "vec")
|
||||
assert.NoError(t, err)
|
||||
})
|
||||
|
||||
t.Run("drop whole struct array field by name - success", func(t *testing.T) {
|
||||
// Top-level vector remains after dropping the struct, so the drop is allowed.
|
||||
schema := &schemapb.CollectionSchema{
|
||||
Name: "test_collection",
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: 100, Name: "id", DataType: schemapb.DataType_Int64, IsPrimaryKey: true},
|
||||
{FieldID: 101, Name: "vec", DataType: schemapb.DataType_FloatVector, TypeParams: []*commonpb.KeyValuePair{{Key: "dim", Value: "128"}}},
|
||||
},
|
||||
StructArrayFields: []*schemapb.StructArrayFieldSchema{
|
||||
{
|
||||
FieldID: 102, Name: "paragraphs",
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: 103, Name: "para_text", DataType: schemapb.DataType_Array},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
err := validateDropField(schema, "paragraphs")
|
||||
assert.NoError(t, err)
|
||||
})
|
||||
|
||||
t.Run("reject drop struct array field when it holds the only vector", func(t *testing.T) {
|
||||
// The struct contains the only vector in the collection (an ArrayOfVector
|
||||
// sub-field); dropping it would leave no vector field.
|
||||
schema := &schemapb.CollectionSchema{
|
||||
Name: "test_collection",
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: 100, Name: "id", DataType: schemapb.DataType_Int64, IsPrimaryKey: true},
|
||||
},
|
||||
StructArrayFields: []*schemapb.StructArrayFieldSchema{
|
||||
{
|
||||
FieldID: 101, Name: "paragraphs",
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: 102, Name: "para_text", DataType: schemapb.DataType_Array},
|
||||
{FieldID: 103, Name: "para_embed", DataType: schemapb.DataType_ArrayOfVector},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
err := validateDropField(schema, "paragraphs")
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "would leave no vector field")
|
||||
})
|
||||
|
||||
t.Run("reject drop struct array field with protected sub-field", func(t *testing.T) {
|
||||
schema := &schemapb.CollectionSchema{
|
||||
Name: "test_collection",
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: 100, Name: "id", DataType: schemapb.DataType_Int64, IsPrimaryKey: true},
|
||||
{FieldID: 101, Name: "vec", DataType: schemapb.DataType_FloatVector, TypeParams: []*commonpb.KeyValuePair{{Key: "dim", Value: "128"}}},
|
||||
},
|
||||
StructArrayFields: []*schemapb.StructArrayFieldSchema{
|
||||
{
|
||||
FieldID: 102, Name: "paragraphs",
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: 103, Name: "para_key", DataType: schemapb.DataType_Array, IsPartitionKey: true},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
err := validateDropField(schema, "paragraphs")
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "sub-field para_key is partition key")
|
||||
})
|
||||
|
||||
t.Run("reject drop struct array field referenced by function", func(t *testing.T) {
|
||||
schema := &schemapb.CollectionSchema{
|
||||
Name: "test_collection",
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: 100, Name: "id", DataType: schemapb.DataType_Int64, IsPrimaryKey: true},
|
||||
{FieldID: 101, Name: "vec", DataType: schemapb.DataType_FloatVector, TypeParams: []*commonpb.KeyValuePair{{Key: "dim", Value: "128"}}},
|
||||
},
|
||||
StructArrayFields: []*schemapb.StructArrayFieldSchema{
|
||||
{
|
||||
FieldID: 102, Name: "paragraphs",
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: 103, Name: "para_text", DataType: schemapb.DataType_Array},
|
||||
},
|
||||
},
|
||||
},
|
||||
Functions: []*schemapb.FunctionSchema{
|
||||
{Name: "embed_func", InputFieldNames: []string{"para_text"}, OutputFieldNames: []string{"vec"}},
|
||||
},
|
||||
}
|
||||
err := validateDropField(schema, "paragraphs")
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "sub-field para_text is referenced by function embed_func as input")
|
||||
})
|
||||
|
||||
t.Run("reject drop sub-field of struct array field by name", func(t *testing.T) {
|
||||
schema := &schemapb.CollectionSchema{
|
||||
Name: "test_collection",
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: 100, Name: "id", DataType: schemapb.DataType_Int64, IsPrimaryKey: true},
|
||||
{FieldID: 101, Name: "vec", DataType: schemapb.DataType_FloatVector, TypeParams: []*commonpb.KeyValuePair{{Key: "dim", Value: "128"}}},
|
||||
},
|
||||
StructArrayFields: []*schemapb.StructArrayFieldSchema{
|
||||
{
|
||||
FieldID: 102, Name: "paragraphs",
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: 103, Name: "para_text", DataType: schemapb.DataType_Array},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
err := validateDropField(schema, "para_text")
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "cannot drop sub-field of struct array field")
|
||||
assert.Contains(t, err.Error(), "paragraphs.para_text")
|
||||
})
|
||||
}
|
||||
|
||||
func TestAlterCollectionSchemaTask_PreExecute(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
baseSchema := testDropFieldBaseSchema()
|
||||
|
||||
t.Run("nil action", func(t *testing.T) {
|
||||
task := &alterCollectionSchemaTask{
|
||||
ctx: ctx,
|
||||
oldSchema: baseSchema,
|
||||
AlterCollectionSchemaRequest: &milvuspb.AlterCollectionSchemaRequest{
|
||||
CollectionName: "test_collection",
|
||||
Action: nil,
|
||||
},
|
||||
}
|
||||
err := task.PreExecute(ctx)
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "action is nil")
|
||||
})
|
||||
|
||||
t.Run("empty add request fails validation", func(t *testing.T) {
|
||||
task := &alterCollectionSchemaTask{
|
||||
ctx: ctx,
|
||||
oldSchema: baseSchema,
|
||||
AlterCollectionSchemaRequest: &milvuspb.AlterCollectionSchemaRequest{
|
||||
CollectionName: "test_collection",
|
||||
Action: &milvuspb.AlterCollectionSchemaRequest_Action{
|
||||
Op: &milvuspb.AlterCollectionSchemaRequest_Action_AddRequest{},
|
||||
},
|
||||
},
|
||||
}
|
||||
err := task.PreExecute(ctx)
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "exactly one function schema is required")
|
||||
})
|
||||
|
||||
t.Run("drop by field_name - validation error", func(t *testing.T) {
|
||||
task := &alterCollectionSchemaTask{
|
||||
ctx: ctx,
|
||||
oldSchema: baseSchema,
|
||||
AlterCollectionSchemaRequest: &milvuspb.AlterCollectionSchemaRequest{
|
||||
CollectionName: "test_collection",
|
||||
Action: &milvuspb.AlterCollectionSchemaRequest_Action{
|
||||
Op: &milvuspb.AlterCollectionSchemaRequest_Action_DropRequest{
|
||||
DropRequest: &milvuspb.AlterCollectionSchemaRequest_DropRequest{
|
||||
Identifier: &milvuspb.AlterCollectionSchemaRequest_DropRequest_FieldName{
|
||||
FieldName: "id",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
err := task.PreExecute(ctx)
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "cannot drop primary key field")
|
||||
})
|
||||
|
||||
t.Run("drop by field_id - success", func(t *testing.T) {
|
||||
schema := &schemapb.CollectionSchema{
|
||||
Name: "test_collection",
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: 100, Name: "id", DataType: schemapb.DataType_Int64, IsPrimaryKey: true},
|
||||
{FieldID: 101, Name: "vector", DataType: schemapb.DataType_FloatVector, TypeParams: []*commonpb.KeyValuePair{{Key: "dim", Value: "128"}}},
|
||||
{FieldID: 102, Name: "droppable", DataType: schemapb.DataType_VarChar},
|
||||
},
|
||||
}
|
||||
task := &alterCollectionSchemaTask{
|
||||
ctx: ctx,
|
||||
oldSchema: schema,
|
||||
AlterCollectionSchemaRequest: &milvuspb.AlterCollectionSchemaRequest{
|
||||
CollectionName: "test_collection",
|
||||
Action: &milvuspb.AlterCollectionSchemaRequest_Action{
|
||||
Op: &milvuspb.AlterCollectionSchemaRequest_Action_DropRequest{
|
||||
DropRequest: &milvuspb.AlterCollectionSchemaRequest_DropRequest{
|
||||
Identifier: &milvuspb.AlterCollectionSchemaRequest_DropRequest_FieldId{
|
||||
FieldId: 102,
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
err := task.PreExecute(ctx)
|
||||
assert.NoError(t, err)
|
||||
})
|
||||
|
||||
t.Run("drop by field_id - not found", func(t *testing.T) {
|
||||
task := &alterCollectionSchemaTask{
|
||||
ctx: ctx,
|
||||
oldSchema: baseSchema,
|
||||
AlterCollectionSchemaRequest: &milvuspb.AlterCollectionSchemaRequest{
|
||||
CollectionName: "test_collection",
|
||||
Action: &milvuspb.AlterCollectionSchemaRequest_Action{
|
||||
Op: &milvuspb.AlterCollectionSchemaRequest_Action_DropRequest{
|
||||
DropRequest: &milvuspb.AlterCollectionSchemaRequest_DropRequest{
|
||||
Identifier: &milvuspb.AlterCollectionSchemaRequest_DropRequest_FieldId{
|
||||
FieldId: 999,
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
err := task.PreExecute(ctx)
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "field not found with id")
|
||||
})
|
||||
|
||||
t.Run("drop by field_name - success", func(t *testing.T) {
|
||||
schema := &schemapb.CollectionSchema{
|
||||
Name: "test_collection",
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: 100, Name: "id", DataType: schemapb.DataType_Int64, IsPrimaryKey: true},
|
||||
{FieldID: 101, Name: "vector", DataType: schemapb.DataType_FloatVector, TypeParams: []*commonpb.KeyValuePair{{Key: "dim", Value: "128"}}},
|
||||
{FieldID: 102, Name: "droppable", DataType: schemapb.DataType_VarChar},
|
||||
},
|
||||
}
|
||||
task := &alterCollectionSchemaTask{
|
||||
ctx: ctx,
|
||||
oldSchema: schema,
|
||||
AlterCollectionSchemaRequest: &milvuspb.AlterCollectionSchemaRequest{
|
||||
CollectionName: "test_collection",
|
||||
Action: &milvuspb.AlterCollectionSchemaRequest_Action{
|
||||
Op: &milvuspb.AlterCollectionSchemaRequest_Action_DropRequest{
|
||||
DropRequest: &milvuspb.AlterCollectionSchemaRequest_DropRequest{
|
||||
Identifier: &milvuspb.AlterCollectionSchemaRequest_DropRequest_FieldName{
|
||||
FieldName: "droppable",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
err := task.PreExecute(ctx)
|
||||
assert.NoError(t, err)
|
||||
})
|
||||
|
||||
t.Run("drop by function_name - success", func(t *testing.T) {
|
||||
schemaWithFunc := &schemapb.CollectionSchema{
|
||||
Name: "test_collection",
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: 100, Name: "id", DataType: schemapb.DataType_Int64, IsPrimaryKey: true},
|
||||
{FieldID: 101, Name: "vector", DataType: schemapb.DataType_FloatVector, TypeParams: []*commonpb.KeyValuePair{{Key: "dim", Value: "128"}}},
|
||||
{FieldID: 102, Name: "text", DataType: schemapb.DataType_VarChar},
|
||||
},
|
||||
Functions: []*schemapb.FunctionSchema{
|
||||
{Name: "bm25_func", Type: schemapb.FunctionType_BM25},
|
||||
},
|
||||
}
|
||||
task := &alterCollectionSchemaTask{
|
||||
ctx: ctx,
|
||||
oldSchema: schemaWithFunc,
|
||||
AlterCollectionSchemaRequest: &milvuspb.AlterCollectionSchemaRequest{
|
||||
CollectionName: "test_collection",
|
||||
Action: &milvuspb.AlterCollectionSchemaRequest_Action{
|
||||
Op: &milvuspb.AlterCollectionSchemaRequest_Action_DropRequest{
|
||||
DropRequest: &milvuspb.AlterCollectionSchemaRequest_DropRequest{
|
||||
Identifier: &milvuspb.AlterCollectionSchemaRequest_DropRequest_FunctionName{
|
||||
FunctionName: "bm25_func",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
err := task.PreExecute(ctx)
|
||||
assert.NoError(t, err)
|
||||
})
|
||||
|
||||
t.Run("drop by function_name - not found", func(t *testing.T) {
|
||||
task := &alterCollectionSchemaTask{
|
||||
ctx: ctx,
|
||||
oldSchema: baseSchema,
|
||||
AlterCollectionSchemaRequest: &milvuspb.AlterCollectionSchemaRequest{
|
||||
CollectionName: "test_collection",
|
||||
Action: &milvuspb.AlterCollectionSchemaRequest_Action{
|
||||
Op: &milvuspb.AlterCollectionSchemaRequest_Action_DropRequest{
|
||||
DropRequest: &milvuspb.AlterCollectionSchemaRequest_DropRequest{
|
||||
Identifier: &milvuspb.AlterCollectionSchemaRequest_DropRequest_FunctionName{
|
||||
FunctionName: "nonexistent_func",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
err := task.PreExecute(ctx)
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "function not found")
|
||||
})
|
||||
|
||||
t.Run("drop with empty schema", func(t *testing.T) {
|
||||
task := &alterCollectionSchemaTask{
|
||||
ctx: ctx,
|
||||
oldSchema: nil,
|
||||
AlterCollectionSchemaRequest: &milvuspb.AlterCollectionSchemaRequest{
|
||||
CollectionName: "test_collection",
|
||||
Action: &milvuspb.AlterCollectionSchemaRequest_Action{
|
||||
Op: &milvuspb.AlterCollectionSchemaRequest_Action_DropRequest{
|
||||
DropRequest: &milvuspb.AlterCollectionSchemaRequest_DropRequest{
|
||||
Identifier: &milvuspb.AlterCollectionSchemaRequest_DropRequest_FieldName{
|
||||
FieldName: "field",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
err := task.PreExecute(ctx)
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "empty old schema")
|
||||
})
|
||||
|
||||
t.Run("drop by field_id - struct array field success", func(t *testing.T) {
|
||||
schema := &schemapb.CollectionSchema{
|
||||
Name: "test_collection",
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: 100, Name: "id", DataType: schemapb.DataType_Int64, IsPrimaryKey: true},
|
||||
{FieldID: 101, Name: "vec", DataType: schemapb.DataType_FloatVector, TypeParams: []*commonpb.KeyValuePair{{Key: "dim", Value: "128"}}},
|
||||
},
|
||||
StructArrayFields: []*schemapb.StructArrayFieldSchema{
|
||||
{
|
||||
FieldID: 102, Name: "paragraphs",
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: 103, Name: "para_text", DataType: schemapb.DataType_Array},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
task := &alterCollectionSchemaTask{
|
||||
ctx: ctx,
|
||||
oldSchema: schema,
|
||||
AlterCollectionSchemaRequest: &milvuspb.AlterCollectionSchemaRequest{
|
||||
CollectionName: "test_collection",
|
||||
Action: &milvuspb.AlterCollectionSchemaRequest_Action{
|
||||
Op: &milvuspb.AlterCollectionSchemaRequest_Action_DropRequest{
|
||||
DropRequest: &milvuspb.AlterCollectionSchemaRequest_DropRequest{
|
||||
Identifier: &milvuspb.AlterCollectionSchemaRequest_DropRequest_FieldId{
|
||||
FieldId: 102,
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
err := task.PreExecute(ctx)
|
||||
assert.NoError(t, err)
|
||||
})
|
||||
|
||||
t.Run("drop by field_id - reject sub-field of struct array field", func(t *testing.T) {
|
||||
schema := &schemapb.CollectionSchema{
|
||||
Name: "test_collection",
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: 100, Name: "id", DataType: schemapb.DataType_Int64, IsPrimaryKey: true},
|
||||
{FieldID: 101, Name: "vec", DataType: schemapb.DataType_FloatVector, TypeParams: []*commonpb.KeyValuePair{{Key: "dim", Value: "128"}}},
|
||||
},
|
||||
StructArrayFields: []*schemapb.StructArrayFieldSchema{
|
||||
{
|
||||
FieldID: 102, Name: "paragraphs",
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: 103, Name: "para_text", DataType: schemapb.DataType_Array},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
task := &alterCollectionSchemaTask{
|
||||
ctx: ctx,
|
||||
oldSchema: schema,
|
||||
AlterCollectionSchemaRequest: &milvuspb.AlterCollectionSchemaRequest{
|
||||
CollectionName: "test_collection",
|
||||
Action: &milvuspb.AlterCollectionSchemaRequest_Action{
|
||||
Op: &milvuspb.AlterCollectionSchemaRequest_Action_DropRequest{
|
||||
DropRequest: &milvuspb.AlterCollectionSchemaRequest_DropRequest{
|
||||
Identifier: &milvuspb.AlterCollectionSchemaRequest_DropRequest_FieldId{
|
||||
FieldId: 103,
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
err := task.PreExecute(ctx)
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "cannot drop sub-field of struct array field")
|
||||
assert.Contains(t, err.Error(), "paragraphs.para_text")
|
||||
})
|
||||
|
||||
t.Run("unknown action type", func(t *testing.T) {
|
||||
task := &alterCollectionSchemaTask{
|
||||
ctx: ctx,
|
||||
oldSchema: baseSchema,
|
||||
AlterCollectionSchemaRequest: &milvuspb.AlterCollectionSchemaRequest{
|
||||
CollectionName: "test_collection",
|
||||
Action: &milvuspb.AlterCollectionSchemaRequest_Action{},
|
||||
},
|
||||
}
|
||||
err := task.PreExecute(ctx)
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "unknown action type")
|
||||
})
|
||||
}
|
||||
|
||||
func TestValidateDropFunction(t *testing.T) {
|
||||
t.Run("empty function name", func(t *testing.T) {
|
||||
err := validateDropFunction(&schemapb.CollectionSchema{}, "")
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "function name is empty")
|
||||
})
|
||||
|
||||
t.Run("function not found", func(t *testing.T) {
|
||||
schema := &schemapb.CollectionSchema{
|
||||
Functions: []*schemapb.FunctionSchema{
|
||||
{Name: "embedding_func", Type: schemapb.FunctionType_TextEmbedding},
|
||||
},
|
||||
}
|
||||
err := validateDropFunction(schema, "nonexistent")
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "function not found")
|
||||
})
|
||||
|
||||
t.Run("drop function leaves another vector field", func(t *testing.T) {
|
||||
// Cascade removes vec_func_out, but vec_other remains.
|
||||
schema := &schemapb.CollectionSchema{
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: 100, Name: "id", DataType: schemapb.DataType_Int64, IsPrimaryKey: true},
|
||||
{FieldID: 101, Name: "text", DataType: schemapb.DataType_VarChar},
|
||||
{FieldID: 102, Name: "vec_func_out", DataType: schemapb.DataType_FloatVector},
|
||||
{FieldID: 103, Name: "vec_other", DataType: schemapb.DataType_FloatVector},
|
||||
},
|
||||
Functions: []*schemapb.FunctionSchema{
|
||||
{
|
||||
Name: "embed_func", Type: schemapb.FunctionType_TextEmbedding,
|
||||
InputFieldNames: []string{"text"}, OutputFieldNames: []string{"vec_func_out"},
|
||||
},
|
||||
},
|
||||
}
|
||||
err := validateDropFunction(schema, "embed_func")
|
||||
assert.NoError(t, err)
|
||||
})
|
||||
|
||||
t.Run("drop function would leave no vector field", func(t *testing.T) {
|
||||
// BM25-only collection: sparse_vec is the only vector and is the
|
||||
// function's output. Cascade removal would leave 0 vectors.
|
||||
schema := &schemapb.CollectionSchema{
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: 100, Name: "id", DataType: schemapb.DataType_Int64, IsPrimaryKey: true},
|
||||
{FieldID: 101, Name: "text", DataType: schemapb.DataType_VarChar},
|
||||
{FieldID: 102, Name: "sparse_vec", DataType: schemapb.DataType_SparseFloatVector},
|
||||
},
|
||||
Functions: []*schemapb.FunctionSchema{
|
||||
{
|
||||
Name: "bm25_func", Type: schemapb.FunctionType_BM25,
|
||||
InputFieldNames: []string{"text"}, OutputFieldNames: []string{"sparse_vec"},
|
||||
},
|
||||
},
|
||||
}
|
||||
err := validateDropFunction(schema, "bm25_func")
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "would leave no vector field")
|
||||
})
|
||||
|
||||
t.Run("drop function preserves vector in struct array field", func(t *testing.T) {
|
||||
// Cascade removes the function's output vector, but a vector inside a
|
||||
// StructArrayField remains, so the collection is still searchable.
|
||||
schema := &schemapb.CollectionSchema{
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: 100, Name: "id", DataType: schemapb.DataType_Int64, IsPrimaryKey: true},
|
||||
{FieldID: 101, Name: "text", DataType: schemapb.DataType_VarChar},
|
||||
{FieldID: 102, Name: "sparse_vec", DataType: schemapb.DataType_SparseFloatVector},
|
||||
},
|
||||
StructArrayFields: []*schemapb.StructArrayFieldSchema{
|
||||
{
|
||||
FieldID: 103, Name: "paragraphs",
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: 104, Name: "para_embed", DataType: schemapb.DataType_ArrayOfVector},
|
||||
},
|
||||
},
|
||||
},
|
||||
Functions: []*schemapb.FunctionSchema{
|
||||
{
|
||||
Name: "bm25_func", Type: schemapb.FunctionType_BM25,
|
||||
InputFieldNames: []string{"text"}, OutputFieldNames: []string{"sparse_vec"},
|
||||
},
|
||||
},
|
||||
}
|
||||
err := validateDropFunction(schema, "bm25_func")
|
||||
assert.NoError(t, err)
|
||||
})
|
||||
}
|
||||
|
||||
@@ -1997,7 +1997,9 @@ func estimateLoadingResourceUsageOfSegment(schema *schemapb.CollectionSchema, lo
|
||||
if len(fieldIndexInfo.GetIndexFilePaths()) > 0 {
|
||||
fieldSchema, err := schemaHelper.GetFieldFromID(fieldID)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
// field might have been dropped, skip its index
|
||||
log.Info("skip index for dropped field", zap.Int64("fieldID", fieldID), zap.String("name", schema.GetName()))
|
||||
continue
|
||||
}
|
||||
indexedFields[fieldID] = struct{}{}
|
||||
|
||||
@@ -2077,8 +2079,10 @@ func estimateLoadingResourceUsageOfSegment(schema *schemapb.CollectionSchema, lo
|
||||
// get field schema from fieldID
|
||||
fieldSchema, err := schemaHelper.GetFieldFromID(fieldID)
|
||||
if err != nil {
|
||||
log.Warn("failed to get field schema", zap.Int64("fieldID", fieldID), zap.String("name", schema.GetName()), zap.Error(err))
|
||||
return nil, err
|
||||
// field might have been dropped, skip it and continue processing
|
||||
// other fields in the same column group
|
||||
log.Info("skip binlog for dropped field", zap.Int64("fieldID", fieldID), zap.String("name", schema.GetName()))
|
||||
continue
|
||||
}
|
||||
if _, ok := indexedFields[fieldID]; !ok {
|
||||
hasIndex = false
|
||||
|
||||
@@ -1847,6 +1847,59 @@ func (suite *ExternalSegmentEstimateSuite) TestLazyLoadSubtractsRawData() {
|
||||
"lazy load memory (%d) should be <= non-lazy (%d)", lazyUsage.MemorySize, nonLazyUsage.MemorySize)
|
||||
}
|
||||
|
||||
func TestEstimateLoadingResourceUsage_DroppedFieldSkipped(t *testing.T) {
|
||||
paramtable.Init()
|
||||
|
||||
// Schema only has fieldID=100 and 101; fieldID=999 is "dropped" (not in schema)
|
||||
schema := &schemapb.CollectionSchema{
|
||||
Name: "test_dropped_field",
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: 100, Name: "pk", DataType: schemapb.DataType_Int64, IsPrimaryKey: true},
|
||||
{FieldID: 101, Name: "text", DataType: schemapb.DataType_VarChar},
|
||||
},
|
||||
}
|
||||
|
||||
t.Run("index on dropped field is skipped", func(t *testing.T) {
|
||||
loadInfo := &querypb.SegmentLoadInfo{
|
||||
SegmentID: 1,
|
||||
CollectionID: 10,
|
||||
NumOfRows: 100,
|
||||
IndexInfos: []*querypb.FieldIndexInfo{
|
||||
{
|
||||
FieldID: 999, // dropped field
|
||||
IndexID: 2001,
|
||||
IndexFilePaths: []string{"index/999/file1"},
|
||||
},
|
||||
},
|
||||
}
|
||||
factor := resourceEstimateFactor{}
|
||||
usage, err := estimateLoadingResourceUsageOfSegment(schema, loadInfo, factor)
|
||||
assert.NoError(t, err)
|
||||
assert.NotNil(t, usage)
|
||||
})
|
||||
|
||||
t.Run("binlog with dropped field in child fields is skipped", func(t *testing.T) {
|
||||
loadInfo := &querypb.SegmentLoadInfo{
|
||||
SegmentID: 2,
|
||||
CollectionID: 10,
|
||||
NumOfRows: 100,
|
||||
BinlogPaths: []*datapb.FieldBinlog{
|
||||
{
|
||||
FieldID: 888, // column group containing a dropped field
|
||||
Binlogs: []*datapb.Binlog{
|
||||
{LogSize: 1024},
|
||||
},
|
||||
ChildFields: []int64{999}, // dropped field
|
||||
},
|
||||
},
|
||||
}
|
||||
factor := resourceEstimateFactor{}
|
||||
usage, err := estimateLoadingResourceUsageOfSegment(schema, loadInfo, factor)
|
||||
assert.NoError(t, err)
|
||||
assert.NotNil(t, usage)
|
||||
})
|
||||
}
|
||||
|
||||
func TestSegmentLoader(t *testing.T) {
|
||||
suite.Run(t, &SegmentLoaderSuite{})
|
||||
suite.Run(t, &SegmentLoaderDetailSuite{})
|
||||
|
||||
@@ -498,7 +498,7 @@ func (t *createCollectionTask) prepareSchema(ctx context.Context) error {
|
||||
}
|
||||
|
||||
// Set properties for persistent
|
||||
t.body.CollectionSchema.Properties = t.Req.GetProperties()
|
||||
t.body.CollectionSchema.Properties = updateMaxFieldIDProperty(t.Req.GetProperties(), maxAssignedFieldIDFromSchema(t.body.CollectionSchema))
|
||||
t.body.CollectionSchema.Version = 0
|
||||
t.appendSysFields(t.body.CollectionSchema)
|
||||
return nil
|
||||
|
||||
@@ -1749,9 +1749,10 @@ func TestCreateCollectionTask_Prepare_WithProperty(t *testing.T) {
|
||||
}
|
||||
task.Req.ShardsNum = common.DefaultShardsNum
|
||||
err := task.Prepare(context.Background())
|
||||
assert.Len(t, task.body.CollectionSchema.Properties, 2)
|
||||
require.NoError(t, err)
|
||||
assert.Len(t, task.body.CollectionSchema.Properties, 3)
|
||||
assert.Equal(t, "100", common.CloneKeyValuePairs(task.body.CollectionSchema.Properties).ToMap()[common.MaxFieldIDKey])
|
||||
assert.Len(t, task.Req.Properties, 2)
|
||||
assert.NoError(t, err)
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -22,6 +22,7 @@ import (
|
||||
|
||||
"github.com/cockroachdb/errors"
|
||||
|
||||
"github.com/milvus-io/milvus/internal/streamingcoord/server/balancer/balance"
|
||||
"github.com/milvus-io/milvus/internal/streamingcoord/server/broadcaster"
|
||||
"github.com/milvus-io/milvus/internal/streamingcoord/server/broadcaster/broadcast"
|
||||
"github.com/milvus-io/milvus/internal/streamingcoord/server/broadcaster/registry"
|
||||
@@ -171,6 +172,17 @@ func (*Core) startBroadcastWithCollectionLock(ctx context.Context, dbName string
|
||||
return broadcaster, nil
|
||||
}
|
||||
|
||||
func waitUntilSchemaDropReady(ctx context.Context) error {
|
||||
balancer, err := balance.GetWithContext(ctx)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if err := balancer.WaitUntilSchemaDropReady(ctx); err != nil {
|
||||
return errors.Wrap(err, "failed to wait until schema drop ready")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// startBroadcastWithAliasOrCollectionLock starts a broadcast with alias or collection lock.
|
||||
// Some API like AlterCollection can be called with alias or collection name,
|
||||
// so we need to get the real collection name to add resource key lock.
|
||||
|
||||
@@ -68,12 +68,14 @@ func (c *Core) broadcastAlterCollectionForAddField(ctx context.Context, req *mil
|
||||
return merr.WrapErrParameterInvalidMsg("field already exists, name: %s", fieldSchema.Name)
|
||||
}
|
||||
|
||||
// assign a new field id.
|
||||
fieldSchema.FieldID = nextFieldID(coll)
|
||||
// build new collection schema.
|
||||
schema := coll.ToCollectionSchemaPB()
|
||||
// assign a new field id.
|
||||
fieldSchema.FieldID = maxAssignedFieldIDFromSchema(schema) + 1
|
||||
schema.Version = coll.SchemaVersion + 1
|
||||
schema.Fields = append(schema.Fields, fieldSchema)
|
||||
properties := updateMaxFieldIDProperty(coll.Properties, fieldSchema.GetFieldID())
|
||||
schema.Properties = properties
|
||||
if err := typeutil.ValidateExternalCollectionResolvedSchema(schema); err != nil {
|
||||
return err
|
||||
}
|
||||
@@ -92,13 +94,14 @@ func (c *Core) broadcastAlterCollectionForAddField(ctx context.Context, req *mil
|
||||
DbId: coll.DBID,
|
||||
CollectionId: coll.CollectionID,
|
||||
UpdateMask: &fieldmaskpb.FieldMask{
|
||||
Paths: []string{message.FieldMaskCollectionSchema},
|
||||
Paths: []string{message.FieldMaskCollectionSchema, message.FieldMaskCollectionProperties},
|
||||
},
|
||||
CacheExpirations: cacheExpirations,
|
||||
}).
|
||||
WithBody(&messagespb.AlterCollectionMessageBody{
|
||||
Updates: &messagespb.AlterCollectionMessageUpdates{
|
||||
Schema: schema,
|
||||
Schema: schema,
|
||||
Properties: properties,
|
||||
},
|
||||
}).
|
||||
WithBroadcast(channels).
|
||||
|
||||
@@ -67,15 +67,17 @@ func (c *Core) broadcastAlterCollectionForAddStructField(ctx context.Context, re
|
||||
return err
|
||||
}
|
||||
|
||||
fieldIDStart := nextFieldID(coll)
|
||||
schema := coll.ToCollectionSchemaPB()
|
||||
fieldIDStart := maxAssignedFieldIDFromSchema(schema) + 1
|
||||
structArrayField.FieldID = fieldIDStart
|
||||
for i, field := range structArrayField.GetFields() {
|
||||
field.FieldID = fieldIDStart + int64(i) + 1
|
||||
}
|
||||
|
||||
schema := coll.ToCollectionSchemaPB()
|
||||
schema.Version = coll.SchemaVersion + 1
|
||||
schema.StructArrayFields = append(schema.StructArrayFields, structArrayField)
|
||||
properties := updateMaxFieldIDProperty(coll.Properties, maxAssignedFieldIDFromSchema(schema))
|
||||
schema.Properties = properties
|
||||
|
||||
cacheExpirations, err := c.getCacheExpireForCollection(ctx, req.GetDbName(), req.GetCollectionName())
|
||||
if err != nil {
|
||||
@@ -90,13 +92,14 @@ func (c *Core) broadcastAlterCollectionForAddStructField(ctx context.Context, re
|
||||
DbId: coll.DBID,
|
||||
CollectionId: coll.CollectionID,
|
||||
UpdateMask: &fieldmaskpb.FieldMask{
|
||||
Paths: []string{message.FieldMaskCollectionSchema},
|
||||
Paths: []string{message.FieldMaskCollectionSchema, message.FieldMaskCollectionProperties},
|
||||
},
|
||||
CacheExpirations: cacheExpirations,
|
||||
}).
|
||||
WithBody(&messagespb.AlterCollectionMessageBody{
|
||||
Updates: &messagespb.AlterCollectionMessageUpdates{
|
||||
Schema: schema,
|
||||
Schema: schema,
|
||||
Properties: properties,
|
||||
},
|
||||
}).
|
||||
WithBroadcast(channels).
|
||||
|
||||
@@ -75,6 +75,7 @@ func TestDDLCallbacksAlterCollectionAddStructField(t *testing.T) {
|
||||
require.Equal(t, schemapb.DataType_ArrayOfVector, structField.Fields[1].DataType)
|
||||
require.True(t, structField.Fields[1].Nullable)
|
||||
assertSchemaVersion(t, ctx, core, dbName, collectionName, 1)
|
||||
assertMaxFieldIDProperty(t, ctx, core, dbName, collectionName, 103)
|
||||
|
||||
resp, err = core.AddCollectionStructField(ctx, &milvuspb.AddCollectionStructFieldRequest{
|
||||
DbName: dbName,
|
||||
|
||||
@@ -12,9 +12,13 @@ import (
|
||||
"github.com/milvus-io/milvus-proto/go-api/v3/milvuspb"
|
||||
"github.com/milvus-io/milvus-proto/go-api/v3/schemapb"
|
||||
"github.com/milvus-io/milvus/internal/distributed/streaming"
|
||||
"github.com/milvus-io/milvus/internal/metastore/model"
|
||||
"github.com/milvus-io/milvus/internal/streamingcoord/server/broadcaster"
|
||||
"github.com/milvus-io/milvus/internal/streamingcoord/server/broadcaster/registry"
|
||||
"github.com/milvus-io/milvus/internal/util/hookutil"
|
||||
"github.com/milvus-io/milvus/pkg/v3/common"
|
||||
"github.com/milvus-io/milvus/pkg/v3/log"
|
||||
"github.com/milvus-io/milvus/pkg/v3/proto/indexpb"
|
||||
"github.com/milvus-io/milvus/pkg/v3/proto/messagespb"
|
||||
"github.com/milvus-io/milvus/pkg/v3/proto/proxypb"
|
||||
"github.com/milvus-io/milvus/pkg/v3/proto/querypb"
|
||||
@@ -56,7 +60,8 @@ func (c *Core) broadcastAlterCollectionForAlterCollection(ctx context.Context, r
|
||||
|
||||
isEnableDynamicSchema, targetValue, err := common.IsEnableDynamicSchema(req.GetProperties())
|
||||
if err != nil {
|
||||
return merr.WrapErrParameterInvalidMsg("invalid dynamic schema property value: %s", req.GetProperties()[0].GetValue())
|
||||
rawValue, _ := funcutil.TryGetAttrByKeyFromRepeatedKV(common.EnableDynamicSchemaKey, req.GetProperties())
|
||||
return merr.WrapErrParameterInvalidMsg("invalid dynamic schema property value: %s", rawValue)
|
||||
}
|
||||
if isEnableDynamicSchema {
|
||||
// if there's dynamic schema property, it will add a new dynamic field into the collection.
|
||||
@@ -204,25 +209,37 @@ func (c *Core) broadcastAlterCollectionForAlterDynamicField(ctx context.Context,
|
||||
if len(req.GetProperties()) != 1 {
|
||||
return merr.WrapErrParameterInvalidMsg("cannot alter dynamic schema with other properties at the same time")
|
||||
}
|
||||
broadcaster, err := c.startBroadcastWithAliasOrCollectionLock(ctx, req.GetDbName(), req.GetCollectionName())
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
defer broadcaster.Close()
|
||||
|
||||
coll, err := c.meta.GetCollectionByName(ctx, req.GetDbName(), req.GetCollectionName(), typeutil.MaxTimestamp, false)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if coll.EnableDynamicField == targetValue {
|
||||
return errIgnoredAlterCollection
|
||||
}
|
||||
if !targetValue {
|
||||
if err := waitUntilSchemaDropReady(ctx); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
// return nil for no-op
|
||||
broadcaster, err := c.startBroadcastWithCollectionLock(ctx, req.GetDbName(), coll.Name)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
defer broadcaster.Close()
|
||||
|
||||
coll, err = c.meta.GetCollectionByName(ctx, req.GetDbName(), req.GetCollectionName(), typeutil.MaxTimestamp, false)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if coll.EnableDynamicField == targetValue {
|
||||
return errIgnoredAlterCollection
|
||||
}
|
||||
|
||||
// not support disabling since remove field not support yet.
|
||||
// Disable dynamic field: remove $meta field from schema.
|
||||
if !targetValue {
|
||||
return merr.WrapErrParameterInvalidMsg("dynamic schema cannot supported to be disabled")
|
||||
return c.broadcastDisableDynamicField(ctx, req, coll, broadcaster)
|
||||
}
|
||||
|
||||
// convert to add $meta json field, nullable, default value `{}`
|
||||
@@ -241,11 +258,13 @@ func (c *Core) broadcastAlterCollectionForAlterDynamicField(ctx context.Context,
|
||||
return err
|
||||
}
|
||||
|
||||
fieldSchema.FieldID = nextFieldID(coll)
|
||||
schema := coll.ToCollectionSchemaPB()
|
||||
fieldSchema.FieldID = maxAssignedFieldIDFromSchema(schema) + 1
|
||||
schema.Version = coll.SchemaVersion + 1
|
||||
schema.EnableDynamicField = targetValue
|
||||
schema.Fields = append(schema.Fields, fieldSchema)
|
||||
properties := updateMaxFieldIDProperty(coll.Properties, fieldSchema.GetFieldID())
|
||||
schema.Properties = properties
|
||||
|
||||
channels := make([]string, 0, len(coll.VirtualChannelNames)+1)
|
||||
channels = append(channels, streaming.WAL().ControlChannel())
|
||||
@@ -260,13 +279,14 @@ func (c *Core) broadcastAlterCollectionForAlterDynamicField(ctx context.Context,
|
||||
DbId: coll.DBID,
|
||||
CollectionId: coll.CollectionID,
|
||||
UpdateMask: &fieldmaskpb.FieldMask{
|
||||
Paths: []string{message.FieldMaskCollectionSchema},
|
||||
Paths: []string{message.FieldMaskCollectionSchema, message.FieldMaskCollectionProperties},
|
||||
},
|
||||
CacheExpirations: cacheExpirations,
|
||||
}).
|
||||
WithBody(&messagespb.AlterCollectionMessageBody{
|
||||
Updates: &messagespb.AlterCollectionMessageUpdates{
|
||||
Schema: schema,
|
||||
Schema: schema,
|
||||
Properties: properties,
|
||||
},
|
||||
}).
|
||||
WithBroadcast(channels).
|
||||
@@ -277,6 +297,65 @@ func (c *Core) broadcastAlterCollectionForAlterDynamicField(ctx context.Context,
|
||||
return nil
|
||||
}
|
||||
|
||||
// broadcastDisableDynamicField removes the $meta field to disable dynamic schema.
|
||||
func (c *Core) broadcastDisableDynamicField(ctx context.Context, req *milvuspb.AlterCollectionRequest, coll *model.Collection, bc broadcaster.BroadcastAPI) error {
|
||||
// Find and remove $meta field, record its ID for cascade index cleanup.
|
||||
fields := model.MarshalFieldModels(coll.Fields)
|
||||
var dynamicFieldID int64
|
||||
newFields := make([]*schemapb.FieldSchema, 0, len(fields))
|
||||
for _, f := range fields {
|
||||
if f.IsDynamic {
|
||||
dynamicFieldID = f.FieldID
|
||||
} else {
|
||||
newFields = append(newFields, f)
|
||||
}
|
||||
}
|
||||
if dynamicFieldID == 0 {
|
||||
return merr.WrapErrParameterInvalidMsg("dynamic field not found")
|
||||
}
|
||||
|
||||
schema := coll.ToCollectionSchemaPB()
|
||||
maxFieldID := maxAssignedFieldIDFromSchema(schema)
|
||||
properties := updateMaxFieldIDProperty(coll.Properties, maxFieldID)
|
||||
schema.Fields = newFields
|
||||
schema.EnableDynamicField = false
|
||||
schema.Properties = properties
|
||||
schema.Version = coll.SchemaVersion + 1
|
||||
|
||||
channels := make([]string, 0, len(coll.VirtualChannelNames)+1)
|
||||
channels = append(channels, streaming.WAL().ControlChannel())
|
||||
channels = append(channels, coll.VirtualChannelNames...)
|
||||
cacheExpirations, err := c.getCacheExpireForCollection(ctx, req.GetDbName(), req.GetCollectionName())
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
msg := message.NewAlterCollectionMessageBuilderV2().
|
||||
WithHeader(&messagespb.AlterCollectionMessageHeader{
|
||||
DbId: coll.DBID,
|
||||
CollectionId: coll.CollectionID,
|
||||
UpdateMask: &fieldmaskpb.FieldMask{
|
||||
Paths: []string{
|
||||
message.FieldMaskCollectionSchema,
|
||||
message.FieldMaskCollectionProperties,
|
||||
},
|
||||
},
|
||||
CacheExpirations: cacheExpirations,
|
||||
DroppedFieldIds: []int64{dynamicFieldID},
|
||||
}).
|
||||
WithBody(&messagespb.AlterCollectionMessageBody{
|
||||
Updates: &messagespb.AlterCollectionMessageUpdates{
|
||||
Schema: schema,
|
||||
Properties: properties,
|
||||
},
|
||||
}).
|
||||
WithBroadcast(channels).
|
||||
MustBuildBroadcast()
|
||||
if _, err := bc.Broadcast(ctx, msg); err != nil {
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// getCacheExpireForCollection gets the cache expirations for collection.
|
||||
func (c *Core) getCacheExpireForCollection(ctx context.Context, dbName string, collectionNameOrAlias string) (*message.CacheExpirations, error) {
|
||||
coll, err := c.meta.GetCollectionByName(ctx, dbName, collectionNameOrAlias, typeutil.MaxTimestamp, false)
|
||||
@@ -351,6 +430,9 @@ func (c *DDLCallback) alterCollectionV2AckCallback(ctx context.Context, result m
|
||||
return errors.Wrap(err, "failed to update load config")
|
||||
}
|
||||
}
|
||||
if err := c.cascadeDropFieldIndexesInline(ctx, result); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := c.broker.BroadcastAlteredCollection(ctx, header.CollectionId); err != nil {
|
||||
return errors.Wrap(err, "failed to broadcast altered collection")
|
||||
}
|
||||
@@ -371,3 +453,63 @@ func (c *DDLCallback) alterCollectionV2AckCallback(ctx context.Context, result m
|
||||
|
||||
return c.ExpireCaches(ctx, header)
|
||||
}
|
||||
|
||||
// cascadeDropFieldIndexesInline drops indexes on dropped fields by inlining the
|
||||
// DropIndex ack callback, same pattern as dropCollectionV1AckCallback.
|
||||
// Cannot use DropIndex RPC here because it would deadlock on the resource key lock.
|
||||
func (c *DDLCallback) cascadeDropFieldIndexesInline(ctx context.Context, result message.BroadcastResultAlterCollectionMessageV2) error {
|
||||
header := result.Message.Header()
|
||||
droppedFieldIDs := header.GetDroppedFieldIds()
|
||||
if len(droppedFieldIDs) == 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
resp, err := c.mixCoord.DescribeIndex(ctx, &indexpb.DescribeIndexRequest{
|
||||
CollectionID: header.CollectionId,
|
||||
IndexName: "",
|
||||
})
|
||||
if err := merr.CheckRPCCall(resp.GetStatus(), err); err != nil {
|
||||
if merr.ErrIndexNotFound.Is(err) {
|
||||
return nil
|
||||
}
|
||||
return errors.Wrap(err, "failed to describe indexes for cascade drop")
|
||||
}
|
||||
|
||||
droppedFieldSet := make(map[int64]struct{}, len(droppedFieldIDs))
|
||||
for _, fid := range droppedFieldIDs {
|
||||
droppedFieldSet[fid] = struct{}{}
|
||||
}
|
||||
var indexIDs []int64
|
||||
for _, indexInfo := range resp.GetIndexInfos() {
|
||||
if _, ok := droppedFieldSet[indexInfo.GetFieldID()]; ok {
|
||||
log.Ctx(ctx).Info("cascade dropping index on dropped field",
|
||||
log.FieldMessage(result.Message),
|
||||
zap.Int64("fieldID", indexInfo.GetFieldID()),
|
||||
zap.String("indexName", indexInfo.GetIndexName()),
|
||||
zap.Int64("indexID", indexInfo.GetIndexID()),
|
||||
)
|
||||
indexIDs = append(indexIDs, indexInfo.GetIndexID())
|
||||
}
|
||||
}
|
||||
if len(indexIDs) == 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
controlChannelResult := result.GetControlChannelResult()
|
||||
dropIndexMsg := message.NewDropIndexMessageBuilderV2().
|
||||
WithHeader(&message.DropIndexMessageHeader{
|
||||
CollectionId: header.CollectionId,
|
||||
IndexIds: indexIDs,
|
||||
}).
|
||||
WithBody(&message.DropIndexMessageBody{}).
|
||||
WithBroadcast([]string{streaming.WAL().ControlChannel()}).
|
||||
MustBuildBroadcast().
|
||||
WithBroadcastID(result.Message.BroadcastHeader().BroadcastID)
|
||||
|
||||
if err := registry.CallMessageAckCallback(ctx, dropIndexMsg, map[string]*message.AppendResult{
|
||||
streaming.WAL().ControlChannel(): controlChannelResult,
|
||||
}); err != nil {
|
||||
return errors.Wrap(err, "failed to cascade drop field indexes")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -28,7 +28,9 @@ import (
|
||||
"github.com/milvus-io/milvus-proto/go-api/v3/milvuspb"
|
||||
"github.com/milvus-io/milvus-proto/go-api/v3/schemapb"
|
||||
imocks "github.com/milvus-io/milvus/internal/mocks"
|
||||
"github.com/milvus-io/milvus/internal/mocks/streamingcoord/server/mock_balancer"
|
||||
mockrootcoord "github.com/milvus-io/milvus/internal/rootcoord/mocks"
|
||||
"github.com/milvus-io/milvus/internal/streamingcoord/server/balancer/balance"
|
||||
"github.com/milvus-io/milvus/pkg/v3/common"
|
||||
"github.com/milvus-io/milvus/pkg/v3/proto/messagespb"
|
||||
"github.com/milvus-io/milvus/pkg/v3/streaming/util/message"
|
||||
@@ -431,13 +433,77 @@ func TestDDLCallbacksAlterCollectionPropertiesForDynamicField(t *testing.T) {
|
||||
assertDynamicSchema(t, ctx, core, dbName, collectionName, true)
|
||||
assertSchemaVersion(t, ctx, core, dbName, collectionName, 1)
|
||||
|
||||
// disable dynamic schema property should return error.
|
||||
// disable dynamic schema property should succeed.
|
||||
resp, err = core.AlterCollection(ctx, &milvuspb.AlterCollectionRequest{
|
||||
DbName: dbName,
|
||||
CollectionName: collectionName,
|
||||
Properties: []*commonpb.KeyValuePair{{Key: common.EnableDynamicSchemaKey, Value: "false"}},
|
||||
})
|
||||
require.ErrorIs(t, merr.CheckRPCCall(resp, err), merr.ErrParameterInvalid)
|
||||
require.NoError(t, merr.CheckRPCCall(resp, err))
|
||||
assertDynamicSchema(t, ctx, core, dbName, collectionName, false)
|
||||
assertSchemaVersion(t, ctx, core, dbName, collectionName, 2) // drop dynamic field should increment schema version.
|
||||
|
||||
// disable dynamic schema property should be idempotent.
|
||||
resp, err = core.AlterCollection(ctx, &milvuspb.AlterCollectionRequest{
|
||||
DbName: dbName,
|
||||
CollectionName: collectionName,
|
||||
Properties: []*commonpb.KeyValuePair{{Key: common.EnableDynamicSchemaKey, Value: "false"}},
|
||||
})
|
||||
require.NoError(t, merr.CheckRPCCall(resp, err))
|
||||
assertDynamicSchema(t, ctx, core, dbName, collectionName, false)
|
||||
assertSchemaVersion(t, ctx, core, dbName, collectionName, 2)
|
||||
|
||||
// re-enable dynamic schema property should succeed with a new field ID.
|
||||
resp, err = core.AlterCollection(ctx, &milvuspb.AlterCollectionRequest{
|
||||
DbName: dbName,
|
||||
CollectionName: collectionName,
|
||||
Properties: []*commonpb.KeyValuePair{{Key: common.EnableDynamicSchemaKey, Value: "true"}},
|
||||
})
|
||||
require.NoError(t, merr.CheckRPCCall(resp, err))
|
||||
assertDynamicSchema(t, ctx, core, dbName, collectionName, true)
|
||||
assertSchemaVersion(t, ctx, core, dbName, collectionName, 3)
|
||||
// The re-enabled $meta field should have a new FieldID (102, not 101).
|
||||
coll, err := core.meta.GetCollectionByName(ctx, dbName, collectionName, typeutil.MaxTimestamp, false)
|
||||
require.NoError(t, err)
|
||||
dynamicField := coll.Fields[len(coll.Fields)-1]
|
||||
require.True(t, dynamicField.IsDynamic)
|
||||
require.Equal(t, int64(102), dynamicField.FieldID)
|
||||
assertMaxFieldIDProperty(t, ctx, core, dbName, collectionName, 102)
|
||||
}
|
||||
|
||||
func TestDDLCallbacksAlterCollectionPropertiesDisableDynamicFieldWaitsForSchemaDropReady(t *testing.T) {
|
||||
core := initStreamingSystemAndCore(t)
|
||||
ctx := context.Background()
|
||||
dbName := "testDB" + funcutil.RandomString(10)
|
||||
collectionName := "testCollection" + funcutil.RandomString(10)
|
||||
|
||||
createCollectionAndAliasForTest(t, ctx, core, dbName, collectionName)
|
||||
resp, err := core.AlterCollection(ctx, &milvuspb.AlterCollectionRequest{
|
||||
DbName: dbName,
|
||||
CollectionName: collectionName,
|
||||
Properties: []*commonpb.KeyValuePair{{Key: common.EnableDynamicSchemaKey, Value: "true"}},
|
||||
})
|
||||
require.NoError(t, merr.CheckRPCCall(resp, err))
|
||||
assertDynamicSchema(t, ctx, core, dbName, collectionName, true)
|
||||
assertSchemaVersion(t, ctx, core, dbName, collectionName, 1)
|
||||
|
||||
barrierErr := errors.New("proxy version barrier")
|
||||
b := mock_balancer.NewMockBalancer(t)
|
||||
b.EXPECT().WaitUntilWALbasedDDLReady(mock.Anything).Return(nil).Maybe()
|
||||
b.EXPECT().WaitUntilSchemaDropReady(mock.Anything).Return(barrierErr).Once()
|
||||
b.EXPECT().Close().Return().Maybe()
|
||||
balance.ResetBalancer()
|
||||
balance.Register(b)
|
||||
|
||||
resp, err = core.AlterCollection(ctx, &milvuspb.AlterCollectionRequest{
|
||||
DbName: dbName,
|
||||
CollectionName: collectionName,
|
||||
Properties: []*commonpb.KeyValuePair{{Key: common.EnableDynamicSchemaKey, Value: "false"}},
|
||||
})
|
||||
require.Error(t, merr.CheckRPCCall(resp, err))
|
||||
require.Contains(t, resp.GetDetail(), "failed to wait until schema drop ready")
|
||||
assertDynamicSchema(t, ctx, core, dbName, collectionName, true)
|
||||
assertSchemaVersion(t, ctx, core, dbName, collectionName, 1)
|
||||
}
|
||||
|
||||
func TestDDLCallbacksAlterCollectionProperties_TTLFieldShouldBroadcastSchema(t *testing.T) {
|
||||
@@ -791,10 +857,12 @@ func assertDynamicSchema(t *testing.T, ctx context.Context, core *Core, dbName s
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, dynamicSchema, coll.EnableDynamicField)
|
||||
if !dynamicSchema {
|
||||
// Verify no dynamic field exists.
|
||||
for _, field := range coll.Fields {
|
||||
require.False(t, field.IsDynamic, "expected no dynamic field after disabling")
|
||||
}
|
||||
return
|
||||
}
|
||||
require.Len(t, coll.Fields, 4)
|
||||
require.True(t, coll.Fields[len(coll.Fields)-1].IsDynamic)
|
||||
require.Equal(t, coll.Fields[len(coll.Fields)-1].DataType, schemapb.DataType_JSON)
|
||||
require.Equal(t, coll.Fields[len(coll.Fields)-1].FieldID, int64(101))
|
||||
}
|
||||
|
||||
@@ -22,9 +22,12 @@ import (
|
||||
"github.com/cockroachdb/errors"
|
||||
"google.golang.org/protobuf/types/known/fieldmaskpb"
|
||||
|
||||
"github.com/milvus-io/milvus-proto/go-api/v3/commonpb"
|
||||
"github.com/milvus-io/milvus-proto/go-api/v3/milvuspb"
|
||||
"github.com/milvus-io/milvus-proto/go-api/v3/schemapb"
|
||||
"github.com/milvus-io/milvus/internal/distributed/streaming"
|
||||
"github.com/milvus-io/milvus/internal/metastore/model"
|
||||
"github.com/milvus-io/milvus/internal/streamingcoord/server/broadcaster"
|
||||
"github.com/milvus-io/milvus/pkg/v3/proto/messagespb"
|
||||
"github.com/milvus-io/milvus/pkg/v3/streaming/util/message"
|
||||
"github.com/milvus-io/milvus/pkg/v3/util/merr"
|
||||
@@ -33,24 +36,46 @@ import (
|
||||
|
||||
// broadcastAlterCollectionSchema broadcasts the alter collection schema message to all channels.
|
||||
func (c *Core) broadcastAlterCollectionSchema(ctx context.Context, req *milvuspb.AlterCollectionSchemaRequest) error {
|
||||
broadcaster, err := c.startBroadcastWithAliasOrCollectionLock(ctx, req.GetDbName(), req.GetCollectionName())
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
defer broadcaster.Close()
|
||||
coll, err := c.meta.GetCollectionByName(ctx, req.GetDbName(), req.GetCollectionName(), typeutil.MaxTimestamp, false)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// 1. check if the request is valid.
|
||||
action := req.GetAction()
|
||||
if action == nil {
|
||||
return merr.WrapErrParameterInvalidMsg("action is nil")
|
||||
}
|
||||
addRequest := action.GetAddRequest()
|
||||
coll, err := c.meta.GetCollectionByName(ctx, req.GetDbName(), req.GetCollectionName(), typeutil.MaxTimestamp, false)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if _, ok := action.GetOp().(*milvuspb.AlterCollectionSchemaRequest_Action_DropRequest); ok {
|
||||
if err := waitUntilSchemaDropReady(ctx); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
broadcaster, err := c.startBroadcastWithCollectionLock(ctx, req.GetDbName(), coll.Name)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
defer broadcaster.Close()
|
||||
|
||||
coll, err = c.meta.GetCollectionByName(ctx, req.GetDbName(), req.GetCollectionName(), typeutil.MaxTimestamp, false)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
switch action.GetOp().(type) {
|
||||
case *milvuspb.AlterCollectionSchemaRequest_Action_AddRequest:
|
||||
return c.broadcastAlterCollectionSchemaAdd(ctx, broadcaster, coll, req)
|
||||
case *milvuspb.AlterCollectionSchemaRequest_Action_DropRequest:
|
||||
return c.broadcastAlterCollectionSchemaDrop(ctx, broadcaster, coll, req)
|
||||
default:
|
||||
return merr.WrapErrParameterInvalidMsg("unknown action type in alter collection schema request")
|
||||
}
|
||||
}
|
||||
|
||||
// broadcastAlterCollectionSchemaAdd handles AddRequest: adding function fields.
|
||||
func (c *Core) broadcastAlterCollectionSchemaAdd(ctx context.Context, broadcaster broadcaster.BroadcastAPI, coll *model.Collection, req *milvuspb.AlterCollectionSchemaRequest) error {
|
||||
addRequest := req.GetAction().GetAddRequest()
|
||||
if addRequest == nil {
|
||||
return merr.WrapErrParameterInvalidMsg("add_request is nil, only add operation is supported for now")
|
||||
return merr.WrapErrParameterInvalidMsg("add_request is nil")
|
||||
}
|
||||
|
||||
fieldInfos := addRequest.GetFieldInfos()
|
||||
@@ -70,7 +95,7 @@ func (c *Core) broadcastAlterCollectionSchema(ctx context.Context, req *milvuspb
|
||||
return merr.WrapErrParameterInvalidMsg("fieldInfos is empty")
|
||||
}
|
||||
|
||||
// 2. check if the field schemas are illegal.
|
||||
// Check field schemas.
|
||||
fieldSchemas := make([]*schemapb.FieldSchema, 0, len(fieldInfos))
|
||||
for _, fieldInfo := range fieldInfos {
|
||||
fieldSchema := fieldInfo.GetFieldSchema()
|
||||
@@ -92,7 +117,7 @@ func (c *Core) broadcastAlterCollectionSchema(ctx context.Context, req *milvuspb
|
||||
}
|
||||
}
|
||||
|
||||
// 3. check if the fields already exist
|
||||
// Check fields don't already exist.
|
||||
fieldNameSet := make(map[string]struct{})
|
||||
for _, field := range coll.Fields {
|
||||
fieldNameSet[field.Name] = struct{}{}
|
||||
@@ -110,15 +135,18 @@ func (c *Core) broadcastAlterCollectionSchema(ctx context.Context, req *milvuspb
|
||||
}
|
||||
}
|
||||
|
||||
// 4. check if the function already exists
|
||||
// Check function doesn't already exist.
|
||||
for _, function := range coll.Functions {
|
||||
if function.Name == functionSchema.GetName() {
|
||||
return merr.WrapErrParameterInvalidMsg("function already exists, name: %s", functionSchema.GetName())
|
||||
}
|
||||
}
|
||||
|
||||
// 5. assign new field and function ids.
|
||||
fieldIDStart := nextFieldID(coll)
|
||||
// Build new collection schema.
|
||||
schema := coll.ToCollectionSchemaPB()
|
||||
|
||||
// Assign new field and function IDs.
|
||||
fieldIDStart := maxAssignedFieldIDFromSchema(schema) + 1
|
||||
for i, fieldSchema := range fieldSchemas {
|
||||
fieldSchema.FieldID = fieldIDStart + int64(i)
|
||||
}
|
||||
@@ -149,16 +177,16 @@ func (c *Core) broadcastAlterCollectionSchema(ctx context.Context, req *milvuspb
|
||||
functionSchema.OutputFieldIds[idx] = fieldID
|
||||
}
|
||||
|
||||
// 6. build new collection schema.
|
||||
schema := coll.ToCollectionSchemaPB()
|
||||
schema.Version = coll.SchemaVersion + 1
|
||||
schema.Fields = append(schema.Fields, fieldSchemas...)
|
||||
schema.Functions = append(schema.Functions, functionSchema)
|
||||
properties := updateMaxFieldIDProperty(coll.Properties, maxAssignedFieldIDFromSchema(schema))
|
||||
schema.Properties = properties
|
||||
if err := typeutil.ValidateExternalCollectionResolvedSchema(schema); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// 7. get cache expirations.
|
||||
// Broadcast.
|
||||
cacheExpirations, err := c.getCacheExpireForCollection(ctx, req.GetDbName(), req.GetCollectionName())
|
||||
if err != nil {
|
||||
return err
|
||||
@@ -172,13 +200,14 @@ func (c *Core) broadcastAlterCollectionSchema(ctx context.Context, req *milvuspb
|
||||
DbId: coll.DBID,
|
||||
CollectionId: coll.CollectionID,
|
||||
UpdateMask: &fieldmaskpb.FieldMask{
|
||||
Paths: []string{message.FieldMaskCollectionSchema},
|
||||
Paths: []string{message.FieldMaskCollectionSchema, message.FieldMaskCollectionProperties},
|
||||
},
|
||||
CacheExpirations: cacheExpirations,
|
||||
}).
|
||||
WithBody(&messagespb.AlterCollectionMessageBody{
|
||||
Updates: &messagespb.AlterCollectionMessageUpdates{
|
||||
Schema: schema,
|
||||
Schema: schema,
|
||||
Properties: properties,
|
||||
},
|
||||
}).
|
||||
WithBroadcast(channels).
|
||||
@@ -210,3 +239,189 @@ func validateAlterSchemaFunctionInputOutput(functionSchema *schemapb.FunctionSch
|
||||
return merr.WrapErrParameterInvalidMsg("unsupported function type in alter schema task: %s", functionSchema.GetType().String())
|
||||
}
|
||||
}
|
||||
|
||||
// broadcastAlterCollectionSchemaDrop handles DropRequest: dropping fields or functions.
|
||||
func (c *Core) broadcastAlterCollectionSchemaDrop(ctx context.Context, broadcaster broadcaster.BroadcastAPI, coll *model.Collection, req *milvuspb.AlterCollectionSchemaRequest) error {
|
||||
dropReq := req.GetAction().GetDropRequest()
|
||||
if dropReq == nil {
|
||||
return merr.WrapErrParameterInvalidMsg("drop_request is nil")
|
||||
}
|
||||
|
||||
var schema *schemapb.CollectionSchema
|
||||
var properties []*commonpb.KeyValuePair
|
||||
var droppedFieldIds []int64
|
||||
var err error
|
||||
|
||||
switch id := dropReq.GetIdentifier().(type) {
|
||||
case *milvuspb.AlterCollectionSchemaRequest_DropRequest_FunctionName:
|
||||
schema, properties, droppedFieldIds, err = buildSchemaForDropFunction(coll, id.FunctionName)
|
||||
case *milvuspb.AlterCollectionSchemaRequest_DropRequest_FieldName:
|
||||
schema, properties, droppedFieldIds, err = buildSchemaForDropField(coll, id.FieldName, 0)
|
||||
case *milvuspb.AlterCollectionSchemaRequest_DropRequest_FieldId:
|
||||
schema, properties, droppedFieldIds, err = buildSchemaForDropField(coll, "", id.FieldId)
|
||||
default:
|
||||
return merr.WrapErrParameterInvalidMsg("drop request must specify field_name, field_id, or function_name")
|
||||
}
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
cacheExpirations, err := c.getCacheExpireForCollection(ctx, req.GetDbName(), req.GetCollectionName())
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
channels := make([]string, 0, len(coll.VirtualChannelNames)+1)
|
||||
channels = append(channels, streaming.WAL().ControlChannel())
|
||||
channels = append(channels, coll.VirtualChannelNames...)
|
||||
msg := message.NewAlterCollectionMessageBuilderV2().
|
||||
WithHeader(&messagespb.AlterCollectionMessageHeader{
|
||||
DbId: coll.DBID,
|
||||
CollectionId: coll.CollectionID,
|
||||
UpdateMask: &fieldmaskpb.FieldMask{
|
||||
Paths: []string{message.FieldMaskCollectionSchema, message.FieldMaskCollectionProperties},
|
||||
},
|
||||
CacheExpirations: cacheExpirations,
|
||||
DroppedFieldIds: droppedFieldIds,
|
||||
}).
|
||||
WithBody(&messagespb.AlterCollectionMessageBody{
|
||||
Updates: &messagespb.AlterCollectionMessageUpdates{
|
||||
Schema: schema,
|
||||
Properties: properties,
|
||||
},
|
||||
}).
|
||||
WithBroadcast(channels).
|
||||
MustBuildBroadcast()
|
||||
if _, err := broadcaster.Broadcast(ctx, msg); err != nil {
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// buildSchemaForDropField builds the new schema, properties, and droppedFieldIds for dropping a field.
|
||||
// It looks up the target by fieldName or fieldID across top-level Fields and StructArrayFields,
|
||||
// removes it from the schema, and updates max_field_id. Dropping a sub-field of a struct array
|
||||
// field is rejected (no symmetric add-sub-field support).
|
||||
func buildSchemaForDropField(coll *model.Collection, fieldName string, fieldID int64) (
|
||||
schema *schemapb.CollectionSchema,
|
||||
properties []*commonpb.KeyValuePair,
|
||||
droppedFieldIds []int64,
|
||||
err error,
|
||||
) {
|
||||
matchField := func(f *model.Field) bool {
|
||||
if fieldName != "" {
|
||||
return f.Name == fieldName
|
||||
}
|
||||
return fieldID > 0 && f.FieldID == fieldID
|
||||
}
|
||||
matchStruct := func(sf *model.StructArrayField) bool {
|
||||
if fieldName != "" {
|
||||
return sf.Name == fieldName
|
||||
}
|
||||
return fieldID > 0 && sf.FieldID == fieldID
|
||||
}
|
||||
|
||||
// Top-level field path: remove from fields.
|
||||
var droppedField *model.Field
|
||||
newFields := make([]*schemapb.FieldSchema, 0, len(coll.Fields))
|
||||
for _, f := range coll.Fields {
|
||||
if droppedField == nil && matchField(f) {
|
||||
droppedField = f
|
||||
continue
|
||||
}
|
||||
newFields = append(newFields, model.MarshalFieldModel(f))
|
||||
}
|
||||
if droppedField != nil {
|
||||
schema = coll.ToCollectionSchemaPB()
|
||||
maxFieldID := maxAssignedFieldIDFromSchema(schema)
|
||||
properties = updateMaxFieldIDProperty(coll.Properties, maxFieldID)
|
||||
schema.Fields = newFields
|
||||
schema.Properties = properties
|
||||
schema.Version = coll.SchemaVersion + 1
|
||||
return schema, properties, []int64{droppedField.FieldID}, nil
|
||||
}
|
||||
|
||||
// Struct array field path: remove the whole entry from StructArrayFields.
|
||||
// droppedFieldIds includes the struct ID plus every sub-field ID so that
|
||||
// index cascade (matched by FieldID) and segcore filtering (schema.has_field)
|
||||
// naturally cover every column that physically goes away.
|
||||
// Sub-field drops are already rejected at the proxy layer; if one reaches
|
||||
// here we fall through to the generic "field not found" tail.
|
||||
var droppedStruct *model.StructArrayField
|
||||
newStructs := make([]*schemapb.StructArrayFieldSchema, 0, len(coll.StructArrayFields))
|
||||
for _, s := range coll.StructArrayFields {
|
||||
if droppedStruct == nil && matchStruct(s) {
|
||||
droppedStruct = s
|
||||
continue
|
||||
}
|
||||
newStructs = append(newStructs, model.MarshalStructArrayFieldModel(s))
|
||||
}
|
||||
if droppedStruct != nil {
|
||||
schema = coll.ToCollectionSchemaPB()
|
||||
maxFieldID := maxAssignedFieldIDFromSchema(schema)
|
||||
properties = updateMaxFieldIDProperty(coll.Properties, maxFieldID)
|
||||
schema.StructArrayFields = newStructs
|
||||
schema.Properties = properties
|
||||
schema.Version = coll.SchemaVersion + 1
|
||||
droppedFieldIds = append(droppedFieldIds, droppedStruct.FieldID)
|
||||
for _, subField := range droppedStruct.Fields {
|
||||
droppedFieldIds = append(droppedFieldIds, subField.FieldID)
|
||||
}
|
||||
return schema, properties, droppedFieldIds, nil
|
||||
}
|
||||
|
||||
if fieldName != "" {
|
||||
return nil, nil, nil, merr.WrapErrParameterInvalidMsg("field not found: %s", fieldName)
|
||||
}
|
||||
return nil, nil, nil, merr.WrapErrParameterInvalidMsg("field not found with id: %d", fieldID)
|
||||
}
|
||||
|
||||
// buildSchemaForDropFunction builds the new schema for dropping a function and its output fields.
|
||||
// It removes the function from Functions, removes all output fields from Fields, and updates max_field_id.
|
||||
func buildSchemaForDropFunction(coll *model.Collection, functionName string) (
|
||||
schema *schemapb.CollectionSchema,
|
||||
properties []*commonpb.KeyValuePair,
|
||||
droppedFieldIds []int64,
|
||||
err error,
|
||||
) {
|
||||
var targetFunc *model.Function
|
||||
for _, fn := range coll.Functions {
|
||||
if fn.Name == functionName {
|
||||
targetFunc = fn
|
||||
break
|
||||
}
|
||||
}
|
||||
if targetFunc == nil {
|
||||
return nil, nil, nil, merr.WrapErrParameterInvalidMsg("function not found: %s", functionName)
|
||||
}
|
||||
|
||||
droppedFieldIds = append(droppedFieldIds, targetFunc.OutputFieldIDs...)
|
||||
outputFieldIDSet := make(map[int64]struct{}, len(targetFunc.OutputFieldIDs))
|
||||
for _, fid := range targetFunc.OutputFieldIDs {
|
||||
outputFieldIDSet[fid] = struct{}{}
|
||||
}
|
||||
|
||||
newFields := make([]*schemapb.FieldSchema, 0, len(coll.Fields))
|
||||
for _, field := range coll.Fields {
|
||||
if _, ok := outputFieldIDSet[field.FieldID]; !ok {
|
||||
newFields = append(newFields, model.MarshalFieldModel(field))
|
||||
}
|
||||
}
|
||||
|
||||
newFunctions := make([]*schemapb.FunctionSchema, 0, len(coll.Functions)-1)
|
||||
for _, fn := range coll.Functions {
|
||||
if fn.Name != functionName {
|
||||
newFunctions = append(newFunctions, model.MarshalFunctionModel(fn))
|
||||
}
|
||||
}
|
||||
|
||||
schema = coll.ToCollectionSchemaPB()
|
||||
maxFieldID := maxAssignedFieldIDFromSchema(schema)
|
||||
properties = updateMaxFieldIDProperty(coll.Properties, maxFieldID)
|
||||
schema.Fields = newFields
|
||||
schema.Functions = newFunctions
|
||||
schema.Properties = properties
|
||||
schema.Version = coll.SchemaVersion + 1
|
||||
|
||||
return schema, properties, droppedFieldIds, nil
|
||||
}
|
||||
|
||||
@@ -18,15 +18,26 @@ package rootcoord
|
||||
|
||||
import (
|
||||
"context"
|
||||
"strconv"
|
||||
"testing"
|
||||
|
||||
"github.com/cockroachdb/errors"
|
||||
"github.com/stretchr/testify/mock"
|
||||
"github.com/stretchr/testify/require"
|
||||
"google.golang.org/protobuf/proto"
|
||||
"google.golang.org/protobuf/types/known/fieldmaskpb"
|
||||
|
||||
"github.com/milvus-io/milvus-proto/go-api/v3/commonpb"
|
||||
"github.com/milvus-io/milvus-proto/go-api/v3/milvuspb"
|
||||
"github.com/milvus-io/milvus-proto/go-api/v3/schemapb"
|
||||
"github.com/milvus-io/milvus/internal/metastore/model"
|
||||
imocks "github.com/milvus-io/milvus/internal/mocks"
|
||||
"github.com/milvus-io/milvus/internal/mocks/streamingcoord/server/mock_balancer"
|
||||
"github.com/milvus-io/milvus/internal/streamingcoord/server/balancer/balance"
|
||||
"github.com/milvus-io/milvus/pkg/v3/common"
|
||||
"github.com/milvus-io/milvus/pkg/v3/proto/indexpb"
|
||||
"github.com/milvus-io/milvus/pkg/v3/proto/messagespb"
|
||||
"github.com/milvus-io/milvus/pkg/v3/streaming/util/message"
|
||||
"github.com/milvus-io/milvus/pkg/v3/util/funcutil"
|
||||
"github.com/milvus-io/milvus/pkg/v3/util/merr"
|
||||
"github.com/milvus-io/milvus/pkg/v3/util/typeutil"
|
||||
@@ -161,7 +172,7 @@ func TestDDLCallbacksBroadcastAlterCollectionSchema(t *testing.T) {
|
||||
})
|
||||
require.ErrorIs(t, merr.CheckRPCCall(resp.GetAlterStatus(), err), merr.ErrParameterInvalid)
|
||||
|
||||
// case 6: output field already exists (field1 created by createCollectionForTest)
|
||||
// case 7: output field already exists (field1 created by createCollectionForTest)
|
||||
resp, err = core.AlterCollectionSchema(ctx, &milvuspb.AlterCollectionSchemaRequest{
|
||||
DbName: dbName,
|
||||
CollectionName: collectionName,
|
||||
@@ -180,7 +191,7 @@ func TestDDLCallbacksBroadcastAlterCollectionSchema(t *testing.T) {
|
||||
})
|
||||
require.ErrorIs(t, merr.CheckRPCCall(resp.GetAlterStatus(), err), merr.ErrParameterInvalid)
|
||||
|
||||
// case 7: output field points to an existing field while FieldInfos adds a different field
|
||||
// case 8: output field points to an existing field while FieldInfos adds a different field
|
||||
resp, err = core.AlterCollectionSchema(ctx, &milvuspb.AlterCollectionSchemaRequest{
|
||||
DbName: dbName,
|
||||
CollectionName: collectionName,
|
||||
@@ -249,7 +260,7 @@ func TestDDLCallbacksBroadcastAlterCollectionSchema(t *testing.T) {
|
||||
require.False(t, schema.GetDoPhysicalBackfill())
|
||||
require.EqualValues(t, 3, schema.GetVersion())
|
||||
|
||||
// case 7: function already exists (same name "bm25_fn")
|
||||
// case 9: function already exists (same name "bm25_fn")
|
||||
resp, err = core.AlterCollectionSchema(ctx, &milvuspb.AlterCollectionSchemaRequest{
|
||||
DbName: dbName,
|
||||
CollectionName: collectionName,
|
||||
@@ -268,7 +279,7 @@ func TestDDLCallbacksBroadcastAlterCollectionSchema(t *testing.T) {
|
||||
})
|
||||
require.ErrorIs(t, merr.CheckRPCCall(resp.GetAlterStatus(), err), merr.ErrParameterInvalid)
|
||||
|
||||
// case 8: input field not found
|
||||
// case 10: input field not found
|
||||
resp, err = core.AlterCollectionSchema(ctx, &milvuspb.AlterCollectionSchemaRequest{
|
||||
DbName: dbName,
|
||||
CollectionName: collectionName,
|
||||
@@ -287,7 +298,7 @@ func TestDDLCallbacksBroadcastAlterCollectionSchema(t *testing.T) {
|
||||
})
|
||||
require.Error(t, merr.CheckRPCCall(resp.GetAlterStatus(), err))
|
||||
|
||||
// case 9: output field not found (function references "ghost_output" but FieldInfos has "sparse_output4")
|
||||
// case 11: output field not found (function references "ghost_output" but FieldInfos has "sparse_output4")
|
||||
resp, err = core.AlterCollectionSchema(ctx, &milvuspb.AlterCollectionSchemaRequest{
|
||||
DbName: dbName,
|
||||
CollectionName: collectionName,
|
||||
@@ -306,3 +317,521 @@ func TestDDLCallbacksBroadcastAlterCollectionSchema(t *testing.T) {
|
||||
})
|
||||
require.Error(t, merr.CheckRPCCall(resp.GetAlterStatus(), err))
|
||||
}
|
||||
|
||||
func TestDDLCallbacksAlterCollectionDropField(t *testing.T) {
|
||||
core := initStreamingSystemAndCore(t)
|
||||
|
||||
ctx := context.Background()
|
||||
dbName := "testDB" + funcutil.RandomString(10)
|
||||
collectionName := "testCollection" + funcutil.RandomString(10)
|
||||
|
||||
// database not found
|
||||
resp, err := core.AlterCollectionSchema(ctx, &milvuspb.AlterCollectionSchemaRequest{
|
||||
DbName: dbName,
|
||||
CollectionName: collectionName,
|
||||
Action: &milvuspb.AlterCollectionSchemaRequest_Action{
|
||||
Op: &milvuspb.AlterCollectionSchemaRequest_Action_DropRequest{
|
||||
DropRequest: &milvuspb.AlterCollectionSchemaRequest_DropRequest{
|
||||
Identifier: &milvuspb.AlterCollectionSchemaRequest_DropRequest_FieldName{FieldName: "field1"},
|
||||
},
|
||||
},
|
||||
},
|
||||
})
|
||||
require.ErrorIs(t, merr.CheckRPCCall(resp.GetAlterStatus(), err), merr.ErrDatabaseNotFound)
|
||||
|
||||
// create collection with field1
|
||||
createCollectionForTest(t, ctx, core, dbName, collectionName)
|
||||
|
||||
// add field2 and field3
|
||||
addResp, err := core.AddCollectionField(ctx, &milvuspb.AddCollectionFieldRequest{
|
||||
DbName: dbName,
|
||||
CollectionName: collectionName,
|
||||
Schema: getFieldSchema("field2"),
|
||||
})
|
||||
require.NoError(t, merr.CheckRPCCall(addResp, err))
|
||||
assertFieldExists(t, ctx, core, dbName, collectionName, "field2", 101)
|
||||
|
||||
addResp, err = core.AddCollectionField(ctx, &milvuspb.AddCollectionFieldRequest{
|
||||
DbName: dbName,
|
||||
CollectionName: collectionName,
|
||||
Schema: getFieldSchema("field3"),
|
||||
})
|
||||
require.NoError(t, merr.CheckRPCCall(addResp, err))
|
||||
assertFieldExists(t, ctx, core, dbName, collectionName, "field3", 102)
|
||||
assertSchemaVersion(t, ctx, core, dbName, collectionName, 2)
|
||||
|
||||
// helper to build drop-field request
|
||||
dropFieldReq := func(fieldName string) *milvuspb.AlterCollectionSchemaRequest {
|
||||
return &milvuspb.AlterCollectionSchemaRequest{
|
||||
DbName: dbName,
|
||||
CollectionName: collectionName,
|
||||
Action: &milvuspb.AlterCollectionSchemaRequest_Action{
|
||||
Op: &milvuspb.AlterCollectionSchemaRequest_Action_DropRequest{
|
||||
DropRequest: &milvuspb.AlterCollectionSchemaRequest_DropRequest{
|
||||
Identifier: &milvuspb.AlterCollectionSchemaRequest_DropRequest_FieldName{FieldName: fieldName},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
// field not found
|
||||
resp, err = core.AlterCollectionSchema(ctx, dropFieldReq("nonexistent"))
|
||||
require.ErrorIs(t, merr.CheckRPCCall(resp.GetAlterStatus(), err), merr.ErrParameterInvalid)
|
||||
|
||||
// drop field2 successfully
|
||||
resp, err = core.AlterCollectionSchema(ctx, dropFieldReq("field2"))
|
||||
require.NoError(t, merr.CheckRPCCall(resp.GetAlterStatus(), err))
|
||||
assertFieldNotExists(t, ctx, core, dbName, collectionName, "field2")
|
||||
assertFieldExists(t, ctx, core, dbName, collectionName, "field1", 100)
|
||||
assertFieldExists(t, ctx, core, dbName, collectionName, "field3", 102)
|
||||
assertSchemaVersion(t, ctx, core, dbName, collectionName, 3)
|
||||
assertMaxFieldIDProperty(t, ctx, core, dbName, collectionName, 102)
|
||||
|
||||
// add field4 after drop: fieldID should not reuse 101 (the dropped field2's ID)
|
||||
addResp, err = core.AddCollectionField(ctx, &milvuspb.AddCollectionFieldRequest{
|
||||
DbName: dbName,
|
||||
CollectionName: collectionName,
|
||||
Schema: getFieldSchema("field4"),
|
||||
})
|
||||
require.NoError(t, merr.CheckRPCCall(addResp, err))
|
||||
assertFieldExists(t, ctx, core, dbName, collectionName, "field4", 103)
|
||||
assertSchemaVersion(t, ctx, core, dbName, collectionName, 4)
|
||||
assertMaxFieldIDProperty(t, ctx, core, dbName, collectionName, 103)
|
||||
|
||||
// drop field3 successfully
|
||||
resp, err = core.AlterCollectionSchema(ctx, dropFieldReq("field3"))
|
||||
require.NoError(t, merr.CheckRPCCall(resp.GetAlterStatus(), err))
|
||||
assertFieldNotExists(t, ctx, core, dbName, collectionName, "field3")
|
||||
assertFieldExists(t, ctx, core, dbName, collectionName, "field1", 100)
|
||||
assertFieldExists(t, ctx, core, dbName, collectionName, "field4", 103)
|
||||
assertSchemaVersion(t, ctx, core, dbName, collectionName, 5)
|
||||
assertMaxFieldIDProperty(t, ctx, core, dbName, collectionName, 103)
|
||||
}
|
||||
|
||||
func TestDDLCallbacksAlterCollectionDropFieldWaitsForSchemaDropReady(t *testing.T) {
|
||||
core := initStreamingSystemAndCore(t)
|
||||
|
||||
ctx := context.Background()
|
||||
dbName := "testDB" + funcutil.RandomString(10)
|
||||
collectionName := "testCollection" + funcutil.RandomString(10)
|
||||
|
||||
createCollectionForTest(t, ctx, core, dbName, collectionName)
|
||||
addResp, err := core.AddCollectionField(ctx, &milvuspb.AddCollectionFieldRequest{
|
||||
DbName: dbName,
|
||||
CollectionName: collectionName,
|
||||
Schema: getFieldSchema("field2"),
|
||||
})
|
||||
require.NoError(t, merr.CheckRPCCall(addResp, err))
|
||||
assertFieldExists(t, ctx, core, dbName, collectionName, "field2", 101)
|
||||
assertSchemaVersion(t, ctx, core, dbName, collectionName, 1)
|
||||
|
||||
barrierErr := errors.New("proxy version barrier")
|
||||
b := mock_balancer.NewMockBalancer(t)
|
||||
b.EXPECT().WaitUntilWALbasedDDLReady(mock.Anything).Return(nil).Maybe()
|
||||
b.EXPECT().WaitUntilSchemaDropReady(mock.Anything).Return(barrierErr).Once()
|
||||
b.EXPECT().Close().Return().Maybe()
|
||||
balance.ResetBalancer()
|
||||
balance.Register(b)
|
||||
|
||||
resp, err := core.AlterCollectionSchema(ctx, &milvuspb.AlterCollectionSchemaRequest{
|
||||
DbName: dbName,
|
||||
CollectionName: collectionName,
|
||||
Action: &milvuspb.AlterCollectionSchemaRequest_Action{
|
||||
Op: &milvuspb.AlterCollectionSchemaRequest_Action_DropRequest{
|
||||
DropRequest: &milvuspb.AlterCollectionSchemaRequest_DropRequest{
|
||||
Identifier: &milvuspb.AlterCollectionSchemaRequest_DropRequest_FieldName{FieldName: "field2"},
|
||||
},
|
||||
},
|
||||
},
|
||||
})
|
||||
require.Error(t, merr.CheckRPCCall(resp.GetAlterStatus(), err))
|
||||
require.Contains(t, resp.GetAlterStatus().GetDetail(), "failed to wait until schema drop ready")
|
||||
assertFieldExists(t, ctx, core, dbName, collectionName, "field2", 101)
|
||||
assertSchemaVersion(t, ctx, core, dbName, collectionName, 1)
|
||||
}
|
||||
|
||||
func TestDDLCallbacksAlterCollectionSchemaAddSkipsSchemaDropReady(t *testing.T) {
|
||||
core := initStreamingSystemAndCore(t)
|
||||
|
||||
ctx := context.Background()
|
||||
dbName := "testDB" + funcutil.RandomString(10)
|
||||
collectionName := "testCollection" + funcutil.RandomString(10)
|
||||
|
||||
createCollectionForTest(t, ctx, core, dbName, collectionName)
|
||||
varcharFieldSchema := &schemapb.FieldSchema{
|
||||
Name: "text_input",
|
||||
DataType: schemapb.DataType_VarChar,
|
||||
TypeParams: []*commonpb.KeyValuePair{
|
||||
{Key: common.MaxLengthKey, Value: "256"},
|
||||
},
|
||||
}
|
||||
varcharBytes, err := proto.Marshal(varcharFieldSchema)
|
||||
require.NoError(t, err)
|
||||
addFieldResp, err := core.AddCollectionField(ctx, &milvuspb.AddCollectionFieldRequest{
|
||||
DbName: dbName,
|
||||
CollectionName: collectionName,
|
||||
Schema: varcharBytes,
|
||||
})
|
||||
require.NoError(t, merr.CheckRPCCall(addFieldResp, err))
|
||||
assertSchemaVersion(t, ctx, core, dbName, collectionName, 1)
|
||||
|
||||
b := mock_balancer.NewMockBalancer(t)
|
||||
b.EXPECT().WaitUntilWALbasedDDLReady(mock.Anything).Return(nil).Maybe()
|
||||
b.EXPECT().Close().Return().Maybe()
|
||||
balance.ResetBalancer()
|
||||
balance.Register(b)
|
||||
|
||||
resp, err := core.AlterCollectionSchema(ctx, buildAlterSchemaReq(dbName, collectionName, "text_input", "sparse_output", "bm25_fn"))
|
||||
require.NoError(t, merr.CheckRPCCall(resp.GetAlterStatus(), err))
|
||||
assertSchemaVersion(t, ctx, core, dbName, collectionName, 2)
|
||||
}
|
||||
|
||||
func assertFieldNotExists(t *testing.T, ctx context.Context, core *Core, dbName string, collectionName string, fieldName string) {
|
||||
coll, err := core.meta.GetCollectionByName(ctx, dbName, collectionName, typeutil.MaxTimestamp, false)
|
||||
require.NoError(t, err)
|
||||
for _, field := range coll.Fields {
|
||||
if field.Name == fieldName {
|
||||
require.Fail(t, "field should not exist", "field %s still exists", fieldName)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func assertMaxFieldIDProperty(t *testing.T, ctx context.Context, core *Core, dbName string, collectionName string, expectedMaxFieldID int64) {
|
||||
coll, err := core.meta.GetCollectionByName(ctx, dbName, collectionName, typeutil.MaxTimestamp, false)
|
||||
require.NoError(t, err)
|
||||
for _, kv := range coll.Properties {
|
||||
if kv.Key == common.MaxFieldIDKey {
|
||||
require.Equal(t, expectedMaxFieldID, mustParseInt64(kv.Value))
|
||||
return
|
||||
}
|
||||
}
|
||||
require.Fail(t, "max_field_id property not found")
|
||||
}
|
||||
|
||||
func mustParseInt64(s string) int64 {
|
||||
v, err := strconv.ParseInt(s, 10, 64)
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
return v
|
||||
}
|
||||
|
||||
func TestBuildSchemaForDropFunction(t *testing.T) {
|
||||
t.Run("function not found", func(t *testing.T) {
|
||||
coll := &model.Collection{
|
||||
Functions: []*model.Function{
|
||||
{Name: "func1", OutputFieldIDs: []int64{103}},
|
||||
},
|
||||
}
|
||||
_, _, _, err := buildSchemaForDropFunction(coll, "nonexistent")
|
||||
require.Error(t, err)
|
||||
require.Contains(t, err.Error(), "function not found")
|
||||
})
|
||||
|
||||
t.Run("drop function removes function and output fields", func(t *testing.T) {
|
||||
coll := &model.Collection{
|
||||
Name: "test_coll",
|
||||
Fields: []*model.Field{
|
||||
{FieldID: 100, Name: "pk"},
|
||||
{FieldID: 101, Name: "text"},
|
||||
{FieldID: 102, Name: "vec"},
|
||||
{FieldID: 103, Name: "embedding_vec"},
|
||||
},
|
||||
Functions: []*model.Function{
|
||||
{
|
||||
Name: "embedding_func",
|
||||
InputFieldIDs: []int64{101},
|
||||
InputFieldNames: []string{"text"},
|
||||
OutputFieldIDs: []int64{103},
|
||||
OutputFieldNames: []string{"embedding_vec"},
|
||||
},
|
||||
},
|
||||
SchemaVersion: 5,
|
||||
}
|
||||
|
||||
schema, properties, droppedFieldIDs, err := buildSchemaForDropFunction(coll, "embedding_func")
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, []int64{103}, droppedFieldIDs)
|
||||
|
||||
// output field removed
|
||||
require.Equal(t, 3, len(schema.Fields))
|
||||
for _, f := range schema.Fields {
|
||||
require.NotEqual(t, "embedding_vec", f.Name)
|
||||
}
|
||||
|
||||
// function removed
|
||||
require.Equal(t, 0, len(schema.Functions))
|
||||
|
||||
// max_field_id updated
|
||||
require.NotNil(t, properties)
|
||||
|
||||
// schema version incremented
|
||||
require.Equal(t, int32(6), schema.Version)
|
||||
})
|
||||
|
||||
t.Run("preserves input fields and other functions", func(t *testing.T) {
|
||||
coll := &model.Collection{
|
||||
Name: "test_coll",
|
||||
Fields: []*model.Field{
|
||||
{FieldID: 100, Name: "pk"},
|
||||
{FieldID: 101, Name: "text"},
|
||||
{FieldID: 102, Name: "sparse_vec"},
|
||||
{FieldID: 103, Name: "dense_vec"},
|
||||
},
|
||||
Functions: []*model.Function{
|
||||
{
|
||||
Name: "bm25_func",
|
||||
InputFieldIDs: []int64{101},
|
||||
OutputFieldIDs: []int64{102},
|
||||
OutputFieldNames: []string{"sparse_vec"},
|
||||
},
|
||||
{
|
||||
Name: "embed_func",
|
||||
InputFieldIDs: []int64{101},
|
||||
OutputFieldIDs: []int64{103},
|
||||
OutputFieldNames: []string{"dense_vec"},
|
||||
},
|
||||
},
|
||||
SchemaVersion: 3,
|
||||
}
|
||||
|
||||
schema, _, droppedFieldIDs, err := buildSchemaForDropFunction(coll, "bm25_func")
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, []int64{102}, droppedFieldIDs)
|
||||
|
||||
// only sparse_vec removed, text and dense_vec preserved
|
||||
fieldNames := make([]string, 0, len(schema.Fields))
|
||||
for _, f := range schema.Fields {
|
||||
fieldNames = append(fieldNames, f.Name)
|
||||
}
|
||||
require.Contains(t, fieldNames, "pk")
|
||||
require.Contains(t, fieldNames, "text")
|
||||
require.Contains(t, fieldNames, "dense_vec")
|
||||
require.NotContains(t, fieldNames, "sparse_vec")
|
||||
|
||||
// only bm25_func removed, embed_func preserved
|
||||
require.Equal(t, 1, len(schema.Functions))
|
||||
require.Equal(t, "embed_func", schema.Functions[0].Name)
|
||||
})
|
||||
}
|
||||
|
||||
func TestBuildSchemaForDropField(t *testing.T) {
|
||||
baseColl := func() *model.Collection {
|
||||
return &model.Collection{
|
||||
Name: "test_coll",
|
||||
Fields: []*model.Field{
|
||||
{FieldID: 100, Name: "pk"},
|
||||
{FieldID: 101, Name: "vec"},
|
||||
{FieldID: 102, Name: "extra"},
|
||||
},
|
||||
SchemaVersion: 3,
|
||||
}
|
||||
}
|
||||
|
||||
t.Run("drop by field name", func(t *testing.T) {
|
||||
schema, properties, droppedFieldIDs, err := buildSchemaForDropField(baseColl(), "extra", 0)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, 2, len(schema.Fields))
|
||||
require.NotNil(t, properties)
|
||||
require.Equal(t, []int64{102}, droppedFieldIDs)
|
||||
require.Equal(t, int32(4), schema.Version)
|
||||
})
|
||||
|
||||
t.Run("drop by field id", func(t *testing.T) {
|
||||
schema, _, droppedFieldIDs, err := buildSchemaForDropField(baseColl(), "", 101)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, []int64{101}, droppedFieldIDs)
|
||||
require.Equal(t, 2, len(schema.Fields))
|
||||
// remaining fields should be pk and extra
|
||||
fieldNames := make([]string, 0, len(schema.Fields))
|
||||
for _, f := range schema.Fields {
|
||||
fieldNames = append(fieldNames, f.Name)
|
||||
}
|
||||
require.Contains(t, fieldNames, "pk")
|
||||
require.Contains(t, fieldNames, "extra")
|
||||
})
|
||||
|
||||
t.Run("field not found by name", func(t *testing.T) {
|
||||
_, _, _, err := buildSchemaForDropField(baseColl(), "nonexistent", 0)
|
||||
require.Error(t, err)
|
||||
require.Contains(t, err.Error(), "field not found: nonexistent")
|
||||
})
|
||||
|
||||
t.Run("field not found by id", func(t *testing.T) {
|
||||
_, _, _, err := buildSchemaForDropField(baseColl(), "", 999)
|
||||
require.Error(t, err)
|
||||
require.Contains(t, err.Error(), "field not found with id: 999")
|
||||
})
|
||||
|
||||
t.Run("max_field_id property updated", func(t *testing.T) {
|
||||
coll := baseColl()
|
||||
coll.Properties = []*commonpb.KeyValuePair{
|
||||
{Key: common.MaxFieldIDKey, Value: "102"},
|
||||
}
|
||||
_, properties, _, err := buildSchemaForDropField(coll, "extra", 0)
|
||||
require.NoError(t, err)
|
||||
var found bool
|
||||
for _, kv := range properties {
|
||||
if kv.Key == common.MaxFieldIDKey {
|
||||
require.Equal(t, "102", kv.Value)
|
||||
found = true
|
||||
}
|
||||
}
|
||||
require.True(t, found)
|
||||
})
|
||||
|
||||
collWithStruct := func() *model.Collection {
|
||||
return &model.Collection{
|
||||
Name: "test_coll",
|
||||
Fields: []*model.Field{
|
||||
{FieldID: 100, Name: "pk"},
|
||||
{FieldID: 101, Name: "vec"},
|
||||
},
|
||||
StructArrayFields: []*model.StructArrayField{
|
||||
{
|
||||
FieldID: 102, Name: "paragraphs",
|
||||
Fields: []*model.Field{
|
||||
{FieldID: 103, Name: "para_text"},
|
||||
{FieldID: 104, Name: "para_embed"},
|
||||
},
|
||||
},
|
||||
},
|
||||
SchemaVersion: 3,
|
||||
}
|
||||
}
|
||||
|
||||
t.Run("drop whole struct array field by name", func(t *testing.T) {
|
||||
schema, _, droppedFieldIDs, err := buildSchemaForDropField(collWithStruct(), "paragraphs", 0)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, 2, len(schema.Fields)) // pk + vec unchanged
|
||||
require.Equal(t, 0, len(schema.StructArrayFields)) // struct removed
|
||||
require.Equal(t, []int64{102, 103, 104}, droppedFieldIDs)
|
||||
require.Equal(t, int32(4), schema.Version)
|
||||
})
|
||||
|
||||
t.Run("drop whole struct array field by id", func(t *testing.T) {
|
||||
schema, _, droppedFieldIDs, err := buildSchemaForDropField(collWithStruct(), "", 102)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, 0, len(schema.StructArrayFields))
|
||||
require.Equal(t, []int64{102, 103, 104}, droppedFieldIDs)
|
||||
})
|
||||
}
|
||||
|
||||
func TestCascadeDropFieldIndexesInline(t *testing.T) {
|
||||
buildResult := func(droppedFieldIDs []int64) message.BroadcastResultAlterCollectionMessageV2 {
|
||||
raw := message.NewAlterCollectionMessageBuilderV2().
|
||||
WithHeader(&messagespb.AlterCollectionMessageHeader{
|
||||
CollectionId: 1,
|
||||
DroppedFieldIds: droppedFieldIDs,
|
||||
}).
|
||||
WithBody(&messagespb.AlterCollectionMessageBody{
|
||||
Updates: &messagespb.AlterCollectionMessageUpdates{},
|
||||
}).
|
||||
WithBroadcast([]string{funcutil.GetControlChannel("test")}).
|
||||
MustBuildBroadcast()
|
||||
msg := message.MustAsBroadcastAlterCollectionMessageV2(raw)
|
||||
return message.BroadcastResultAlterCollectionMessageV2{
|
||||
Message: msg,
|
||||
Results: map[string]*message.AppendResult{},
|
||||
}
|
||||
}
|
||||
|
||||
t.Run("no dropped fields short circuits", func(t *testing.T) {
|
||||
c := newTestCore()
|
||||
cb := &DDLCallback{Core: c}
|
||||
err := cb.cascadeDropFieldIndexesInline(context.Background(), buildResult(nil))
|
||||
require.NoError(t, err)
|
||||
})
|
||||
|
||||
t.Run("DescribeIndex returns ErrIndexNotFound", func(t *testing.T) {
|
||||
mixc := imocks.NewMixCoord(t)
|
||||
mixc.EXPECT().DescribeIndex(mock.Anything, mock.Anything).Return(
|
||||
&indexpb.DescribeIndexResponse{Status: merr.Status(merr.WrapErrIndexNotFound("idx"))}, nil,
|
||||
)
|
||||
c := newTestCore(withMixCoord(mixc))
|
||||
cb := &DDLCallback{Core: c}
|
||||
err := cb.cascadeDropFieldIndexesInline(context.Background(), buildResult([]int64{101}))
|
||||
require.NoError(t, err)
|
||||
})
|
||||
|
||||
t.Run("DescribeIndex returns other error", func(t *testing.T) {
|
||||
mixc := imocks.NewMixCoord(t)
|
||||
mixc.EXPECT().DescribeIndex(mock.Anything, mock.Anything).Return(
|
||||
nil, errors.New("rpc unavailable"),
|
||||
)
|
||||
c := newTestCore(withMixCoord(mixc))
|
||||
cb := &DDLCallback{Core: c}
|
||||
err := cb.cascadeDropFieldIndexesInline(context.Background(), buildResult([]int64{101}))
|
||||
require.Error(t, err)
|
||||
// Match the stable prefix of the DescribeIndex failure path. The
|
||||
// "for cascade drop" suffix is incidental and may be reworded without
|
||||
// changing behavior, so we don't assert on it.
|
||||
require.Contains(t, err.Error(), "failed to describe indexes")
|
||||
require.ErrorContains(t, err, "rpc unavailable")
|
||||
})
|
||||
|
||||
t.Run("no matching indexes for dropped field", func(t *testing.T) {
|
||||
mixc := imocks.NewMixCoord(t)
|
||||
mixc.EXPECT().DescribeIndex(mock.Anything, mock.Anything).Return(
|
||||
&indexpb.DescribeIndexResponse{
|
||||
Status: merr.Success(),
|
||||
IndexInfos: []*indexpb.IndexInfo{
|
||||
{FieldID: 200, IndexID: 1, IndexName: "idx_other"},
|
||||
},
|
||||
}, nil,
|
||||
)
|
||||
c := newTestCore(withMixCoord(mixc))
|
||||
cb := &DDLCallback{Core: c}
|
||||
err := cb.cascadeDropFieldIndexesInline(context.Background(), buildResult([]int64{101}))
|
||||
require.NoError(t, err)
|
||||
})
|
||||
}
|
||||
|
||||
func TestAlterCollectionV2AckCallbackUsesHeaderDroppedFieldIDs(t *testing.T) {
|
||||
raw := message.NewAlterCollectionMessageBuilderV2().
|
||||
WithHeader(&messagespb.AlterCollectionMessageHeader{
|
||||
CollectionId: 1,
|
||||
UpdateMask: &fieldmaskpb.FieldMask{
|
||||
Paths: []string{message.FieldMaskCollectionSchema},
|
||||
},
|
||||
CacheExpirations: &messagespb.CacheExpirations{},
|
||||
DroppedFieldIds: []int64{101},
|
||||
}).
|
||||
WithBody(&messagespb.AlterCollectionMessageBody{
|
||||
Updates: &messagespb.AlterCollectionMessageUpdates{
|
||||
Schema: &schemapb.CollectionSchema{
|
||||
Name: "test",
|
||||
Version: 2,
|
||||
},
|
||||
},
|
||||
}).
|
||||
WithBroadcast([]string{funcutil.GetControlChannel("test")}).
|
||||
MustBuildBroadcast()
|
||||
|
||||
meta := &mockMetaTable{}
|
||||
meta.AlterCollectionFunc = func(ctx context.Context, result message.BroadcastResultAlterCollectionMessageV2) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
mixc := imocks.NewMixCoord(t)
|
||||
mixc.EXPECT().DescribeIndex(mock.Anything, mock.Anything).Return(
|
||||
&indexpb.DescribeIndexResponse{
|
||||
Status: merr.Success(),
|
||||
IndexInfos: []*indexpb.IndexInfo{
|
||||
{FieldID: 200, IndexID: 1, IndexName: "idx_other"},
|
||||
},
|
||||
}, nil,
|
||||
)
|
||||
|
||||
broker := newValidMockBroker()
|
||||
c := newTestCore(withMeta(meta), withMixCoord(mixc), withBroker(broker))
|
||||
cb := &DDLCallback{Core: c}
|
||||
err := cb.alterCollectionV2AckCallback(context.Background(), message.BroadcastResultAlterCollectionMessageV2{
|
||||
Message: message.MustAsBroadcastAlterCollectionMessageV2(raw),
|
||||
Results: map[string]*message.AppendResult{
|
||||
funcutil.GetControlChannel("test"): {TimeTick: 100},
|
||||
},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
@@ -266,6 +266,7 @@ func (mt *MetaTable) reload() error {
|
||||
)
|
||||
}
|
||||
collection.DBName = dbName // some collections may not have db name or its dbname is not correct, we should fix it here.
|
||||
ensureCollectionMaxFieldIDProperty(collection)
|
||||
mt.collID2Meta[collection.CollectionID] = collection
|
||||
// Build partition name index
|
||||
mt.partitionName2ID[collection.CollectionID] = make(map[string]int64)
|
||||
@@ -343,6 +344,7 @@ func (mt *MetaTable) reloadWithNonDatabase() error {
|
||||
}
|
||||
|
||||
for _, collection := range oldCollections {
|
||||
ensureCollectionMaxFieldIDProperty(collection)
|
||||
mt.collID2Meta[collection.CollectionID] = collection
|
||||
if collection.Available() {
|
||||
mt.names.insert(util.DefaultDBName, collection.Name, collection.CollectionID)
|
||||
|
||||
@@ -28,6 +28,7 @@ import (
|
||||
|
||||
"github.com/milvus-io/milvus-proto/go-api/v3/commonpb"
|
||||
"github.com/milvus-io/milvus-proto/go-api/v3/milvuspb"
|
||||
"github.com/milvus-io/milvus-proto/go-api/v3/schemapb"
|
||||
etcdkv "github.com/milvus-io/milvus/internal/kv/etcd"
|
||||
"github.com/milvus-io/milvus/internal/metastore"
|
||||
"github.com/milvus-io/milvus/internal/metastore/kv/rootcoord"
|
||||
@@ -2788,3 +2789,52 @@ func TestMetaTable_TruncateCollection(t *testing.T) {
|
||||
require.Equal(t, 1, len(coll.ShardInfos))
|
||||
require.Equal(t, uint64(1000), coll.ShardInfos["vchannel1"].LastTruncateTimeTick)
|
||||
}
|
||||
|
||||
func TestMetaTableReloadNormalizesMaxFieldIDProperty(t *testing.T) {
|
||||
channel.ResetStaticPChannelStatsManager()
|
||||
|
||||
kv, _ := kvfactory.GetEtcdAndPath()
|
||||
path := funcutil.RandomString(10) + "/meta"
|
||||
catalogKV := etcdkv.NewEtcdKV(kv, path)
|
||||
catalog := rootcoord.NewCatalog(catalogKV)
|
||||
|
||||
allocator := mocktso.NewAllocator(t)
|
||||
allocator.EXPECT().GenerateTSO(mock.Anything).Return(1000, nil)
|
||||
|
||||
meta, err := NewMetaTable(context.Background(), catalog, allocator)
|
||||
require.NoError(t, err)
|
||||
|
||||
err = meta.AddCollection(context.Background(), &model.Collection{
|
||||
CollectionID: 1,
|
||||
DBID: util.DefaultDBID,
|
||||
DBName: util.DefaultDBName,
|
||||
Name: "test_reload_max_field_id",
|
||||
PhysicalChannelNames: []string{"pchannel1"},
|
||||
VirtualChannelNames: []string{"vchannel1"},
|
||||
State: pb.CollectionState_CollectionCreated,
|
||||
Fields: []*model.Field{
|
||||
{FieldID: 100, Name: "pk", DataType: schemapb.DataType_Int64},
|
||||
{FieldID: 105, Name: "vec", DataType: schemapb.DataType_FloatVector},
|
||||
},
|
||||
Properties: common.NewKeyValuePairs(map[string]string{
|
||||
common.CollectionReplicaNumber: "1",
|
||||
}),
|
||||
ShardInfos: map[string]*model.ShardInfo{
|
||||
"vchannel1": {
|
||||
VChannelName: "vchannel1",
|
||||
PChannelName: "pchannel1",
|
||||
LastTruncateTimeTick: 0,
|
||||
},
|
||||
},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
channel.ResetStaticPChannelStatsManager()
|
||||
meta, err = NewMetaTable(context.Background(), catalog, allocator)
|
||||
require.NoError(t, err)
|
||||
|
||||
coll, err := meta.GetCollectionByID(context.Background(), util.DefaultDBName, 1, typeutil.MaxTimestamp, false)
|
||||
require.NoError(t, err)
|
||||
props := common.CloneKeyValuePairs(coll.Properties).ToMap()
|
||||
require.Equal(t, "105", props[common.MaxFieldIDKey])
|
||||
}
|
||||
|
||||
@@ -742,6 +742,11 @@ func withValidMixCoord() Opt {
|
||||
mixc.EXPECT().DropIndex(mock.Anything, mock.Anything).Return(
|
||||
merr.Success(), nil,
|
||||
)
|
||||
mixc.EXPECT().DescribeIndex(mock.Anything, mock.Anything).Return(
|
||||
&indexpb.DescribeIndexResponse{
|
||||
Status: merr.Status(merr.WrapErrIndexNotFound("")),
|
||||
}, nil,
|
||||
)
|
||||
mixc.EXPECT().NotifyDropPartition(mock.Anything, mock.Anything, mock.Anything).Return(nil)
|
||||
|
||||
mixc.EXPECT().DropSegmentsByTime(mock.Anything, mock.Anything, mock.Anything).Return(nil)
|
||||
|
||||
@@ -173,11 +173,13 @@ func initStreamingSystemAndCore(t *testing.T) *Core {
|
||||
return vchannels, nil
|
||||
}).Maybe()
|
||||
b.EXPECT().WaitUntilWALbasedDDLReady(mock.Anything).Return(nil).Maybe()
|
||||
b.EXPECT().WaitUntilSchemaDropReady(mock.Anything).Return(nil).Maybe()
|
||||
b.EXPECT().WatchChannelAssignments(mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, callback balancer.WatchChannelAssignmentsCallback) error {
|
||||
<-ctx.Done()
|
||||
return ctx.Err()
|
||||
}).Maybe()
|
||||
b.EXPECT().Close().Return().Maybe()
|
||||
balance.ResetBalancer()
|
||||
balance.Register(b)
|
||||
channel.ResetStaticPChannelStatsManager()
|
||||
channel.RecoverPChannelStatsManager([]string{})
|
||||
|
||||
+69
-13
@@ -578,27 +578,83 @@ func validateStructArrayFieldDataType(fieldSchemas []*schemapb.StructArrayFieldS
|
||||
return nil
|
||||
}
|
||||
|
||||
func nextFieldID(coll *model.Collection) int64 {
|
||||
func maxAssignedFieldIDFromSchema(schema *schemapb.CollectionSchema) int64 {
|
||||
maxFieldID := int64(common.StartOfUserFieldID)
|
||||
for _, field := range coll.Fields {
|
||||
if field.FieldID > maxFieldID {
|
||||
maxFieldID = field.FieldID
|
||||
if schema == nil {
|
||||
return maxFieldID
|
||||
}
|
||||
for _, field := range schema.GetFields() {
|
||||
if field.GetFieldID() > maxFieldID {
|
||||
maxFieldID = field.GetFieldID()
|
||||
}
|
||||
}
|
||||
|
||||
// Also check StructArrayFields and their sub-fields to avoid ID conflicts
|
||||
for _, structField := range coll.StructArrayFields {
|
||||
if structField.FieldID > maxFieldID {
|
||||
maxFieldID = structField.FieldID
|
||||
for _, structField := range schema.GetStructArrayFields() {
|
||||
if structField.GetFieldID() > maxFieldID {
|
||||
maxFieldID = structField.GetFieldID()
|
||||
}
|
||||
for _, subField := range structField.Fields {
|
||||
if subField.FieldID > maxFieldID {
|
||||
maxFieldID = subField.FieldID
|
||||
for _, subField := range structField.GetFields() {
|
||||
if subField.GetFieldID() > maxFieldID {
|
||||
maxFieldID = subField.GetFieldID()
|
||||
}
|
||||
}
|
||||
}
|
||||
for _, kv := range schema.GetProperties() {
|
||||
if kv.GetKey() != common.MaxFieldIDKey {
|
||||
continue
|
||||
}
|
||||
v, err := strconv.ParseInt(kv.GetValue(), 10, 64)
|
||||
if err != nil {
|
||||
log.Warn("failed to parse max_field_id property, metadata may be corrupted",
|
||||
zap.String("value", kv.GetValue()),
|
||||
zap.Error(err),
|
||||
)
|
||||
} else if v > maxFieldID {
|
||||
maxFieldID = v
|
||||
}
|
||||
break
|
||||
}
|
||||
return maxFieldID
|
||||
}
|
||||
|
||||
return maxFieldID + 1
|
||||
// updateMaxFieldIDProperty returns a new properties slice with max_field_id set.
|
||||
// The original slice is not modified.
|
||||
func updateMaxFieldIDProperty(properties []*commonpb.KeyValuePair, maxFieldID int64) []*commonpb.KeyValuePair {
|
||||
result := make([]*commonpb.KeyValuePair, 0, len(properties)+1)
|
||||
found := false
|
||||
for _, kv := range properties {
|
||||
if kv.GetKey() == common.MaxFieldIDKey {
|
||||
v, err := strconv.ParseInt(kv.GetValue(), 10, 64)
|
||||
if err != nil {
|
||||
log.Warn("failed to parse max_field_id property, metadata may be corrupted",
|
||||
zap.String("value", kv.GetValue()),
|
||||
zap.Error(err),
|
||||
)
|
||||
} else if v > maxFieldID {
|
||||
maxFieldID = v
|
||||
}
|
||||
result = append(result, &commonpb.KeyValuePair{
|
||||
Key: common.MaxFieldIDKey,
|
||||
Value: strconv.FormatInt(maxFieldID, 10),
|
||||
})
|
||||
found = true
|
||||
continue
|
||||
}
|
||||
result = append(result, kv)
|
||||
}
|
||||
if !found {
|
||||
result = append(result, &commonpb.KeyValuePair{
|
||||
Key: common.MaxFieldIDKey,
|
||||
Value: strconv.FormatInt(maxFieldID, 10),
|
||||
})
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
func ensureCollectionMaxFieldIDProperty(coll *model.Collection) {
|
||||
if coll == nil {
|
||||
return
|
||||
}
|
||||
coll.Properties = updateMaxFieldIDProperty(coll.Properties, maxAssignedFieldIDFromSchema(coll.ToCollectionSchemaPB()))
|
||||
}
|
||||
|
||||
func nextFunctionID(coll *model.Collection) int64 {
|
||||
|
||||
@@ -456,9 +456,9 @@ func TestIsSubsetOfProperties(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func Test_nextFieldID(t *testing.T) {
|
||||
func Test_maxAssignedFieldIDFromSchema(t *testing.T) {
|
||||
type args struct {
|
||||
coll *model.Collection
|
||||
schema *schemapb.CollectionSchema
|
||||
}
|
||||
tests := []struct {
|
||||
name string
|
||||
@@ -468,14 +468,14 @@ func Test_nextFieldID(t *testing.T) {
|
||||
{
|
||||
name: "collection with max field ID in struct array sub-field",
|
||||
args: args{
|
||||
coll: &model.Collection{
|
||||
Fields: []*model.Field{
|
||||
schema: &schemapb.CollectionSchema{
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: common.StartOfUserFieldID},
|
||||
},
|
||||
StructArrayFields: []*model.StructArrayField{
|
||||
StructArrayFields: []*schemapb.StructArrayFieldSchema{
|
||||
{
|
||||
FieldID: common.StartOfUserFieldID + 1,
|
||||
Fields: []*model.Field{
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: common.StartOfUserFieldID + 2},
|
||||
{FieldID: common.StartOfUserFieldID + 10},
|
||||
},
|
||||
@@ -483,25 +483,25 @@ func Test_nextFieldID(t *testing.T) {
|
||||
},
|
||||
},
|
||||
},
|
||||
want: common.StartOfUserFieldID + 11,
|
||||
want: common.StartOfUserFieldID + 10,
|
||||
},
|
||||
{
|
||||
name: "collection with multiple struct array fields",
|
||||
args: args{
|
||||
coll: &model.Collection{
|
||||
Fields: []*model.Field{
|
||||
schema: &schemapb.CollectionSchema{
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: common.StartOfUserFieldID},
|
||||
},
|
||||
StructArrayFields: []*model.StructArrayField{
|
||||
StructArrayFields: []*schemapb.StructArrayFieldSchema{
|
||||
{
|
||||
FieldID: common.StartOfUserFieldID + 1,
|
||||
Fields: []*model.Field{
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: common.StartOfUserFieldID + 2},
|
||||
},
|
||||
},
|
||||
{
|
||||
FieldID: common.StartOfUserFieldID + 5,
|
||||
Fields: []*model.Field{
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: common.StartOfUserFieldID + 6},
|
||||
{FieldID: common.StartOfUserFieldID + 7},
|
||||
},
|
||||
@@ -509,12 +509,42 @@ func Test_nextFieldID(t *testing.T) {
|
||||
},
|
||||
},
|
||||
},
|
||||
want: common.StartOfUserFieldID + 8,
|
||||
want: common.StartOfUserFieldID + 7,
|
||||
},
|
||||
{
|
||||
name: "collection with max_field_id property after field drop",
|
||||
args: args{
|
||||
schema: &schemapb.CollectionSchema{
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: common.StartOfUserFieldID}, // 100
|
||||
{FieldID: common.StartOfUserFieldID + 1}, // 101
|
||||
},
|
||||
Properties: []*commonpb.KeyValuePair{
|
||||
{Key: common.MaxFieldIDKey, Value: "102"},
|
||||
},
|
||||
},
|
||||
},
|
||||
want: common.StartOfUserFieldID + 2, // 102, not 101
|
||||
},
|
||||
{
|
||||
name: "max_field_id property smaller than current fields",
|
||||
args: args{
|
||||
schema: &schemapb.CollectionSchema{
|
||||
Fields: []*schemapb.FieldSchema{
|
||||
{FieldID: common.StartOfUserFieldID}, // 100
|
||||
{FieldID: common.StartOfUserFieldID + 5}, // 105
|
||||
},
|
||||
Properties: []*commonpb.KeyValuePair{
|
||||
{Key: common.MaxFieldIDKey, Value: "102"},
|
||||
},
|
||||
},
|
||||
},
|
||||
want: common.StartOfUserFieldID + 5, // 105, current fields dominate
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := nextFieldID(tt.args.coll)
|
||||
got := maxAssignedFieldIDFromSchema(tt.args.schema)
|
||||
assert.Equal(t, tt.want, got)
|
||||
})
|
||||
}
|
||||
@@ -688,3 +718,45 @@ func TestCheckStructArrayFieldSchema_MaxCapacityValidation(t *testing.T) {
|
||||
assert.Contains(t, err.Error(), "sub_b")
|
||||
})
|
||||
}
|
||||
|
||||
func Test_updateMaxFieldIDProperty(t *testing.T) {
|
||||
t.Run("add to empty properties", func(t *testing.T) {
|
||||
props := updateMaxFieldIDProperty(nil, 105)
|
||||
assert.Len(t, props, 1)
|
||||
assert.Equal(t, common.MaxFieldIDKey, props[0].Key)
|
||||
assert.Equal(t, "105", props[0].Value)
|
||||
})
|
||||
|
||||
t.Run("update existing property", func(t *testing.T) {
|
||||
props := []*commonpb.KeyValuePair{
|
||||
{Key: "other_key", Value: "other_value"},
|
||||
{Key: common.MaxFieldIDKey, Value: "100"},
|
||||
}
|
||||
result := updateMaxFieldIDProperty(props, 105)
|
||||
assert.Len(t, result, 2)
|
||||
assert.Equal(t, "105", result[1].Value)
|
||||
})
|
||||
|
||||
t.Run("update existing property does not mutate original", func(t *testing.T) {
|
||||
original := &commonpb.KeyValuePair{Key: common.MaxFieldIDKey, Value: "100"}
|
||||
props := []*commonpb.KeyValuePair{
|
||||
{Key: "other_key", Value: "other_value"},
|
||||
original,
|
||||
}
|
||||
result := updateMaxFieldIDProperty(props, 105)
|
||||
assert.Len(t, result, 2)
|
||||
assert.Equal(t, "105", result[1].Value)
|
||||
// verify original is NOT modified
|
||||
assert.Equal(t, "100", original.Value)
|
||||
})
|
||||
|
||||
t.Run("append to non-empty properties", func(t *testing.T) {
|
||||
props := []*commonpb.KeyValuePair{
|
||||
{Key: "other_key", Value: "other_value"},
|
||||
}
|
||||
result := updateMaxFieldIDProperty(props, 103)
|
||||
assert.Len(t, result, 2)
|
||||
assert.Equal(t, common.MaxFieldIDKey, result[1].Key)
|
||||
assert.Equal(t, "103", result[1].Value)
|
||||
})
|
||||
}
|
||||
|
||||
@@ -57,6 +57,9 @@ type Balancer interface {
|
||||
// WaitUntilWALbasedDDLReady waits until the WAL based DDL is ready.
|
||||
WaitUntilWALbasedDDLReady(ctx context.Context) error
|
||||
|
||||
// WaitUntilSchemaDropReady waits until schema-drop DDL can be accepted.
|
||||
WaitUntilSchemaDropReady(ctx context.Context) error
|
||||
|
||||
// RegisterStreamingEnabledNotifier registers a notifier into the balancer.
|
||||
// If the error is returned, the balancer is closed.
|
||||
// Otherwise, the following rules are applied:
|
||||
|
||||
@@ -28,6 +28,7 @@ import (
|
||||
const (
|
||||
versionChecker260 = "<2.6.0-dev"
|
||||
versionChecker265 = "<2.6.6-dev"
|
||||
versionChecker300 = "<3.0.0-beta"
|
||||
)
|
||||
|
||||
// RecoverBalancer recover the balancer working.
|
||||
@@ -184,7 +185,7 @@ func (b *balancerImpl) GetLatestWALLocated(ctx context.Context, pchannel string)
|
||||
|
||||
// WaitUntilWALbasedDDLReady waits until the WAL based DDL is ready.
|
||||
func (b *balancerImpl) WaitUntilWALbasedDDLReady(ctx context.Context) error {
|
||||
if b.channelMetaManager.IsWALBasedDDLEnabled() {
|
||||
if b.channelMetaManager.IsStreamingVersionAtLeast(channel.StreamingVersion265) {
|
||||
return nil
|
||||
}
|
||||
if err := b.channelMetaManager.WaitUntilStreamingEnabled(ctx); err != nil {
|
||||
@@ -193,7 +194,22 @@ func (b *balancerImpl) WaitUntilWALbasedDDLReady(ctx context.Context) error {
|
||||
if err := b.blockUntilRoleGreaterThanVersion(ctx, typeutil.StreamingNodeRole, versionChecker265); err != nil {
|
||||
return err
|
||||
}
|
||||
return b.channelMetaManager.MarkWALBasedDDLEnabled(ctx)
|
||||
return b.channelMetaManager.MarkStreamingVersion(ctx, channel.StreamingVersion265)
|
||||
}
|
||||
|
||||
// WaitUntilSchemaDropReady waits until every Proxy can attach schema version
|
||||
// to insert messages, so schema-drop DDL cannot race with legacy writes.
|
||||
func (b *balancerImpl) WaitUntilSchemaDropReady(ctx context.Context) error {
|
||||
if b.channelMetaManager.IsStreamingVersionAtLeast(channel.StreamingVersion300) {
|
||||
return nil
|
||||
}
|
||||
if err := b.WaitUntilWALbasedDDLReady(ctx); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := b.blockUntilRoleGreaterThanVersion(ctx, typeutil.ProxyRole, versionChecker300); err != nil {
|
||||
return err
|
||||
}
|
||||
return b.channelMetaManager.MarkStreamingVersion(ctx, channel.StreamingVersion300)
|
||||
}
|
||||
|
||||
// WatchChannelAssignments watches the balance result.
|
||||
@@ -479,7 +495,7 @@ func (b *balancerImpl) blockUntilExpectedInitialStreamingNodeNumReached(ctx cont
|
||||
}
|
||||
}
|
||||
|
||||
// blockUntilRoleGreaterThanVersion block until the role is greater than 2.6.0 at background.
|
||||
// blockUntilRoleGreaterThanVersion blocks until every session of the role is outside versionChecker.
|
||||
func (b *balancerImpl) blockUntilRoleGreaterThanVersion(ctx context.Context, role string, versionChecker string) error {
|
||||
doneErr := errors.New("done")
|
||||
logger := b.Logger().With(zap.String("role", role))
|
||||
|
||||
@@ -3,6 +3,7 @@ package balancer_test
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"path"
|
||||
"testing"
|
||||
"time"
|
||||
@@ -284,6 +285,143 @@ func TestBalancer(t *testing.T) {
|
||||
assert.ErrorIs(t, f.Get(), balancer.ErrBalancerClosed)
|
||||
}
|
||||
|
||||
func TestBalancerWaitUntilSchemaDropReady(t *testing.T) {
|
||||
paramtable.Init()
|
||||
oldRootPath := paramtable.Get().EtcdCfg.RootPath.SwapTempValue(fmt.Sprintf("schema-drop-ready-%d", time.Now().UnixNano()))
|
||||
oldMetaSubPath := paramtable.Get().EtcdCfg.MetaSubPath.SwapTempValue("meta")
|
||||
oldExpectedStreamingNodeNum := paramtable.Get().StreamingCfg.WALBalancerExpectedInitialStreamingNodeNum.SwapTempValue("0")
|
||||
defer paramtable.Get().EtcdCfg.RootPath.SwapTempValue(oldRootPath)
|
||||
defer paramtable.Get().EtcdCfg.MetaSubPath.SwapTempValue(oldMetaSubPath)
|
||||
defer paramtable.Get().StreamingCfg.WALBalancerExpectedInitialStreamingNodeNum.SwapTempValue(oldExpectedStreamingNodeNum)
|
||||
metaRoot := paramtable.Get().EtcdCfg.MetaRootPath.GetValue()
|
||||
|
||||
etcdClient, _ := kvfactory.GetEtcdAndPath()
|
||||
channel.ResetStaticPChannelStatsManager()
|
||||
channel.RecoverPChannelStatsManager([]string{})
|
||||
|
||||
streamingNodeManager := mock_manager.NewMockManagerClient(t)
|
||||
streamingNodeManager.EXPECT().WatchNodeChanged(mock.Anything).Return(make(chan struct{}), nil).Maybe()
|
||||
streamingNodeManager.EXPECT().Assign(mock.Anything, mock.Anything).Return(nil).Maybe()
|
||||
streamingNodeManager.EXPECT().Remove(mock.Anything, mock.Anything).Return(nil).Maybe()
|
||||
streamingNodeManager.EXPECT().GetAllStreamingNodes(mock.Anything).Return(map[int64]*types.StreamingNodeInfoWithResourceGroup{}, nil).Maybe()
|
||||
streamingNodeManager.EXPECT().CollectAllStatus(mock.Anything, mock.Anything).Return(map[int64]*types.StreamingNodeStatus{}, nil).Maybe()
|
||||
|
||||
catalog := mock_metastore.NewMockStreamingCoordCataLog(t)
|
||||
s := sessionutil.NewMockSession(t)
|
||||
s.EXPECT().GetRegisteredRevision().Return(int64(1))
|
||||
resource.InitForTest(
|
||||
resource.OptETCD(etcdClient),
|
||||
resource.OptStreamingCatalog(catalog),
|
||||
resource.OptStreamingManagerClient(streamingNodeManager),
|
||||
resource.OptSession(s),
|
||||
)
|
||||
catalog.EXPECT().GetCChannel(mock.Anything).Return(&streamingpb.CChannelMeta{Pchannel: "schema-drop-ready-channel"}, nil)
|
||||
catalog.EXPECT().GetVersion(mock.Anything).Return(nil, nil)
|
||||
savedVersions := make(chan int64, 4)
|
||||
catalog.EXPECT().SaveVersion(mock.Anything, mock.Anything).Run(func(_ context.Context, version *streamingpb.StreamingVersion) {
|
||||
savedVersions <- version.GetVersion()
|
||||
}).Return(nil).Maybe()
|
||||
catalog.EXPECT().ListPChannel(mock.Anything).Return(nil, nil)
|
||||
catalog.EXPECT().SavePChannels(mock.Anything, mock.Anything).Return(nil).Maybe()
|
||||
catalog.EXPECT().GetReplicateConfiguration(mock.Anything).Return(nil, nil)
|
||||
|
||||
ctx := context.Background()
|
||||
b, err := balancer.RecoverBalancer(ctx, newStaticChannelProvider())
|
||||
if !assert.NoError(t, err) {
|
||||
return
|
||||
}
|
||||
defer b.Close()
|
||||
|
||||
waitCtx, cancel := context.WithTimeout(ctx, time.Second)
|
||||
defer cancel()
|
||||
|
||||
readyProxyKey := path.Join(metaRoot, sessionutil.DefaultServiceRoot, typeutil.ProxyRole+"-ready")
|
||||
putProxySession(t, waitCtx, readyProxyKey, "3.0.0-beta")
|
||||
defer resource.Resource().ETCD().Delete(context.Background(), readyProxyKey)
|
||||
|
||||
legacyProxyKey := path.Join(metaRoot, sessionutil.DefaultServiceRoot, typeutil.ProxyRole+"-legacy")
|
||||
putProxySession(t, waitCtx, legacyProxyKey, "2.6.6")
|
||||
defer resource.Resource().ETCD().Delete(context.Background(), legacyProxyKey)
|
||||
|
||||
cancelCtx, cancel := context.WithTimeout(ctx, 100*time.Millisecond)
|
||||
err = b.WaitUntilSchemaDropReady(cancelCtx)
|
||||
cancel()
|
||||
assert.ErrorIs(t, err, context.DeadlineExceeded)
|
||||
|
||||
waitDone := make(chan error, 1)
|
||||
go func() {
|
||||
waitDone <- b.WaitUntilSchemaDropReady(context.Background())
|
||||
}()
|
||||
select {
|
||||
case err := <-waitDone:
|
||||
assert.NoError(t, err)
|
||||
assert.Fail(t, "schema drop readiness should wait for legacy Proxy sessions")
|
||||
case <-time.After(100 * time.Millisecond):
|
||||
}
|
||||
|
||||
_, err = resource.Resource().ETCD().Delete(context.Background(), legacyProxyKey)
|
||||
assert.NoError(t, err)
|
||||
select {
|
||||
case err := <-waitDone:
|
||||
assert.NoError(t, err)
|
||||
case <-time.After(3 * time.Second):
|
||||
assert.Fail(t, "schema drop readiness did not unblock after legacy Proxy session disappeared")
|
||||
}
|
||||
assertSavedStreamingVersion(t, savedVersions, channel.StreamingVersion300)
|
||||
}
|
||||
|
||||
func TestBalancerWaitUntilSchemaDropReadySkipsAfterPersistedVersion(t *testing.T) {
|
||||
paramtable.Init()
|
||||
oldRootPath := paramtable.Get().EtcdCfg.RootPath.SwapTempValue(fmt.Sprintf("schema-drop-ready-skip-%d", time.Now().UnixNano()))
|
||||
oldMetaSubPath := paramtable.Get().EtcdCfg.MetaSubPath.SwapTempValue("meta")
|
||||
oldExpectedStreamingNodeNum := paramtable.Get().StreamingCfg.WALBalancerExpectedInitialStreamingNodeNum.SwapTempValue("0")
|
||||
defer paramtable.Get().EtcdCfg.RootPath.SwapTempValue(oldRootPath)
|
||||
defer paramtable.Get().EtcdCfg.MetaSubPath.SwapTempValue(oldMetaSubPath)
|
||||
defer paramtable.Get().StreamingCfg.WALBalancerExpectedInitialStreamingNodeNum.SwapTempValue(oldExpectedStreamingNodeNum)
|
||||
metaRoot := paramtable.Get().EtcdCfg.MetaRootPath.GetValue()
|
||||
|
||||
etcdClient, _ := kvfactory.GetEtcdAndPath()
|
||||
channel.ResetStaticPChannelStatsManager()
|
||||
channel.RecoverPChannelStatsManager([]string{})
|
||||
|
||||
streamingNodeManager := mock_manager.NewMockManagerClient(t)
|
||||
streamingNodeManager.EXPECT().WatchNodeChanged(mock.Anything).Return(make(chan struct{}), nil).Maybe()
|
||||
streamingNodeManager.EXPECT().Assign(mock.Anything, mock.Anything).Return(nil).Maybe()
|
||||
streamingNodeManager.EXPECT().Remove(mock.Anything, mock.Anything).Return(nil).Maybe()
|
||||
streamingNodeManager.EXPECT().GetAllStreamingNodes(mock.Anything).Return(map[int64]*types.StreamingNodeInfoWithResourceGroup{}, nil).Maybe()
|
||||
streamingNodeManager.EXPECT().CollectAllStatus(mock.Anything, mock.Anything).Return(map[int64]*types.StreamingNodeStatus{}, nil).Maybe()
|
||||
|
||||
catalog := mock_metastore.NewMockStreamingCoordCataLog(t)
|
||||
s := sessionutil.NewMockSession(t)
|
||||
s.EXPECT().GetRegisteredRevision().Return(int64(1))
|
||||
resource.InitForTest(
|
||||
resource.OptETCD(etcdClient),
|
||||
resource.OptStreamingCatalog(catalog),
|
||||
resource.OptStreamingManagerClient(streamingNodeManager),
|
||||
resource.OptSession(s),
|
||||
)
|
||||
catalog.EXPECT().GetCChannel(mock.Anything).Return(&streamingpb.CChannelMeta{Pchannel: "schema-drop-ready-skip-channel"}, nil)
|
||||
catalog.EXPECT().GetVersion(mock.Anything).Return(&streamingpb.StreamingVersion{Version: channel.StreamingVersion300}, nil)
|
||||
catalog.EXPECT().ListPChannel(mock.Anything).Return(nil, nil)
|
||||
catalog.EXPECT().SavePChannels(mock.Anything, mock.Anything).Return(nil).Maybe()
|
||||
catalog.EXPECT().GetReplicateConfiguration(mock.Anything).Return(nil, nil)
|
||||
|
||||
ctx := context.Background()
|
||||
legacyProxyKey := path.Join(metaRoot, sessionutil.DefaultServiceRoot, typeutil.ProxyRole+"-legacy")
|
||||
putProxySession(t, ctx, legacyProxyKey, "2.6.6")
|
||||
defer resource.Resource().ETCD().Delete(context.Background(), legacyProxyKey)
|
||||
|
||||
b, err := balancer.RecoverBalancer(ctx, newStaticChannelProvider())
|
||||
if !assert.NoError(t, err) {
|
||||
return
|
||||
}
|
||||
defer b.Close()
|
||||
|
||||
waitCtx, cancel := context.WithTimeout(ctx, 100*time.Millisecond)
|
||||
defer cancel()
|
||||
assert.NoError(t, b.WaitUntilSchemaDropReady(waitCtx))
|
||||
}
|
||||
|
||||
func TestBalancer_WithRecoveryLag(t *testing.T) {
|
||||
paramtable.Init()
|
||||
etcdClient, _ := kvfactory.GetEtcdAndPath()
|
||||
@@ -574,6 +712,32 @@ func TestBalancer_DynamicChannelProviderClosed(t *testing.T) {
|
||||
b.Close()
|
||||
}
|
||||
|
||||
func putProxySession(t *testing.T, ctx context.Context, key string, version string) {
|
||||
t.Helper()
|
||||
|
||||
raw := sessionutil.SessionRaw{Version: version, ServerID: 1}
|
||||
data, err := json.Marshal(raw)
|
||||
assert.NoError(t, err)
|
||||
_, err = resource.Resource().ETCD().Put(ctx, key, string(data))
|
||||
assert.NoError(t, err)
|
||||
}
|
||||
|
||||
func assertSavedStreamingVersion(t *testing.T, savedVersions <-chan int64, expected int64) {
|
||||
t.Helper()
|
||||
|
||||
for {
|
||||
select {
|
||||
case version := <-savedVersions:
|
||||
if version == expected {
|
||||
return
|
||||
}
|
||||
default:
|
||||
assert.Failf(t, "streaming version was not saved", "expected version %d", expected)
|
||||
return
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// staticChannelProvider is a test helper implementing balancer.ChannelProvider with static channels.
|
||||
type staticChannelProvider struct {
|
||||
channels []string
|
||||
|
||||
@@ -26,6 +26,7 @@ import (
|
||||
const (
|
||||
StreamingVersion260 = 1 // streaming version that since 2.6.0, the streaming based WAL is available.
|
||||
StreamingVersion265 = 2 // streaming version that since 2.6.5, the WAL based DDL is available.
|
||||
StreamingVersion300 = 3 // streaming version that since 3.0.0, schema-drop DDL is available.
|
||||
)
|
||||
|
||||
var ErrChannelNotExist = errors.New("channel not exist")
|
||||
@@ -251,12 +252,12 @@ func (cm *ChannelManager) WaitUntilStreamingEnabled(ctx context.Context) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// IsWALBasedDDLEnabled returns true if the WAL based DDL is enabled.
|
||||
func (cm *ChannelManager) IsWALBasedDDLEnabled() bool {
|
||||
// IsStreamingVersionAtLeast returns true if the persisted streaming version is at least version.
|
||||
func (cm *ChannelManager) IsStreamingVersionAtLeast(version int64) bool {
|
||||
cm.cond.L.Lock()
|
||||
defer cm.cond.L.Unlock()
|
||||
|
||||
return cm.streamingVersion != nil && cm.streamingVersion.Version >= StreamingVersion265
|
||||
return cm.streamingVersion != nil && cm.streamingVersion.Version >= version
|
||||
}
|
||||
|
||||
// ReplicateRole returns the replicate role of the channel manager.
|
||||
@@ -356,17 +357,18 @@ func (cm *ChannelManager) MarkStreamingHasEnabled(ctx context.Context) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
func (cm *ChannelManager) MarkWALBasedDDLEnabled(ctx context.Context) error {
|
||||
// MarkStreamingVersion persists the streaming version after the related cluster-version gate passes.
|
||||
func (cm *ChannelManager) MarkStreamingVersion(ctx context.Context, version int64) error {
|
||||
cm.cond.L.Lock()
|
||||
defer cm.cond.L.Unlock()
|
||||
|
||||
if cm.streamingVersion == nil {
|
||||
return errors.New("streaming service is not enabled, cannot mark WAL based DDL enabled")
|
||||
return errors.New("streaming service is not enabled, cannot mark streaming version")
|
||||
}
|
||||
if cm.streamingVersion.Version >= StreamingVersion265 {
|
||||
if cm.streamingVersion.Version >= version {
|
||||
return nil
|
||||
}
|
||||
cm.streamingVersion.Version = StreamingVersion265
|
||||
cm.streamingVersion.Version = version
|
||||
if err := resource.Resource().StreamingCatalog().SaveVersion(ctx, cm.streamingVersion); err != nil {
|
||||
cm.Logger().Error("failed to save streaming version", zap.Error(err))
|
||||
return err
|
||||
|
||||
@@ -325,6 +325,7 @@ const (
|
||||
TimezoneKey = "timezone"
|
||||
AllowInsertAutoIDKey = "allow_insert_auto_id"
|
||||
DisableFuncRuntimeCheck = "disable_func_runtime_check"
|
||||
MaxFieldIDKey = "max_field_id"
|
||||
|
||||
// query mode
|
||||
QueryModeKey = "query_mode"
|
||||
|
||||
@@ -291,6 +291,7 @@ message AlterCollectionMessageHeader {
|
||||
google.protobuf.FieldMask update_mask = 3;
|
||||
CacheExpirations cache_expirations = 4;
|
||||
repeated int64 flushed_segment_ids = 5; // will be filled by wal shard manager.
|
||||
repeated int64 dropped_field_ids = 6; // field IDs removed by this alter operation, used to cascade-delete indexes in ack callback.
|
||||
}
|
||||
|
||||
// AlterCollectionMessageBody is the body of alter collection message.
|
||||
@@ -826,4 +827,3 @@ message BatchUpdateManifestItem {
|
||||
message BatchUpdateManifestV2ColumnGroups {
|
||||
map<int64, data.FieldBinlog> column_groups = 1;
|
||||
}
|
||||
|
||||
|
||||
+646
-635
File diff suppressed because it is too large
Load Diff
@@ -1326,7 +1326,7 @@ func TestCollectionPropertyTtl(t *testing.T) {
|
||||
err = mc.DropCollectionProperties(ctx, client.NewDropCollectionPropertiesOption(schema.CollectionName, common.CollectionTTLSeconds))
|
||||
common.CheckErr(t, err, true)
|
||||
coll, _ = mc.DescribeCollection(ctx, client.NewDescribeCollectionOption(schema.CollectionName))
|
||||
require.Equal(t, 1, len(coll.Properties))
|
||||
require.NotContains(t, coll.Properties, common.CollectionTTLSeconds)
|
||||
}
|
||||
|
||||
// create collection with property -> alter property -> writing and reading
|
||||
@@ -1395,7 +1395,7 @@ func TestCollectionPropertyMmap(t *testing.T) {
|
||||
err = mc.DropCollectionProperties(ctx, client.NewDropCollectionPropertiesOption(schema.CollectionName, common.MmapEnabled))
|
||||
common.CheckErr(t, err, true)
|
||||
coll, _ = mc.DescribeCollection(ctx, client.NewDescribeCollectionOption(schema.CollectionName))
|
||||
require.Equal(t, 1, len(coll.Properties))
|
||||
require.NotContains(t, coll.Properties, common.MmapEnabled)
|
||||
}
|
||||
|
||||
func TestCollectionFakeProperties(t *testing.T) {
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user