unlock unlimited code runs for $79, limited time →
NOSSLETTER.techLIVE
back to OSSpedia
refactoring

enhance: [2.6] simplify index raw data check

Project: milvus-io/milvus | Language: C++ | Concepts: refactoring | Difficulty: junior
Link: https://github.com/milvus-io/milvus/pull/52032

The problem

The code for checking whether an index has raw data was unnecessarily indirect: it used a static helper function (CheckIndexHasRawData) that simply wrapped a call to IndexFactory::IndexLoadResource and returned a single field. This added extra indirection and made the code less clear.

Why it's non-trivial

A naive fix might just inline the function call, but care must be taken to ensure that all uses of the helper are replaced, and that the logic is still clear and correct. It's also important to keep the code maintainable and avoid duplicating logic.

The approach

1. Remove the unnecessary helper function.

The following static function was deleted from SegmentLoadInfo.cpp and its declaration from SegmentLoadInfo.h:

-bool
-SegmentLoadInfo::CheckIndexHasRawData(const LoadIndexInfo& load_index_info) {
-    auto request = milvus::index::IndexFactory::GetInstance().IndexLoadResource(
-        load_index_info.field_type,
-        load_index_info.element_type,
-        load_index_info.index_engine_version,
-        load_index_info.index_size,
-        load_index_info.index_params,
-        load_index_info.enable_mmap,
-        load_index_info.num_rows,
-        load_index_info.dim);
-
-    return request.has_raw_data;
-}

This function just wrapped a single call and returned one field, so it was redundant.

2. Inline the logic where it was used.

The code in BuildCache() was updated from:

-            // Check if index has raw data before moving
-            if (CheckIndexHasRawData(load_index_info)) {
+            auto request =
+                milvus::index::IndexFactory::GetInstance().IndexLoadResource(
+                    load_index_info.field_type,
+                    load_index_info.element_type,
+                    load_index_info.index_engine_version,
+                    load_index_info.index_size,
+                    load_index_info.index_params,
+                    load_index_info.enable_mmap,
+                    load_index_info.num_rows,
+                    load_index_info.dim);
+            if (request.has_raw_data) {
                 field_index_has_raw_data_.insert(field_id);
             }

Now, the call to IndexLoadResource and the check for has_raw_data are done directly in place, making the logic explicit.

3. Remove the function declaration from the class header.

-    [[nodiscard]] static bool
-    CheckIndexHasRawData(const LoadIndexInfo& load_index_info);

This cleans up the class interface, removing an unnecessary function.

From the review thread

No notable review discussion on this PR.

Takeaways

  • Remove unnecessary abstraction layers when a function simply wraps a single call or field. This simplifies code and makes logic easier to follow.
  • Inline trivial helper logic at the call site to improve clarity, but ensure all uses are updated.
  • Keep class interfaces minimal by removing unused or redundant methods. This improves maintainability and reduces cognitive load.