Milvus support multi index engines #27178

foxspy · 2023-09-18T08:26:38Z

issue: #27177

/kind enhancement

mergify · 2023-09-18T08:52:09Z

@foxspy E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

mergify · 2023-09-18T08:58:26Z

@foxspy ut workflow job failed, comment rerun ut can trigger the job again.

czs007 · 2023-09-19T02:16:20Z

internal/core/build.sh

@@ -82,6 +83,9 @@ while getopts "p:t:s:f:o:ulrcghzme" arg; do
  n)
    BUILD_DISK_ANN="OFF"
    ;;
+  x)


If BUILD_DISK_ANN is no longer in use, then let's remove the associated logic.

No, BUILD_DISK_ANN will be reserved to transfer option to knowhere. But we remove all BUILD_DISK_ANN in segcore because VectorDiskIndex.cpp will process more than DISKANN index.

jiaoew1991 · 2023-09-19T02:27:36Z

Makefile

@@ -37,6 +37,8 @@ MOCKERY_VERSION := 2.32.4
 MOCKERY_OUTPUT := $(shell $(INSTALL_PATH)/mockery --version 2>/dev/null)
 INSTALL_MOCKERY := $(findstring $(MOCKERY_VERSION),$(MOCKERY_OUTPUT))

+build_cardinal = OFF


The parameter name is called "engine_type", which may be better. The options are "knowhere" or "cardinal".

jiaoew1991 · 2023-09-19T02:28:35Z

internal/core/src/index/CMakeLists.txt

@@ -14,15 +14,9 @@ set(INDEX_FILES
        Utils.cpp
        VectorMemIndex.cpp
        IndexFactory.cpp
+        VectorDiskIndex.cpp


Can DISK_ANN be compiled on a Mac?

No, libaio can not be install in mac. For this commit, VectorDiskIndex.cpp will process all vector index create/serialize/deserialize from file, not only DISKANN.

czs007 · 2023-09-19T02:40:10Z

internal/indexnode/task.go

@@ -337,6 +337,11 @@ func (it *indexBuildTask) BuildIndex(ctx context.Context) error {
 		}
 	}

+	if err := buildIndexInfo.AppendIndexEngineVersion(it.req.GetIndexEngineVersion()); err != nil {
+		log.Ctx(ctx).Warn("append index engine version failed")


log the err?

czs007 · 2023-09-19T02:45:17Z

internal/core/thirdparty/knowhere/CMakeLists.txt

@@ -11,7 +11,14 @@
 # or implied. See the License for the specific language governing permissions and limitations under the License.
 #-------------------------------------------------------------------------------

-set( KNOWHERE_VERSION 9aa3e21 )
+set( KNOWHERE_VERSION add_version_support )
+set( GIT_REPOSITORY  "https://github.com/foxspy/knowhere.git")


Please utilize the official repository instead and refrain from linking personal repositories.

knowhere merged and fixed it.

czs007 · 2023-09-19T02:51:57Z

internal/core/src/storage/FileManager.h

@@ -27,6 +27,27 @@
 #include "log/Log.h"

 namespace milvus::storage {
+enum class FileManagerType { MemFile, DiskFile };


It appears that the FileManagerType is not being utilized.

mergify · 2023-09-19T16:30:40Z

@foxspy E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

mergify · 2023-09-19T17:10:22Z

@foxspy ut workflow job failed, comment rerun ut can trigger the job again.

mergify · 2023-09-20T02:46:44Z

@foxspy E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

mergify · 2023-09-20T02:52:01Z

@foxspy ut workflow job failed, comment rerun ut can trigger the job again.

sre-ci-robot · 2023-09-21T04:10:49Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: czs007, foxspy

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [czs007]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

czs007 · 2023-09-21T04:13:35Z

/hold

mergify · 2023-09-21T04:17:45Z

@foxspy E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

foxspy · 2023-09-21T04:55:54Z

/run-cpu-e2e

mergify · 2023-09-21T05:24:16Z

@foxspy E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

foxspy · 2023-09-21T05:58:07Z

/run-cpu-e2e

jiaoew1991 · 2023-09-21T06:01:41Z

internal/core/build.sh

@@ -82,6 +83,9 @@ while getopts "p:t:s:f:o:ulrcghzme" arg; do
  n)
    BUILD_DISK_ANN="OFF"
    ;;
+  x)
+    INDEX_ENGINE="cardinal"


the default INDEX_ENGINE is cardinal ?

jiaoew1991 · 2023-09-21T06:07:33Z

internal/core/src/indexbuilder/index_c.cpp

+        return status;
+    } catch (std::exception& e) {
+        auto status = CStatus();
+        status.error_code = UnexpectedError;


do not use UnexpectedError, use FailureCStatus to handle it

jiaoew1991 · 2023-09-21T06:08:01Z

internal/core/src/segcore/load_index_c.cpp

+        return status;
+    } catch (std::exception& e) {
+        auto status = CStatus();
+        status.error_code = milvus::UnexpectedError;


jiaoew1991 · 2023-09-21T06:10:16Z

internal/core/src/storage/FileManager.h

+        return chunkManagerPtr != nullptr;
+    }
+
+    FieldDataMeta fieldDataMeta;


