furyhawk/milvus
1
0
Fork 0
mirror of https://github.com/milvus-io/milvus.git synced 2026-07-21 02:05:41 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: skip raw vector fielddata on storage-v3 load when index has raw data (#51541)

Browse Source 
issue: #51299

## Problem

storage-v3 (loon/vortex) sealed segments load the raw vector column
resident **on top of** a vector index that already carries the raw data
(`HasRawData=true`), roughly **doubling** the per-segment disk footprint
of DiskANN / IVF_FLAT / HNSW(with-raw) segments. On the 100M / 128-dim
DiskANN benchmark (1 replica, 100Gi querynode ephemeral-storage) this
pushes a querynode over the cap and `load_collection` fails with
`segment request resource failed[resourceType=Disk]`.

The v2 binlog path already skips the raw fielddata when the index has
raw data (`ChunkedSegmentSealedImpl`: *"skip fielddata because index has
raw data"*); the v3 manifest path in
`SegmentLoadInfo::ComputeDiffColumnGroups` only marked the column
**lazy**, and `LoadColumnGroup(eager_load=false)` still materialized a
`ProxyChunkColumn` and set `field_data_ready`, so the raw column stayed
resident anyway.

## Fix

In `ComputeDiffColumnGroups`, when a field's index carries raw data and
`prefer_field_data_when_index_has_raw_data == false`, drop the raw
column from the load set entirely (mirroring the v2 binlog path) instead
of lazy-loading it. This covers vector fields and other indexed scalars
(retrieve/filter read the raw value from the index).

The **primary key is excluded**: sorted segments navigate rows through
the pk column (`num_rows_until_chunk` / `get_chunk_by_offset`) and a pk
scalar index is not registered in `scalar_indexings`, so the pk raw
column must stay resident — otherwise a `ColumnExpr` on the pk falls
back to a column scan (`use_index_data_ == false`) and crashes (`field
100 must exist when getting rows until chunk`, regressed go-sdk
`TestCreateSortedScalarIndex`).

The stale-copy drop is scoped to the real no-raw-index -> raw-index
reopen transition. `prefer_field_data` remains the explicit opt-in to
keep both resident.

---------

Signed-off-by: xiaofanluan <xf@hjjaq.com>
Signed-off-by: Li Liu <li.liu@zilliz.com>
Co-authored-by: xiaofanluan <xf@hjjaq.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Li Liu <li.liu@zilliz.com>
This commit is contained in:
James
2026-07-20 22:36:42 +08:00
committed by GitHub
co-authored by xiaofanluan Claude Opus 4.8 Li Liu
parent e8b9dbff2f
commit 68c58fee03
3 changed files with 165 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/core/src/index/IndexFactory.cpp
+9 -1
View File
@@ -215,8 +215,16 @@ ResolveHybridInternalIndexType(
bool
IndexFactory::CanUseIndexRawDataForField(DataType field_type,
bool has_raw_data) {
// ARRAY and JSON indexes only index a projection of the value (array
// elements / a JSON path) and cannot reconstruct the whole raw value, so
// their index is never a stand-in for the raw column. This mirrors the
// segment runtime contract (HasRawDataFromState returns column-based
// field_data_ready for JSON), so no loader path treats a JSON index as
// raw-serving and skips its raw column. VECTOR_ARRAY indexes may carry raw
// vector payloads, but the field column is still needed for struct offsets
// and parent-row validity.
return has_raw_data && field_type != DataType::ARRAY &&
field_type != DataType::VECTOR_ARRAY;
field_type != DataType::VECTOR_ARRAY && field_type != DataType::JSON;
}
template <typename T>
internal/core/src/segcore/SegmentLoadInfo.cpp
+31 -8
View File
@@ -548,6 +548,32 @@ SegmentLoadInfo::ComputeDiffColumnGroups(LoadDiff& diff,
bool is_replace_field = was_default_filled || files_changed;
bool index_has_raw_data =
new_info.field_index_has_raw_data_.count(fid) > 0;
bool is_vector_field = IsVectorDataType(
new_info.schema_->operator[](fid).get_data_type());
// When a VECTOR field's index already carries the raw data
// (IVF_FLAT / DiskANN / HNSW-with-raw), the separate raw vector
// column is redundant and must NOT be made resident — neither eager
// nor "lazy" (the loon lazy path still materializes a
// ProxyChunkColumn and sets field_data_ready, so it stays on disk on
// top of the index, ~doubling the footprint). retrieve reconstructs
// the vector from the index (get_vector). Restricted to vector
// fields on purpose: scalar / JSON / ARRAY raw columns stay resident
// because their index-read paths do not cover every consumer
// (nullable Reverse_Lookup, column-scan-only exprs like TIMESTAMPTZ
// arith, etc.) — those are tracked separately. prefer_field_data is
// the explicit opt-in to keep both resident; system fields load.
if (field_id >= START_USER_FIELDID && is_vector_field &&
index_has_raw_data && !prefer_field_data) {
// Free a stale resident copy on the no-raw-index -> raw-index
// reopen transition (the raw column was loaded because the
// current index had no raw data; now the index carries it).
// Steady state (already skipped, never resident) drops nothing.
if (cur_iter != cur_field_to_files.end() &&
field_index_has_raw_data_.count(fid) == 0) {
diff.field_data_to_drop.emplace(field_id);
}
continue;
}
// Eager-load when: system field, OR schema says load AND
// (we want to keep field data alongside index, OR index can't serve raw).
bool should_eager_load =
@@ -568,15 +594,12 @@ SegmentLoadInfo::ComputeDiffColumnGroups(LoadDiff& diff,
} else {
lazy_replace_fields.emplace_back(field_id);
}
} else {
// 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(fid) > 0 &&
field_index_has_raw_data_.count(fid) == 0) {
lazy_replace_fields.emplace_back(field_id);
}
}
// No else: a field at the same position whose index gained raw
// data (the no-raw-index -> raw-index transition) is handled by the
// early "skip raw column" branch above, which drops the stale
// resident copy instead of lazifying it (lazy still keeps it on
// disk on top of the index — the double-footprint bug this fixes).
}
if (!fields.empty()) {
diff.column_groups_to_load.emplace_back(i, fields);
internal/core/src/segcore/SegmentLoadInfoTest.cpp
+125
View File
@@ -31,6 +31,7 @@
#include "common/protobuf_utils.h"
#include "filemanager/InputStream.h"
#include "gtest/gtest.h"
#include "index/IndexFactory.h"
#include "index/Meta.h"
#include "knowhere/comp/index_param.h"
#include "pb/common.pb.h"
@@ -3814,3 +3815,127 @@ TEST_F(SegmentLoadInfoTest, ComputeDiffBinlogsDroppedField) {
// 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.
// ==================== storage-v3 raw-index skip (vector-only) ================
// Regression tests for the DiskANN double-footprint fix (PR #51541): on
// storage-v3 the skip is restricted to VECTOR fields — a vector index
// (IVF_FLAT / DiskANN / HNSW-with-raw) reconstructs the raw vector at retrieve
// (get_vector), so its raw column is dropped. Scalar / JSON / ARRAY raw columns
// always stay resident (their index-read paths do not cover every consumer).
// The vector skip + retrieve-from-index correctness is exercised end-to-end by
// the storage-v3 e2e / go-sdk suites; these unit tests pin the LoadDiff
// classification, in particular that scalars are NOT skipped.
// CanUseIndexRawDataForField additionally excludes JSON/ARRAY/VECTOR_ARRAY at
// the source.
namespace {
// True if `field_id` appears in any (group_index, fields) entry.
bool
ColumnGroupFieldPresent(
const std::vector<std::pair<int, std::vector<FieldId>>>& groups,
int64_t field_id) {
for (const auto& [group_index, fields] : groups) {
for (const auto& f : fields) {
if (f.get() == field_id) {
return true;
}
}
}
return false;
}
// Attach a scalar index (index_type) to a field so BuildCache populates
// field_index_has_raw_data_ (ASCENDING_SORT/STL_SORT => has raw data,
// INVERTED => no raw data).
void
AddScalarIndex(proto::segcore::SegmentLoadInfo& proto,
int64_t field_id,
int64_t index_id,
const std::string& index_type) {
auto* idx = proto.add_index_infos();
idx->set_fieldid(field_id);
idx->set_indexid(index_id);
idx->add_index_file_paths("/idx/" + std::to_string(field_id));
auto* param = idx->add_index_params();
param->set_key("index_type");
param->set_value(index_type);
}
} // namespace
TEST_F(SegmentLoadInfoTest,
ComputeDiffColumnGroupDoesNotSkipScalarRawDataIndex) {
// The skip is vector-only. A scalar field (108, INT64) with a raw-data
// index (STL_SORT) must NOT be skipped: its raw column stays resident
// (loaded) and is never dropped, because scalar index-read paths do not
// cover every consumer (nullable Reverse_Lookup, column-scan-only exprs).
auto new_proto = MakeManifestProto("/manifest/new");
AddScalarIndex(new_proto, 108, 6001, milvus::index::ASCENDING_SORT);
SegmentLoadInfo current_info(MakeManifestProto("/manifest/old"), schema_);
current_info.SetColumnGroupsForTesting(MakeColumnGroups({}));
SegmentLoadInfo new_info(new_proto, schema_);
new_info.SetColumnGroupsForTesting(
MakeColumnGroups({{{108}, {"/cg/108.parquet"}}}));
auto diff = current_info.ComputeDiff(new_info);
bool loaded = ColumnGroupFieldPresent(diff.column_groups_to_load, 108) ||
ColumnGroupFieldPresent(diff.column_groups_to_lazyload, 108);
EXPECT_TRUE(loaded)
<< "scalar raw column must stay resident (skip is vector-only)";
EXPECT_EQ(diff.field_data_to_drop.count(FieldId(108)), 0u)
<< "scalar raw column must not be dropped";
}
TEST_F(SegmentLoadInfoTest, ComputeDiffColumnGroupNeverSkipsPrimaryKey) {
// The pk (field 100) column must stay resident even when its index carries
// raw data: sorted-segment row navigation (num_rows_until_chunk /
// get_chunk_by_offset) reads the pk column directly, and a pk scalar index
// is not registered for expression index-reads.
auto new_proto = MakeManifestProto("/manifest/new");
AddScalarIndex(new_proto, 100, 6002, milvus::index::ASCENDING_SORT);
SegmentLoadInfo current_info(MakeManifestProto("/manifest/old"), schema_);
current_info.SetColumnGroupsForTesting(MakeColumnGroups({}));
SegmentLoadInfo new_info(new_proto, schema_);
new_info.SetColumnGroupsForTesting(
MakeColumnGroups({{{100}, {"/cg/100.parquet"}}}));
auto diff = current_info.ComputeDiff(new_info);
// pk must NOT be skipped: it lands in a load list (lazy, since its index
// reports raw data, but still resident).
bool pk_loaded =
ColumnGroupFieldPresent(diff.column_groups_to_load, 100) ||
ColumnGroupFieldPresent(diff.column_groups_to_lazyload, 100);
EXPECT_TRUE(pk_loaded)
<< "primary key column must load even when its index has raw data";
}
TEST(IndexFactoryRawDataTest,
CanUseIndexRawDataForFieldExcludesNonSubstitutableTypes) {
// JSON and ARRAY indexes only index a projection (a JSON path / array
// elements). VECTOR_ARRAY currently keeps its field column for struct
// offsets and parent-row validity. None of them may substitute for the raw
// column, regardless of the index's own has_raw_data flag.
EXPECT_FALSE(milvus::index::IndexFactory::CanUseIndexRawDataForField(
DataType::JSON, true));
EXPECT_FALSE(milvus::index::IndexFactory::CanUseIndexRawDataForField(
DataType::ARRAY, true));
EXPECT_FALSE(milvus::index::IndexFactory::CanUseIndexRawDataForField(
DataType::VECTOR_ARRAY, true));
// Other field types pass the index's has_raw_data through unchanged.
EXPECT_TRUE(milvus::index::IndexFactory::CanUseIndexRawDataForField(
DataType::INT64, true));
EXPECT_TRUE(milvus::index::IndexFactory::CanUseIndexRawDataForField(
DataType::VECTOR_FLOAT, true));
EXPECT_TRUE(milvus::index::IndexFactory::CanUseIndexRawDataForField(
DataType::VARCHAR, true));
// has_raw_data == false is always false.
EXPECT_FALSE(milvus::index::IndexFactory::CanUseIndexRawDataForField(
DataType::INT64, false));
EXPECT_FALSE(milvus::index::IndexFactory::CanUseIndexRawDataForField(
DataType::JSON, false));
}