why not use pointer 🤔？

This is a config object transferred from outside. It is a little strange to use it with pointer.

jiaoew1991 · 2023-09-21T06:12:06Z

pkg/util/paramtable/component_param.go

+
+	p.IndexEngineVersion = ParamItem{
+		Key:          "common.indexEngineVersion",
+		Version:      "2.3.0",


shoud be 2.3.2 or later

mergify · 2023-09-21T06:31:22Z

@foxspy E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

mergify · 2023-09-21T07:16:36Z

@foxspy ut workflow job failed, comment rerun ut can trigger the job again.

mergify · 2023-09-21T07:35:16Z

@foxspy E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

foxspy · 2023-09-21T07:48:32Z

/run-cpu-e2e

mergify · 2023-09-21T10:15:00Z

@foxspy E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

Signed-off-by: xianliang <xianliang.li@zilliz.com> Co-authored-by: longjiquan <jiquan.long@zilliz.com>

mergify · 2023-09-21T11:53:21Z

@foxspy E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

foxspy · 2023-09-21T14:52:59Z

/run-cpu-e2e

czs007 · 2023-09-22T01:58:58Z

/unhold

czs007 · 2023-09-22T01:59:09Z

/lgtm

sre-ci-robot added area/compilation area/internal-api area/test sig/testing labels Sep 18, 2023

sre-ci-robot requested review from bigsheeper and congqixia September 18, 2023 08:26

sre-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines. label Sep 18, 2023

foxspy changed the title ~~milvus support multi index engine~~ Milvus support multi index engine Sep 18, 2023

mergify bot added the dco-passed DCO check passed. label Sep 18, 2023

foxspy changed the title ~~Milvus support multi index engine~~ Milvus support multi index engines Sep 18, 2023

foxspy force-pushed the multi_index_engine_support branch from 02619a1 to 3f4f077 Compare September 18, 2023 13:06

czs007 reviewed Sep 19, 2023

View reviewed changes

jiaoew1991 reviewed Sep 19, 2023

View reviewed changes

czs007 reviewed Sep 19, 2023

View reviewed changes

foxspy force-pushed the multi_index_engine_support branch 4 times, most recently from 83960aa to 1c9775c Compare September 19, 2023 16:02

foxspy force-pushed the multi_index_engine_support branch from 1c9775c to b008d27 Compare September 20, 2023 02:39

foxspy force-pushed the multi_index_engine_support branch 3 times, most recently from 128b4c8 to 798e8df Compare September 20, 2023 12:02

sre-ci-robot added the lgtm label Sep 21, 2023

sre-ci-robot added the approved label Sep 21, 2023

sre-ci-robot added the do-not-merge/hold label Sep 21, 2023

foxspy force-pushed the multi_index_engine_support branch from 2b43f25 to 66b0331 Compare September 21, 2023 04:16

sre-ci-robot removed the lgtm label Sep 21, 2023

jiaoew1991 reviewed Sep 21, 2023

View reviewed changes

foxspy force-pushed the multi_index_engine_support branch from 66b0331 to d17367d Compare September 21, 2023 06:52

foxspy force-pushed the multi_index_engine_support branch from d17367d to 0c53c5a Compare September 21, 2023 09:50

milvus support multi index engine

eda1885

Signed-off-by: xianliang <xianliang.li@zilliz.com> Co-authored-by: longjiquan <jiquan.long@zilliz.com>

foxspy force-pushed the multi_index_engine_support branch from 0c53c5a to eda1885 Compare September 21, 2023 11:33

mergify bot added the ci-passed label Sep 21, 2023

sre-ci-robot removed the do-not-merge/hold label Sep 22, 2023

sre-ci-robot added the lgtm label Sep 22, 2023

sre-ci-robot merged commit 370b6fd into milvus-io:master Sep 22, 2023

foxspy mentioned this pull request Nov 7, 2023

[2.3] Publish cardinal for milvus 2.3.3 #28254

Merged

Milvus support multi index engines #27178

Milvus support multi index engines #27178

Conversation

foxspy commented Sep 18, 2023 • edited Loading

mergify bot commented Sep 18, 2023

mergify bot commented Sep 18, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mergify bot commented Sep 19, 2023

mergify bot commented Sep 19, 2023

mergify bot commented Sep 20, 2023

mergify bot commented Sep 20, 2023

sre-ci-robot commented Sep 21, 2023

czs007 commented Sep 21, 2023

mergify bot commented Sep 21, 2023

foxspy commented Sep 21, 2023

mergify bot commented Sep 21, 2023

foxspy commented Sep 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mergify bot commented Sep 21, 2023

mergify bot commented Sep 21, 2023

mergify bot commented Sep 21, 2023

foxspy commented Sep 21, 2023

mergify bot commented Sep 21, 2023

mergify bot commented Sep 21, 2023

foxspy commented Sep 21, 2023

czs007 commented Sep 22, 2023

czs007 commented Sep 22, 2023

foxspy commented Sep 18, 2023 •

edited

Loading