From e16ac72a25f0a3a56144f6768c9e9726a949743f Mon Sep 17 00:00:00 2001 From: starlord Date: Tue, 17 Sep 2019 17:37:31 +0800 Subject: [PATCH 1/4] MS-558 refine status code Former-commit-id: cb176cf273ec66e1f7f5e89e9ba4c39cabc511ea --- cpp/CHANGELOG.md | 1 + cpp/src/db/DB.h | 5 +- cpp/src/db/engine/EngineFactory.h | 3 +- cpp/src/db/engine/ExecutionEngine.h | 2 +- cpp/src/db/insert/MemManager.h | 2 +- cpp/src/db/insert/MemManagerImpl.h | 4 +- cpp/src/db/insert/MemTable.h | 2 +- cpp/src/db/insert/MemTableFile.h | 4 +- cpp/src/db/insert/VectorSource.h | 2 +- cpp/src/db/meta/Meta.h | 3 +- cpp/src/metrics/Metrics.h | 5 +- cpp/src/sdk/CMakeLists.txt | 9 +- .../examples/grpcsimple/src/ClientTest.cpp | 6 +- cpp/src/sdk/grpc/ClientProxy.cpp | 2 +- cpp/src/sdk/include/Status.h | 349 ++---------------- cpp/src/sdk/interface/Status.cpp | 167 ++++----- cpp/src/sdk/util/Exception.h | 44 --- cpp/src/server/grpc_impl/GrpcRequestTask.cpp | 28 +- cpp/src/{db => utils}/Status.cpp | 81 ++-- cpp/src/{db => utils}/Status.h | 51 +-- cpp/unittest/db/CMakeLists.txt | 4 +- cpp/unittest/db/db_tests.cpp | 16 +- cpp/unittest/db/mem_test.cpp | 16 +- cpp/unittest/db/misc_test.cpp | 39 +- cpp/unittest/db/mysql_db_test.cpp | 8 +- cpp/unittest/metrics/CMakeLists.txt | 4 +- cpp/unittest/metrics/metrics_test.cpp | 6 +- cpp/unittest/metrics/utils.h | 2 +- cpp/unittest/scheduler/CMakeLists.txt | 4 +- cpp/unittest/server/CMakeLists.txt | 4 +- cpp/unittest/server/util_test.cpp | 42 +++ 31 files changed, 301 insertions(+), 614 deletions(-) delete mode 100644 cpp/src/sdk/util/Exception.h rename cpp/src/{db => utils}/Status.cpp (64%) rename cpp/src/{db => utils}/Status.h (55%) diff --git a/cpp/CHANGELOG.md b/cpp/CHANGELOG.md index de7d8306fd..1f0aedef85 100644 --- a/cpp/CHANGELOG.md +++ b/cpp/CHANGELOG.md @@ -11,6 +11,7 @@ Please mark all change in change log and use the ticket from JIRA. - MS-553 - Refine cache code - MS-557 - Merge Log.h - MS-556 - Add Job Definition in Scheduler +- MS-558 - Refine status code ## New Feature diff --git a/cpp/src/db/DB.h b/cpp/src/db/DB.h index 1de424ed78..5fe6df491f 100644 --- a/cpp/src/db/DB.h +++ b/cpp/src/db/DB.h @@ -15,12 +15,13 @@ // specific language governing permissions and limitations // under the License. + #pragma once #include "Options.h" -#include "meta/Meta.h" -#include "Status.h" #include "Types.h" +#include "meta/Meta.h" +#include "utils/Status.h" #include #include diff --git a/cpp/src/db/engine/EngineFactory.h b/cpp/src/db/engine/EngineFactory.h index 906260b966..c3fb41adde 100644 --- a/cpp/src/db/engine/EngineFactory.h +++ b/cpp/src/db/engine/EngineFactory.h @@ -15,10 +15,11 @@ // specific language governing permissions and limitations // under the License. + #pragma once -#include "db/Status.h" #include "ExecutionEngine.h" +#include "utils/Status.h" namespace zilliz { namespace milvus { diff --git a/cpp/src/db/engine/ExecutionEngine.h b/cpp/src/db/engine/ExecutionEngine.h index 417d002a21..5868f15d33 100644 --- a/cpp/src/db/engine/ExecutionEngine.h +++ b/cpp/src/db/engine/ExecutionEngine.h @@ -17,7 +17,7 @@ #pragma once -#include "db/Status.h" +#include "utils/Status.h" #include #include diff --git a/cpp/src/db/insert/MemManager.h b/cpp/src/db/insert/MemManager.h index de6ee85ed1..37f0548646 100644 --- a/cpp/src/db/insert/MemManager.h +++ b/cpp/src/db/insert/MemManager.h @@ -18,7 +18,7 @@ #pragma once -#include "db/Status.h" +#include "utils/Status.h" #include "db/Types.h" #include diff --git a/cpp/src/db/insert/MemManagerImpl.h b/cpp/src/db/insert/MemManagerImpl.h index 9b503216a7..51e8f1044e 100644 --- a/cpp/src/db/insert/MemManagerImpl.h +++ b/cpp/src/db/insert/MemManagerImpl.h @@ -18,10 +18,10 @@ #pragma once -#include "db/meta/Meta.h" #include "MemTable.h" -#include "db/Status.h" #include "MemManager.h" +#include "db/meta/Meta.h" +#include "utils/Status.h" #include #include diff --git a/cpp/src/db/insert/MemTable.h b/cpp/src/db/insert/MemTable.h index a443186900..def4a6dcca 100644 --- a/cpp/src/db/insert/MemTable.h +++ b/cpp/src/db/insert/MemTable.h @@ -18,9 +18,9 @@ #pragma once -#include "db/Status.h" #include "MemTableFile.h" #include "VectorSource.h" +#include "utils/Status.h" #include diff --git a/cpp/src/db/insert/MemTableFile.h b/cpp/src/db/insert/MemTableFile.h index d4b11cda7a..5ac3e15ad2 100644 --- a/cpp/src/db/insert/MemTableFile.h +++ b/cpp/src/db/insert/MemTableFile.h @@ -18,10 +18,10 @@ #pragma once -#include "db/Status.h" -#include "db/meta/Meta.h" #include "VectorSource.h" +#include "db/meta/Meta.h" #include "db/engine/ExecutionEngine.h" +#include "utils/Status.h" namespace zilliz { diff --git a/cpp/src/db/insert/VectorSource.h b/cpp/src/db/insert/VectorSource.h index 5c44f580d2..a0292181ce 100644 --- a/cpp/src/db/insert/VectorSource.h +++ b/cpp/src/db/insert/VectorSource.h @@ -18,10 +18,10 @@ #pragma once -#include "db/Status.h" #include "db/meta/Meta.h" #include "db/IDGenerator.h" #include "db/engine/ExecutionEngine.h" +#include "utils/Status.h" namespace zilliz { diff --git a/cpp/src/db/meta/Meta.h b/cpp/src/db/meta/Meta.h index 755e0170f2..ee4972ae6b 100644 --- a/cpp/src/db/meta/Meta.h +++ b/cpp/src/db/meta/Meta.h @@ -15,12 +15,13 @@ // specific language governing permissions and limitations // under the License. + #pragma once #include "MetaTypes.h" #include "db/Options.h" -#include "db/Status.h" #include "db/Types.h" +#include "utils/Status.h" #include #include diff --git a/cpp/src/metrics/Metrics.h b/cpp/src/metrics/Metrics.h index e298af7acf..30d6e68f52 100644 --- a/cpp/src/metrics/Metrics.h +++ b/cpp/src/metrics/Metrics.h @@ -15,6 +15,7 @@ // specific language governing permissions and limitations // under the License. + #pragma once #include "MetricBase.h" @@ -63,7 +64,7 @@ protected: //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// class CollectInsertMetrics : CollectMetricsBase { public: - CollectInsertMetrics(size_t n, engine::Status& status) : n_(n), status_(status) { + CollectInsertMetrics(size_t n, Status& status) : n_(n), status_(status) { } ~CollectInsertMetrics() { @@ -87,7 +88,7 @@ public: private: size_t n_; - engine::Status& status_; + Status& status_; }; //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/cpp/src/sdk/CMakeLists.txt b/cpp/src/sdk/CMakeLists.txt index 6c3a68b8b8..5d6a1ea465 100644 --- a/cpp/src/sdk/CMakeLists.txt +++ b/cpp/src/sdk/CMakeLists.txt @@ -20,8 +20,6 @@ aux_source_directory(interface interface_files) -aux_source_directory(util util_files) - include_directories(/usr/include) include_directories(include) include_directories(/usr/local/include) @@ -42,12 +40,15 @@ set(grpc_service_files add_library(milvus_sdk STATIC ${interface_files} ${grpc_client_files} - ${util_files} ${grpc_service_files} ) target_link_libraries(milvus_sdk - ${third_party_libs} + ${client_grpc_lib} + bzip2 + lz4 + snappy + zlib ) install(TARGETS milvus_sdk DESTINATION lib) diff --git a/cpp/src/sdk/examples/grpcsimple/src/ClientTest.cpp b/cpp/src/sdk/examples/grpcsimple/src/ClientTest.cpp index abfc92cc0b..f0f7cf5292 100644 --- a/cpp/src/sdk/examples/grpcsimple/src/ClientTest.cpp +++ b/cpp/src/sdk/examples/grpcsimple/src/ClientTest.cpp @@ -36,10 +36,10 @@ const std::string TABLE_NAME = GetTableName(); constexpr int64_t TABLE_DIMENSION = 512; constexpr int64_t TABLE_INDEX_FILE_SIZE = 1024; constexpr int64_t BATCH_ROW_COUNT = 100000; -constexpr int64_t NQ = 100; -constexpr int64_t TOP_K = 1; +constexpr int64_t NQ = 5; +constexpr int64_t TOP_K = 10; constexpr int64_t SEARCH_TARGET = 5000; //change this value, result is different -constexpr int64_t ADD_VECTOR_LOOP = 1; +constexpr int64_t ADD_VECTOR_LOOP = 10; constexpr int64_t SECONDS_EACH_HOUR = 3600; #define BLOCK_SPLITER std::cout << "===========================================" << std::endl; diff --git a/cpp/src/sdk/grpc/ClientProxy.cpp b/cpp/src/sdk/grpc/ClientProxy.cpp index 56ba1e2de6..2a87f7f929 100644 --- a/cpp/src/sdk/grpc/ClientProxy.cpp +++ b/cpp/src/sdk/grpc/ClientProxy.cpp @@ -51,7 +51,7 @@ ClientProxy::Connect(const ConnectParam ¶m) { Status ClientProxy::Connect(const std::string &uri) { if (!UriCheck(uri)) { - return Status::Invalid("Invalid uri"); + return Status(StatusCode::InvalidAgument, "Invalid uri"); } size_t index = uri.find_first_of(':', 0); diff --git a/cpp/src/sdk/include/Status.h b/cpp/src/sdk/include/Status.h index 6dbc762c0a..145d09d47a 100644 --- a/cpp/src/sdk/include/Status.h +++ b/cpp/src/sdk/include/Status.h @@ -19,7 +19,6 @@ #pragma once #include -#include /** \brief Milvus SDK namespace */ @@ -30,345 +29,61 @@ namespace milvus { */ enum class StatusCode { OK = 0, -// system error section + // system error section UnknownError = 1, NotSupported, NotConnected, -// function error section + // function error section InvalidAgument = 1000, RPCFailed, ServerFailed, }; +using ErrorCode = StatusCode; + /** * @brief Status for SDK interface return */ class Status { public: - /** - * @brief Status - * - * Default constructor. - * - */ - Status() = default; + Status(ErrorCode code, const std::string &msg); + Status(); + ~Status(); - /** - * @brief Status - * - * Destructor. - * - */ - ~Status() noexcept; + Status(const Status &s); - /** - * @brief Status - * - * Constructor - * - * @param code, status code. - * @param message, status message. - * - */ - Status(StatusCode code, const std::string &message); + Status & + operator=(const Status &s); - /** - * @brief Status - * - * Copy constructor - * - * @param status, status to be copied. - * - */ - inline - Status(const Status &status); + Status(Status &&s); - /** - * @brief Status - * - * Assignment operator - * - * @param status, status to be copied. - * @return, the status is assigned. - * - */ - Status - &operator=(const Status &s); + Status & + operator=(Status &&s); - /** - * @brief Status - * - * Move constructor - * - * @param status, status to be moved. - * - */ - Status(Status &&s) noexcept ; - - /** - * @brief Status - * - * Move assignment operator - * - * @param status, status to be moved. - * @return, the status is moved. - * - */ - Status - &operator=(Status &&s) noexcept; - - /** - * @brief Status - * - * AND operator - * - * @param status, status to be AND. - * @return, the status after AND operation. - * - */ - inline - Status operator&(const Status &s) const noexcept; - - /** - * @brief Status - * - * AND operator - * - * @param status, status to be AND. - * @return, the status after AND operation. - * - */ - inline - Status operator&(Status &&s) const noexcept; - - /** - * @brief Status - * - * AND operator - * - * @param status, status to be AND. - * @return, the status after AND operation. - * - */ - inline - Status &operator&=(const Status &s) noexcept; - - /** - * @brief Status - * - * AND operator - * - * @param status, status to be AND. - * @return, the status after AND operation. - * - */ - inline - Status &operator&=(Status &&s) noexcept; - - /** - * @brief OK - * - * static OK status constructor - * - * @return, the status with OK. - * - */ - static - Status OK() { return Status(); } - - /** - * @brief OK - * - * static OK status constructor with a specific message - * - * @param, serveral specific messages - * @return, the status with OK. - * - */ - template static Status - OK(Args &&... args) { - return Status(StatusCode::OK, MessageBuilder(std::forward(args)...)); + OK() { return Status(); } + + bool + ok() const { return state_ == nullptr || code() == StatusCode::OK; } + + std::string + ToString() const; + + ErrorCode + code() const { + return (state_ == nullptr) ? StatusCode::OK : *(ErrorCode*)(state_); } -/** - * @brief Invalid - * - * static Invalid status constructor with a specific message - * - * @param, serveral specific messages - * @return, the status with Invalid. - * - */ -template -static Status -Invalid(Args &&... args) { - return Status(StatusCode::InvalidAgument, - MessageBuilder(std::forward(args)...)); -} +private: + inline void + CopyFrom(const Status &s); -/** - * @brief Unknown Error - * - * static unknown error status constructor with a specific message - * - * @param, serveral specific messages - * @return, the status with unknown error. - * - */ -template -static Status -UnknownError(Args &&... args) { - return Status(StatusCode::UnknownError, MessageBuilder(std::forward(args)...)); -} - -/** - * @brief not supported Error - * - * static not supported status constructor with a specific message - * - * @param, serveral specific messages - * @return, the status with not supported error. - * - */ -template -static Status -NotSupported(Args &&... args) { - return Status(StatusCode::NotSupported, MessageBuilder(std::forward(args)...)); -} - -/** - * @brief ok - * - * Return true iff the status indicates success. - * - * @return, if the status indicates success. - * - */ -bool -ok() const { return (state_ == nullptr); } - -/** - * @brief IsInvalid - * - * Return true iff the status indicates invalid. - * - * @return, if the status indicates invalid. - * - */ -bool -IsInvalid() const { return code() == StatusCode::InvalidAgument; } - -/** - * @brief IsUnknownError - * - * Return true iff the status indicates unknown error. - * - * @return, if the status indicates unknown error. - * - */ -bool -IsUnknownError() const { return code() == StatusCode::UnknownError; } - -/** - * @brief IsNotSupported - * - * Return true iff the status indicates not supported. - * - * @return, if the status indicates not supported. - * - */ -bool -IsNotSupported() const { return code() == StatusCode::NotSupported; } - -/** - * @brief ToString - * - * Return error message string. - * - * @return, error message string. - * - */ -std::string -ToString() const; - -/** - * @brief CodeAsString - * - * Return a string representation of the status code. - * - * @return, a string representation of the status code. - * - */ -std::string -CodeAsString() const; - -/** - * @brief code - * - * Return the StatusCode value attached to this status. - * - * @return, the status code value attached to this status. - * - */ -StatusCode -code() const { return ok() ? StatusCode::OK : state_->code; } - -/** - * @brief message - * - * Return the specific error message attached to this status. - * - * @return, the specific error message attached to this status. - * - */ -std::string -message() const { return ok() ? "" : state_->message; } + inline void + MoveFrom(Status &s); private: -struct State { - StatusCode code; - std::string message; -}; + const char *state_ = nullptr; +}; // Status -// OK status has a `nullptr` state_. Otherwise, `state_` points to -// a `State` structure containing the error code and message. -State *state_ = nullptr; - -void -DeleteState() { - delete state_; - state_ = nullptr; -} - -void -CopyFrom(const Status &s); - -inline void -MoveFrom(Status &s); - -template -static void -MessageBuilderRecursive(std::stringstream &stream, Head &&head) { - stream << head; -} - -template -static void -MessageBuilderRecursive(std::stringstream &stream, Head &&head, Tail &&... tail) { - MessageBuilderRecursive(stream, std::forward(head)); - MessageBuilderRecursive(stream, std::forward(tail)...); -} - -template -static std::string -MessageBuilder(Args &&... args) { - std::stringstream stream; - - MessageBuilderRecursive(stream, std::forward(args)...); - - return stream.str(); -} -}; -} \ No newline at end of file +} //Milvus \ No newline at end of file diff --git a/cpp/src/sdk/interface/Status.cpp b/cpp/src/sdk/interface/Status.cpp index 344d86c12f..b04c6581fe 100644 --- a/cpp/src/sdk/interface/Status.cpp +++ b/cpp/src/sdk/interface/Status.cpp @@ -17,18 +17,66 @@ #include "Status.h" +#include + namespace milvus { -Status::~Status() noexcept { - if (state_ != nullptr) { - delete state_; - state_ = nullptr; - } +constexpr int CODE_WIDTH = sizeof(ErrorCode); + +Status::Status(ErrorCode code, const std::string& msg) { + //4 bytes store code + //4 bytes store message length + //the left bytes store message string + const uint32_t length = (uint32_t)msg.size(); + char* result = new char[length + sizeof(length) + CODE_WIDTH]; + memcpy(result, &code, CODE_WIDTH); + memcpy(result + CODE_WIDTH, &length, sizeof(length)); + memcpy(result + sizeof(length) + CODE_WIDTH, msg.data(), length); + + state_ = result; } -static inline std::ostream &operator<<(std::ostream &os, const Status &x) { - os << x.ToString(); - return os; +Status::Status() + : state_(nullptr) { + +} + +Status::~Status() { + delete state_; +} + +Status::Status(const Status &s) + : state_(nullptr) { + CopyFrom(s); +} + +Status &Status::operator=(const Status &s) { + CopyFrom(s); + return *this; +} + +Status::Status(Status &&s) + : state_(nullptr) { + MoveFrom(s); +} + +Status &Status::operator=(Status &&s) { + MoveFrom(s); + return *this; +} + +void Status::CopyFrom(const Status &s) { + delete state_; + state_ = nullptr; + if(s.state_ == nullptr) { + return; + } + + uint32_t length = 0; + std::memcpy(&length, s.state_ + CODE_WIDTH, sizeof(length)); + int buff_len = length + sizeof(length) + CODE_WIDTH; + state_ = new char[buff_len]; + memcpy((void*)state_, (void*)s.state_, buff_len); } void Status::MoveFrom(Status &s) { @@ -37,106 +85,45 @@ void Status::MoveFrom(Status &s) { s.state_ = nullptr; } -Status::Status(const Status &s) - : state_((s.state_ == nullptr) ? nullptr : new State(*s.state_)) {} - -Status::Status(Status &&s) noexcept { - MoveFrom(s); -} - -Status &Status::operator=(const Status &s) { - if (state_ != s.state_) { - CopyFrom(s); - } - return *this; -} - -Status &Status::operator=(Status &&s) noexcept { - MoveFrom(s); - return *this; -} - -Status Status::operator&(const Status &status) const noexcept { - if (ok()) { - return status; - } else { - return *this; - } -} - -Status Status::operator&(Status &&s) const noexcept { - if (ok()) { - return std::move(s); - } else { - return *this; - } -} - -Status &Status::operator&=(const Status &s) noexcept { - if (ok() && !s.ok()) { - CopyFrom(s); - } - return *this; -} - -Status &Status::operator&=(Status &&s) noexcept { - if (ok() && !s.ok()) { - MoveFrom(s); - } - return *this; -} - -Status::Status(StatusCode code, const std::string &message) { - state_ = new State; - state_->code = code; - state_->message = message; -} - -void Status::CopyFrom(const Status &status) { - delete state_; - if (status.state_ == nullptr) { - state_ = nullptr; - } else { - state_ = new State(*status.state_); - } -} - -std::string Status::CodeAsString() const { +std::string Status::ToString() const { if (state_ == nullptr) { return "OK"; } - const char *type = nullptr; + std::string result; switch (code()) { case StatusCode::OK: - type = "OK"; - break; - case StatusCode::InvalidAgument: - type = "Invalid agument"; + result = "OK "; break; case StatusCode::UnknownError: - type = "Unknown error"; + result = "Unknown error: "; break; case StatusCode::NotSupported: - type = "Not Supported"; + result = "Not supported: "; break; case StatusCode::NotConnected: - type = "Not Connected"; + result = "Not connected: "; + break; + case StatusCode::InvalidAgument: + result = "Invalid agument: "; + break; + case StatusCode::RPCFailed: + result = "Remote call failed: "; + break; + case StatusCode::ServerFailed: + result = "Service error: "; break; default: - type = "Unknown"; + result = "Error code(" + std::to_string((int)code()) + "): "; break; } - return std::string(type); -} -std::string Status::ToString() const { - std::string result(CodeAsString()); - if (state_ == nullptr) { - return result; + uint32_t length = 0; + memcpy(&length, state_ + CODE_WIDTH, sizeof(length)); + if(length > 0) { + result.append(state_ + sizeof(length) + CODE_WIDTH, length); } - result += ": "; - result += state_->message; + return result; } diff --git a/cpp/src/sdk/util/Exception.h b/cpp/src/sdk/util/Exception.h deleted file mode 100644 index f55b8f0632..0000000000 --- a/cpp/src/sdk/util/Exception.h +++ /dev/null @@ -1,44 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -#pragma once - -#include "Status.h" - -#include - -namespace milvus { -class Exception : public std::exception { -public: - Exception(StatusCode error_code, - const std::string &message = std::string()) - : error_code_(error_code), message_(message) {} - -public: - StatusCode error_code() const { - return error_code_; - } - - virtual const char *what() const noexcept { - return message_.c_str(); - } - -private: - StatusCode error_code_; - std::string message_; -}; -} \ No newline at end of file diff --git a/cpp/src/server/grpc_impl/GrpcRequestTask.cpp b/cpp/src/server/grpc_impl/GrpcRequestTask.cpp index 8c7697e1dc..d7b39bea05 100644 --- a/cpp/src/server/grpc_impl/GrpcRequestTask.cpp +++ b/cpp/src/server/grpc_impl/GrpcRequestTask.cpp @@ -170,7 +170,7 @@ CreateTableTask::OnExecute() { table_info.metric_type_ = schema_->metric_type(); //step 3: create table - engine::Status stat = DBWrapper::DB()->CreateTable(table_info); + auto stat = DBWrapper::DB()->CreateTable(table_info); if (!stat.ok()) { //table could exist if(stat.code() == DB_ALREADY_EXIST) { @@ -214,7 +214,7 @@ DescribeTableTask::OnExecute() { //step 2: get table info engine::meta::TableSchema table_info; table_info.table_id_ = table_name_; - engine::Status stat = DBWrapper::DB()->DescribeTable(table_info); + auto stat = DBWrapper::DB()->DescribeTable(table_info); if (!stat.ok()) { return SetError(DB_META_TRANSACTION_FAILED, stat.ToString()); } @@ -261,7 +261,7 @@ CreateIndexTask::OnExecute() { } bool has_table = false; - engine::Status stat = DBWrapper::DB()->HasTable(table_name_, has_table); + auto stat = DBWrapper::DB()->HasTable(table_name_, has_table); if (!stat.ok()) { return SetError(DB_META_TRANSACTION_FAILED, stat.ToString()); } @@ -323,7 +323,7 @@ HasTableTask::OnExecute() { } //step 2: check table existence - engine::Status stat = DBWrapper::DB()->HasTable(table_name_, has_table_); + auto stat = DBWrapper::DB()->HasTable(table_name_, has_table_); if (!stat.ok()) { return SetError(DB_META_TRANSACTION_FAILED, stat.ToString()); } @@ -362,7 +362,7 @@ DropTableTask::OnExecute() { //step 2: check table existence engine::meta::TableSchema table_info; table_info.table_id_ = table_name_; - engine::Status stat = DBWrapper::DB()->DescribeTable(table_info); + auto stat = DBWrapper::DB()->DescribeTable(table_info); if (!stat.ok()) { if (stat.code() == DB_NOT_FOUND) { return SetError(SERVER_TABLE_NOT_EXIST, "Table " + table_name_ + " not exists"); @@ -403,7 +403,7 @@ ShowTablesTask::Create(::grpc::ServerWriter<::milvus::grpc::TableName> *writer) ErrorCode ShowTablesTask::OnExecute() { std::vector schema_array; - engine::Status stat = DBWrapper::DB()->AllTables(schema_array); + auto stat = DBWrapper::DB()->AllTables(schema_array); if (!stat.ok()) { return SetError(DB_META_TRANSACTION_FAILED, stat.ToString()); } @@ -460,7 +460,7 @@ InsertTask::OnExecute() { //step 2: check table existence engine::meta::TableSchema table_info; table_info.table_id_ = insert_param_->table_name(); - engine::Status stat = DBWrapper::DB()->DescribeTable(table_info); + auto stat = DBWrapper::DB()->DescribeTable(table_info); if (!stat.ok()) { if (stat.code() == DB_NOT_FOUND) { return SetError(SERVER_TABLE_NOT_EXIST, @@ -599,7 +599,7 @@ SearchTask::OnExecute() { //step 2: check table existence engine::meta::TableSchema table_info; table_info.table_id_ = table_name_; - engine::Status stat = DBWrapper::DB()->DescribeTable(table_info); + auto stat = DBWrapper::DB()->DescribeTable(table_info); if (!stat.ok()) { if (stat.code() == DB_NOT_FOUND) { return SetError(SERVER_TABLE_NOT_EXIST, "Table " + table_name_ + " not exists"); @@ -746,7 +746,7 @@ CountTableTask::OnExecute() { //step 2: get row count uint64_t row_count = 0; - engine::Status stat = DBWrapper::DB()->GetTableRowCount(table_name_, row_count); + auto stat = DBWrapper::DB()->GetTableRowCount(table_name_, row_count); if (!stat.ok()) { return SetError(DB_META_TRANSACTION_FAILED, stat.ToString()); } @@ -819,7 +819,7 @@ DeleteByRangeTask::OnExecute() { //step 2: check table existence engine::meta::TableSchema table_info; table_info.table_id_ = table_name; - engine::Status stat = DBWrapper::DB()->DescribeTable(table_info); + auto stat = DBWrapper::DB()->DescribeTable(table_info); if (!stat.ok()) { if (stat.code(), DB_NOT_FOUND) { return SetError(SERVER_TABLE_NOT_EXIST, "Table " + table_name + " not exists"); @@ -846,8 +846,8 @@ DeleteByRangeTask::OnExecute() { std::string fname = "/tmp/search_nq_" + this->delete_by_range_param_->table_name() + ".profiling"; ProfilerStart(fname.c_str()); #endif - engine::Status status = DBWrapper::DB()->DeleteTable(table_name, dates); - if (!status.ok()) { + stat = DBWrapper::DB()->DeleteTable(table_name, dates); + if (!stat.ok()) { return SetError(DB_META_TRANSACTION_FAILED, stat.ToString()); } @@ -882,7 +882,7 @@ PreloadTableTask::OnExecute() { } //step 2: check table existence - engine::Status stat = DBWrapper::DB()->PreloadTable(table_name_); + auto stat = DBWrapper::DB()->PreloadTable(table_name_); if (!stat.ok()) { return SetError(DB_META_TRANSACTION_FAILED, stat.ToString()); } @@ -923,7 +923,7 @@ DescribeIndexTask::OnExecute() { //step 2: check table existence engine::TableIndex index; - engine::Status stat = DBWrapper::DB()->DescribeIndex(table_name_, index); + auto stat = DBWrapper::DB()->DescribeIndex(table_name_, index); if (!stat.ok()) { return SetError(DB_META_TRANSACTION_FAILED, stat.ToString()); } diff --git a/cpp/src/db/Status.cpp b/cpp/src/utils/Status.cpp similarity index 64% rename from cpp/src/db/Status.cpp rename to cpp/src/utils/Status.cpp index 0bc51ca59e..0232c823f0 100644 --- a/cpp/src/db/Status.cpp +++ b/cpp/src/utils/Status.cpp @@ -14,15 +14,12 @@ // KIND, either express or implied. See the License for the // specific language governing permissions and limitations // under the License. - -#include -#include -#include #include "Status.h" +#include + namespace zilliz { namespace milvus { -namespace engine { constexpr int CODE_WIDTH = sizeof(ErrorCode); @@ -45,55 +42,87 @@ Status::Status() } Status::~Status() { - delete[] state_; + delete state_; } -const char* Status::CopyState(const char* state) { +Status::Status(const Status &s) + : state_(nullptr) { + CopyFrom(s); +} + +Status &Status::operator=(const Status &s) { + CopyFrom(s); + return *this; +} + +Status::Status(Status &&s) + : state_(nullptr) { + MoveFrom(s); +} + +Status &Status::operator=(Status &&s) { + MoveFrom(s); + return *this; +} + +void Status::CopyFrom(const Status &s) { + delete state_; + state_ = nullptr; + if(s.state_ == nullptr) { + return; + } + uint32_t length = 0; - std::memcpy(&length, state + CODE_WIDTH, sizeof(length)); + std::memcpy(&length, s.state_ + CODE_WIDTH, sizeof(length)); int buff_len = length + sizeof(length) + CODE_WIDTH; - char* result = new char[buff_len]; - memcpy(result, state, buff_len); - return result; + state_ = new char[buff_len]; + memcpy((void*)state_, (void*)s.state_, buff_len); +} + +void Status::MoveFrom(Status &s) { + delete state_; + state_ = s.state_; + s.state_ = nullptr; } std::string Status::ToString() const { - if (state_ == nullptr) return "OK"; - char tmp[32]; - const char* type; + if (state_ == nullptr) { + return "OK"; + } + + std::string result; switch (code()) { case DB_SUCCESS: - type = "OK"; + result = "OK "; break; case DB_ERROR: - type = "Error: "; + result = "Error: "; break; case DB_META_TRANSACTION_FAILED: - type = "DBTransactionError: "; + result = "Database error: "; break; case DB_NOT_FOUND: - type = "NotFound: "; + result = "Not found: "; break; case DB_ALREADY_EXIST: - type = "AlreadyExist: "; + result = "Already exist: "; break; case DB_INVALID_PATH: - type = "InvalidPath: "; + result = "Invalid path: "; break; default: - snprintf(tmp, sizeof(tmp), "Error code(0x%x): ", - static_cast(code())); - type = tmp; + result = "Error code(" + std::to_string(code()) + "): "; break; } - std::string result(type); uint32_t length = 0; memcpy(&length, state_ + CODE_WIDTH, sizeof(length)); - result.append(state_ + sizeof(length) + CODE_WIDTH, length); + if(length > 0) { + result.append(state_ + sizeof(length) + CODE_WIDTH, length); + } + return result; } -} // namespace engine } // namespace milvus } // namespace zilliz diff --git a/cpp/src/db/Status.h b/cpp/src/utils/Status.h similarity index 55% rename from cpp/src/db/Status.h rename to cpp/src/utils/Status.h index e3dae4c0ad..153ff61dc5 100644 --- a/cpp/src/db/Status.h +++ b/cpp/src/utils/Status.h @@ -15,6 +15,7 @@ // specific language governing permissions and limitations // under the License. + #pragma once #include "utils/Error.h" @@ -23,7 +24,6 @@ namespace zilliz { namespace milvus { -namespace engine { class Status { public: @@ -31,51 +31,40 @@ class Status { Status(); ~Status(); - Status(const Status &rhs); + Status(const Status &s); Status & - operator=(const Status &rhs); + operator=(const Status &s); - Status(Status &&rhs) noexcept : state_(rhs.state_) { rhs.state_ = nullptr; } + Status(Status &&s); Status & - operator=(Status &&rhs_) noexcept; + operator=(Status &&s); static Status OK() { return Status(); } - bool ok() const { return state_ == nullptr || code() == DB_SUCCESS; } + bool + ok() const { return state_ == nullptr || code() == 0; } - std::string ToString() const; + std::string + ToString() const; - ErrorCode code() const { - return (state_ == nullptr) ? DB_SUCCESS : *(ErrorCode*)(state_); + ErrorCode + code() const { + return (state_ == nullptr) ? 0 : *(ErrorCode*)(state_); } - private: +private: + inline void + CopyFrom(const Status &s); + + inline void + MoveFrom(Status &s); + +private: const char *state_ = nullptr; - - static const char *CopyState(const char *s); - }; // Status -inline Status::Status(const Status &rhs) { - state_ = (rhs.state_ == nullptr) ? nullptr : CopyState(rhs.state_); -} - -inline Status &Status::operator=(const Status &rhs) { - if (state_ != rhs.state_) { - delete[] state_; - state_ = (rhs.state_ == nullptr) ? nullptr : CopyState(rhs.state_); - } - return *this; -} - -inline Status &Status::operator=(Status &&rhs) noexcept { - std::swap(state_, rhs.state_); - return *this; -} - -} // namespace engine } // namespace milvus } // namespace zilliz diff --git a/cpp/unittest/db/CMakeLists.txt b/cpp/unittest/db/CMakeLists.txt index 00b6f64dbf..29440dcf1a 100644 --- a/cpp/unittest/db/CMakeLists.txt +++ b/cpp/unittest/db/CMakeLists.txt @@ -33,9 +33,9 @@ aux_source_directory(${MILVUS_ENGINE_SRC}/scheduler scheduler_srcs) aux_source_directory(./ test_srcs) set(util_files + ${MILVUS_ENGINE_SRC}/utils/Status.cpp ${MILVUS_ENGINE_SRC}/utils/ValidationUtil.cpp - ${MILVUS_ENGINE_SRC}/utils/easylogging++.cc - ${MILVUS_ENGINE_SRC}/utils/easylogging++.h) + ${MILVUS_ENGINE_SRC}/utils/easylogging++.cc) aux_source_directory(${MILVUS_ENGINE_SRC}/db/scheduler scheduler_files) aux_source_directory(${MILVUS_ENGINE_SRC}/db/scheduler/context scheduler_context_files) diff --git a/cpp/unittest/db/db_tests.cpp b/cpp/unittest/db/db_tests.cpp index 372dd336d6..a1fd14a8f4 100644 --- a/cpp/unittest/db/db_tests.cpp +++ b/cpp/unittest/db/db_tests.cpp @@ -154,7 +154,7 @@ TEST_F(DBTest, CONFIG_TEST) { TEST_F(DBTest, DB_TEST) { engine::meta::TableSchema table_info = BuildTableSchema(); - engine::Status stat = db_->CreateTable(table_info); + auto stat = db_->CreateTable(table_info); engine::meta::TableSchema table_info_get; table_info_get.table_id_ = TABLE_NAME; @@ -230,7 +230,7 @@ TEST_F(DBTest, DB_TEST) { TEST_F(DBTest, SEARCH_TEST) { engine::meta::TableSchema table_info = BuildTableSchema(); - engine::Status stat = db_->CreateTable(table_info); + auto stat = db_->CreateTable(table_info); engine::meta::TableSchema table_info_get; table_info_get.table_id_ = TABLE_NAME; @@ -296,7 +296,7 @@ TEST_F(DBTest, SEARCH_TEST) { TEST_F(DBTest, PRELOADTABLE_TEST) { engine::meta::TableSchema table_info = BuildTableSchema(); - engine::Status stat = db_->CreateTable(table_info); + auto stat = db_->CreateTable(table_info); engine::meta::TableSchema table_info_get; table_info_get.table_id_ = TABLE_NAME; @@ -331,7 +331,7 @@ TEST_F(DBTest, SHUTDOWN_TEST) { db_->Stop(); engine::meta::TableSchema table_info = BuildTableSchema(); - engine::Status stat = db_->CreateTable(table_info); + auto stat = db_->CreateTable(table_info); ASSERT_FALSE(stat.ok()); stat = db_->DescribeTable(table_info); @@ -373,7 +373,7 @@ TEST_F(DBTest, SHUTDOWN_TEST) { TEST_F(DBTest, INDEX_TEST) { engine::meta::TableSchema table_info = BuildTableSchema(); - engine::Status stat = db_->CreateTable(table_info); + auto stat = db_->CreateTable(table_info); int64_t nb = VECTOR_COUNT; std::vector xb; @@ -403,7 +403,7 @@ TEST_F(DBTest, INDEX_TEST) { TEST_F(DBTest2, ARHIVE_DISK_CHECK) { engine::meta::TableSchema table_info = BuildTableSchema(); - engine::Status stat = db_->CreateTable(table_info); + auto stat = db_->CreateTable(table_info); std::vector table_schema_array; stat = db_->AllTables(table_schema_array); @@ -446,7 +446,7 @@ TEST_F(DBTest2, ARHIVE_DISK_CHECK) { TEST_F(DBTest2, DELETE_TEST) { engine::meta::TableSchema table_info = BuildTableSchema(); - engine::Status stat = db_->CreateTable(table_info); + auto stat = db_->CreateTable(table_info); engine::meta::TableSchema table_info_get; table_info_get.table_id_ = TABLE_NAME; @@ -480,7 +480,7 @@ TEST_F(DBTest2, DELETE_TEST) { TEST_F(DBTest2, DELETE_BY_RANGE_TEST) { engine::meta::TableSchema table_info = BuildTableSchema(); - engine::Status stat = db_->CreateTable(table_info); + auto stat = db_->CreateTable(table_info); engine::meta::TableSchema table_info_get; table_info_get.table_id_ = TABLE_NAME; diff --git a/cpp/unittest/db/mem_test.cpp b/cpp/unittest/db/mem_test.cpp index 73f6d89f5e..4e958795ad 100644 --- a/cpp/unittest/db/mem_test.cpp +++ b/cpp/unittest/db/mem_test.cpp @@ -216,7 +216,7 @@ TEST_F(MemManagerTest, MEM_TABLE_TEST) { TEST_F(MemManagerTest2, SERIAL_INSERT_SEARCH_TEST) { engine::meta::TableSchema table_info = BuildTableSchema(); - engine::Status stat = db_->CreateTable(table_info); + auto stat = db_->CreateTable(table_info); engine::meta::TableSchema table_info_get; table_info_get.table_id_ = TABLE_NAME; @@ -230,8 +230,8 @@ TEST_F(MemManagerTest2, SERIAL_INSERT_SEARCH_TEST) { int64_t nb = 100000; std::vector xb; BuildVectors(nb, xb); - engine::Status status = db_->InsertVectors(TABLE_NAME, nb, xb.data(), vector_ids); - ASSERT_TRUE(status.ok()); + stat = db_->InsertVectors(TABLE_NAME, nb, xb.data(), vector_ids); + ASSERT_TRUE(stat.ok()); std::this_thread::sleep_for(std::chrono::seconds(3)); @@ -262,7 +262,7 @@ TEST_F(MemManagerTest2, SERIAL_INSERT_SEARCH_TEST) { TEST_F(MemManagerTest2, INSERT_TEST) { engine::meta::TableSchema table_info = BuildTableSchema(); - engine::Status stat = db_->CreateTable(table_info); + auto stat = db_->CreateTable(table_info); engine::meta::TableSchema table_info_get; table_info_get.table_id_ = TABLE_NAME; @@ -278,8 +278,8 @@ TEST_F(MemManagerTest2, INSERT_TEST) { std::vector xb; BuildVectors(nb, xb); engine::IDNumbers vector_ids; - engine::Status status = db_->InsertVectors(TABLE_NAME, nb, xb.data(), vector_ids); - ASSERT_TRUE(status.ok()); + stat = db_->InsertVectors(TABLE_NAME, nb, xb.data(), vector_ids); + ASSERT_TRUE(stat.ok()); } auto end_time = METRICS_NOW_TIME; auto total_time = METRICS_MICROSECONDS(start_time, end_time); @@ -288,7 +288,7 @@ TEST_F(MemManagerTest2, INSERT_TEST) { TEST_F(MemManagerTest2, CONCURRENT_INSERT_SEARCH_TEST) { engine::meta::TableSchema table_info = BuildTableSchema(); - engine::Status stat = db_->CreateTable(table_info); + auto stat = db_->CreateTable(table_info); engine::meta::TableSchema table_info_get; table_info_get.table_id_ = TABLE_NAME; @@ -359,7 +359,7 @@ TEST_F(MemManagerTest2, CONCURRENT_INSERT_SEARCH_TEST) { TEST_F(MemManagerTest2, VECTOR_IDS_TEST) { engine::meta::TableSchema table_info = BuildTableSchema(); - engine::Status stat = db_->CreateTable(table_info); + auto stat = db_->CreateTable(table_info); engine::meta::TableSchema table_info_get; table_info_get.table_id_ = TABLE_NAME; diff --git a/cpp/unittest/db/misc_test.cpp b/cpp/unittest/db/misc_test.cpp index 28ec0f2f9c..ff530b0a73 100644 --- a/cpp/unittest/db/misc_test.cpp +++ b/cpp/unittest/db/misc_test.cpp @@ -15,11 +15,11 @@ // specific language governing permissions and limitations // under the License. -#include "db/Status.h" #include "db/Options.h" #include "db/meta/SqliteMetaImpl.h" #include "db/engine/EngineFactory.h" #include "db/Utils.h" +#include "utils/Status.h" #include "utils/Exception.h" #include "utils/easylogging++.h" @@ -30,13 +30,6 @@ using namespace zilliz::milvus; -namespace { - void CopyStatus(engine::Status& st1, engine::Status& st2) { - st1 = st2; - } - -} - TEST(DBMiscTest, EXCEPTION_TEST) { Exception ex1(""); std::string what = ex1.what(); @@ -47,36 +40,6 @@ TEST(DBMiscTest, EXCEPTION_TEST) { ASSERT_FALSE(what.empty()); } -TEST(DBMiscTest, STATUS_TEST) { - engine::Status status = engine::Status::OK(); - std::string str = status.ToString(); - ASSERT_FALSE(str.empty()); - - status = engine::Status(DB_ERROR, "mistake"); - ASSERT_EQ(status.code(), DB_ERROR); - str = status.ToString(); - ASSERT_FALSE(str.empty()); - - status = engine::Status(DB_NOT_FOUND, "mistake"); - ASSERT_EQ(status.code(), DB_NOT_FOUND); - str = status.ToString(); - ASSERT_FALSE(str.empty()); - - status = engine::Status(DB_ALREADY_EXIST, "mistake"); - ASSERT_EQ(status.code(), DB_ALREADY_EXIST); - str = status.ToString(); - ASSERT_FALSE(str.empty()); - - status = engine::Status(DB_META_TRANSACTION_FAILED, "mistake"); - ASSERT_EQ(status.code(), DB_META_TRANSACTION_FAILED); - str = status.ToString(); - ASSERT_FALSE(str.empty()); - - engine::Status status_copy = engine::Status::OK(); - CopyStatus(status_copy, status); - ASSERT_EQ(status.code(), DB_META_TRANSACTION_FAILED); -} - TEST(DBMiscTest, OPTIONS_TEST) { try { engine::ArchiveConf archive("$$##"); diff --git a/cpp/unittest/db/mysql_db_test.cpp b/cpp/unittest/db/mysql_db_test.cpp index 35f086b033..9d99b4095b 100644 --- a/cpp/unittest/db/mysql_db_test.cpp +++ b/cpp/unittest/db/mysql_db_test.cpp @@ -59,7 +59,7 @@ namespace { TEST_F(MySqlDBTest, DB_TEST) { engine::meta::TableSchema table_info = BuildTableSchema(); - engine::Status stat = db_->CreateTable(table_info); + auto stat = db_->CreateTable(table_info); engine::meta::TableSchema table_info_get; table_info_get.table_id_ = TABLE_NAME; @@ -144,7 +144,7 @@ TEST_F(MySqlDBTest, DB_TEST) { TEST_F(MySqlDBTest, SEARCH_TEST) { engine::meta::TableSchema table_info = BuildTableSchema(); - engine::Status stat = db_->CreateTable(table_info); + auto stat = db_->CreateTable(table_info); engine::meta::TableSchema table_info_get; table_info_get.table_id_ = TABLE_NAME; @@ -196,7 +196,7 @@ TEST_F(MySqlDBTest, SEARCH_TEST) { TEST_F(MySqlDBTest, ARHIVE_DISK_CHECK) { engine::meta::TableSchema table_info = BuildTableSchema(); - engine::Status stat = db_->CreateTable(table_info); + auto stat = db_->CreateTable(table_info); std::vector table_schema_array; stat = db_->AllTables(table_schema_array); @@ -241,7 +241,7 @@ TEST_F(MySqlDBTest, ARHIVE_DISK_CHECK) { TEST_F(MySqlDBTest, DELETE_TEST) { engine::meta::TableSchema table_info = BuildTableSchema(); - engine::Status stat = db_->CreateTable(table_info); + auto stat = db_->CreateTable(table_info); // std::cout << stat.ToString() << std::endl; engine::meta::TableSchema table_info_get; diff --git a/cpp/unittest/metrics/CMakeLists.txt b/cpp/unittest/metrics/CMakeLists.txt index 07f27e1e67..dddba2906a 100644 --- a/cpp/unittest/metrics/CMakeLists.txt +++ b/cpp/unittest/metrics/CMakeLists.txt @@ -37,9 +37,9 @@ aux_source_directory(${MILVUS_ENGINE_SRC}/src/metrics metrics_src) aux_source_directory(./ test_srcs) set(util_files + ${MILVUS_ENGINE_SRC}/utils/Status.cpp ${MILVUS_ENGINE_SRC}/utils/ValidationUtil.cpp - ${MILVUS_ENGINE_SRC}/utils/easylogging++.cc - ${MILVUS_ENGINE_SRC}/utils/easylogging++.h) + ${MILVUS_ENGINE_SRC}/utils/easylogging++.cc) aux_source_directory(${MILVUS_ENGINE_SRC}/db/scheduler scheduler_files) aux_source_directory(${MILVUS_ENGINE_SRC}/db/scheduler/context scheduler_context_files) diff --git a/cpp/unittest/metrics/metrics_test.cpp b/cpp/unittest/metrics/metrics_test.cpp index 2bd06ed091..636e2ffbb1 100644 --- a/cpp/unittest/metrics/metrics_test.cpp +++ b/cpp/unittest/metrics/metrics_test.cpp @@ -57,7 +57,7 @@ TEST_F(MetricTest, METRIC_TEST) { engine::meta::TableSchema group_info; group_info.dimension_ = group_dim; group_info.table_id_ = group_name; - engine::Status stat = db_->CreateTable(group_info); + auto stat = db_->CreateTable(group_info); engine::meta::TableSchema group_info_get; group_info_get.table_id_ = group_name; @@ -134,9 +134,9 @@ TEST_F(MetricTest, METRIC_TEST) { }; TEST_F(MetricTest, COLLECTOR_METRICS_TEST){ - engine::Status status = engine::Status::OK(); + auto status = Status::OK(); server::CollectInsertMetrics insert_metrics0(0, status); - status = engine::Status(DB_ERROR, "error"); + status = Status(DB_ERROR, "error"); server::CollectInsertMetrics insert_metrics1(0, status); server::CollectQueryMetrics query_metrics(10); diff --git a/cpp/unittest/metrics/utils.h b/cpp/unittest/metrics/utils.h index ef25654ed0..7f093dd091 100644 --- a/cpp/unittest/metrics/utils.h +++ b/cpp/unittest/metrics/utils.h @@ -42,7 +42,7 @@ #define STOP_TIMER(name) #endif -void ASSERT_STATS(zilliz::milvus::engine::Status& stat); +void ASSERT_STATS(zilliz::milvus::Status& stat); //class TestEnv : public ::testing::Environment { //public: diff --git a/cpp/unittest/scheduler/CMakeLists.txt b/cpp/unittest/scheduler/CMakeLists.txt index 9ada727f8e..7359420e16 100644 --- a/cpp/unittest/scheduler/CMakeLists.txt +++ b/cpp/unittest/scheduler/CMakeLists.txt @@ -36,9 +36,9 @@ aux_source_directory(${MILVUS_ENGINE_SRC}/scheduler scheduler_srcs) aux_source_directory(./ test_srcs) set(util_files + ${MILVUS_ENGINE_SRC}/utils/Status.cpp ${MILVUS_ENGINE_SRC}/utils/ValidationUtil.cpp - ${MILVUS_ENGINE_SRC}/utils/easylogging++.cc - ${MILVUS_ENGINE_SRC}/utils/easylogging++.h) + ${MILVUS_ENGINE_SRC}/utils/easylogging++.cc) set(db_scheduler_srcs ${scheduler_files} diff --git a/cpp/unittest/server/CMakeLists.txt b/cpp/unittest/server/CMakeLists.txt index 6bbe2183ce..8e06859929 100644 --- a/cpp/unittest/server/CMakeLists.txt +++ b/cpp/unittest/server/CMakeLists.txt @@ -54,9 +54,9 @@ set(db_scheduler_srcs ) set(util_files + ${MILVUS_ENGINE_SRC}/utils/Status.cpp ${MILVUS_ENGINE_SRC}/utils/ValidationUtil.cpp - ${MILVUS_ENGINE_SRC}/utils/easylogging++.cc - ${MILVUS_ENGINE_SRC}/utils/easylogging++.h) + ${MILVUS_ENGINE_SRC}/utils/easylogging++.cc) set(db_src ${config_files} diff --git a/cpp/unittest/server/util_test.cpp b/cpp/unittest/server/util_test.cpp index 3849f79d19..cfbcbab683 100644 --- a/cpp/unittest/server/util_test.cpp +++ b/cpp/unittest/server/util_test.cpp @@ -37,6 +37,10 @@ namespace { static const char* LOG_FILE_PATH = "./milvus/conf/log_config.conf"; +void CopyStatus(Status& st1, Status& st2) { + st1 = st2; +} + } TEST(UtilTest, EXCEPTION_TEST) { @@ -183,6 +187,44 @@ TEST(UtilTest, TIMERECORDER_TEST) { } } +TEST(UtilTest, STATUS_TEST) { + auto status = Status::OK(); + std::string str = status.ToString(); + ASSERT_FALSE(str.empty()); + + status = Status(DB_ERROR, "mistake"); + ASSERT_EQ(status.code(), DB_ERROR); + str = status.ToString(); + ASSERT_FALSE(str.empty()); + + status = Status(DB_NOT_FOUND, "mistake"); + ASSERT_EQ(status.code(), DB_NOT_FOUND); + str = status.ToString(); + ASSERT_FALSE(str.empty()); + + status = Status(DB_ALREADY_EXIST, "mistake"); + ASSERT_EQ(status.code(), DB_ALREADY_EXIST); + str = status.ToString(); + ASSERT_FALSE(str.empty()); + + status = Status(DB_META_TRANSACTION_FAILED, "mistake"); + ASSERT_EQ(status.code(), DB_META_TRANSACTION_FAILED); + str = status.ToString(); + ASSERT_FALSE(str.empty()); + + auto status_copy = Status::OK(); + CopyStatus(status_copy, status); + ASSERT_EQ(status.code(), DB_META_TRANSACTION_FAILED); + + auto status_ref(status); + ASSERT_EQ(status_ref.code(), status.code()); + ASSERT_EQ(status_ref.ToString(), status.ToString()); + + auto status_move = std::move(status); + ASSERT_EQ(status_move.code(), status_ref.code()); + ASSERT_EQ(status_move.ToString(), status_ref.ToString()); +} + TEST(ValidationUtilTest, VALIDATE_TABLENAME_TEST) { std::string table_name = "Normal123_"; ErrorCode res = server::ValidationUtil::ValidateTableName(table_name); From 5a2743946b68de2c155af4315e6b0b5c7604be74 Mon Sep 17 00:00:00 2001 From: wxyu Date: Wed, 18 Sep 2019 10:42:01 +0800 Subject: [PATCH 2/4] MS-562 Add JobMgr and TaskCreator in Scheduler Former-commit-id: bbaf2b649843e86fc7bbd28ec61e5a990fc5951f --- cpp/CHANGELOG.md | 1 + cpp/src/scheduler/JobMgr.cpp | 89 +++++++++++++++++++++++++++++++ cpp/src/scheduler/JobMgr.h | 78 +++++++++++++++++++++++++++ cpp/src/scheduler/TaskCreator.cpp | 65 ++++++++++++++++++++++ cpp/src/scheduler/TaskCreator.h | 61 +++++++++++++++++++++ cpp/src/scheduler/job/DeleteJob.h | 1 + cpp/src/scheduler/job/Job.h | 1 + cpp/src/scheduler/job/SearchJob.h | 6 +++ 8 files changed, 302 insertions(+) create mode 100644 cpp/src/scheduler/JobMgr.cpp create mode 100644 cpp/src/scheduler/JobMgr.h create mode 100644 cpp/src/scheduler/TaskCreator.cpp create mode 100644 cpp/src/scheduler/TaskCreator.h diff --git a/cpp/CHANGELOG.md b/cpp/CHANGELOG.md index 1f0aedef85..c5d27e3461 100644 --- a/cpp/CHANGELOG.md +++ b/cpp/CHANGELOG.md @@ -12,6 +12,7 @@ Please mark all change in change log and use the ticket from JIRA. - MS-557 - Merge Log.h - MS-556 - Add Job Definition in Scheduler - MS-558 - Refine status code +- MS-562 - Add JobMgr and TaskCreator in Scheduler ## New Feature diff --git a/cpp/src/scheduler/JobMgr.cpp b/cpp/src/scheduler/JobMgr.cpp new file mode 100644 index 0000000000..c5c14c1b41 --- /dev/null +++ b/cpp/src/scheduler/JobMgr.cpp @@ -0,0 +1,89 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include "JobMgr.h" +#include "task/Task.h" +#include "TaskCreator.h" + + +namespace zilliz { +namespace milvus { +namespace scheduler { + +using namespace engine; + +JobMgr::JobMgr(ResourceMgrPtr res_mgr) + : res_mgr_(std::move(res_mgr)) {} + +void +JobMgr::Start() { + if (not running_) { + worker_thread_ = std::thread(&JobMgr::worker_function, this); + running_ = true; + } +} + +void +JobMgr::Stop() { + if (running_) { + this->Put(nullptr); + worker_thread_.join(); + running_ = false; + } +} + +void +JobMgr::Put(const JobPtr &job) { + { + std::lock_guard lock(mutex_); + queue_.push(job); + } + cv_.notify_one(); +} + +void +JobMgr::worker_function() { + while (running_) { + std::unique_lock lock(mutex_); + cv_.wait(lock, [this] { return !queue_.empty(); }); + auto job = queue_.front(); + queue_.pop(); + lock.unlock(); + if (job == nullptr) { + break; + } + + auto tasks = build_task(job); + auto disk_list = res_mgr_->GetDiskResources(); + if (!disk_list.empty()) { + if (auto disk = disk_list[0].lock()) { + for (auto &task : tasks) { + disk->task_table().Put(task); + } + } + } + } +} + +std::vector +JobMgr::build_task(const JobPtr &job) { + return TaskCreator::Create(job); +} + +} +} +} diff --git a/cpp/src/scheduler/JobMgr.h b/cpp/src/scheduler/JobMgr.h new file mode 100644 index 0000000000..f096ab6121 --- /dev/null +++ b/cpp/src/scheduler/JobMgr.h @@ -0,0 +1,78 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +#pragma once + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "job/Job.h" +#include "task/Task.h" +#include "ResourceMgr.h" + + +namespace zilliz { +namespace milvus { +namespace scheduler { + +using engine::TaskPtr; +using engine::ResourceMgrPtr; + +class JobMgr { +public: + explicit + JobMgr(ResourceMgrPtr res_mgr); + + void + Start(); + + void + Stop(); + +public: + void + Put(const JobPtr &job); + +private: + void + worker_function(); + + std::vector + build_task(const JobPtr &job); + +private: + bool running_ = false; + std::queue queue_; + + std::thread worker_thread_; + + std::mutex mutex_; + std::condition_variable cv_; + + ResourceMgrPtr res_mgr_ = nullptr; +}; + +} +} +} diff --git a/cpp/src/scheduler/TaskCreator.cpp b/cpp/src/scheduler/TaskCreator.cpp new file mode 100644 index 0000000000..afe0d9d868 --- /dev/null +++ b/cpp/src/scheduler/TaskCreator.cpp @@ -0,0 +1,65 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include "TaskCreator.h" + + +namespace zilliz { +namespace milvus { +namespace scheduler { + +std::vector +TaskCreator::Create(const JobPtr &job) { + switch (job->type()) { + case JobType::SEARCH: { + return Create(std::static_pointer_cast(job)); + } + case JobType::DELETE: { + return Create(std::static_pointer_cast(job)); + } + default: { + // TODO: error + return std::vector(); + } + } +} + +std::vector +TaskCreator::Create(const SearchJobPtr &job) { + std::vector tasks; + for (auto &index_file : job->index_files()) { + auto task = std::make_shared(index_file.second); + tasks.emplace_back(task); + } + + return tasks; +} + +std::vector +TaskCreator::Create(const DeleteJobPtr &job) { + std::vector tasks; +// auto task = std::make_shared(job); +// tasks.emplace_back(task); + + return tasks; +} + + +} +} +} + diff --git a/cpp/src/scheduler/TaskCreator.h b/cpp/src/scheduler/TaskCreator.h new file mode 100644 index 0000000000..b0e947600d --- /dev/null +++ b/cpp/src/scheduler/TaskCreator.h @@ -0,0 +1,61 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +#pragma once + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "job/Job.h" +#include "job/SearchJob.h" +#include "job/DeleteJob.h" +#include "task/Task.h" +#include "task/SearchTask.h" +#include "task/DeleteTask.h" + + +namespace zilliz { +namespace milvus { +namespace scheduler { + +using engine::TaskPtr; +using engine::XSearchTask; +using engine::XDeleteTask; + +class TaskCreator { +public: + static std::vector + Create(const JobPtr &job); + +public: + static std::vector + Create(const SearchJobPtr &job); + + static std::vector + Create(const DeleteJobPtr &job); +}; + +} +} +} diff --git a/cpp/src/scheduler/job/DeleteJob.h b/cpp/src/scheduler/job/DeleteJob.h index 7410577563..d82262b235 100644 --- a/cpp/src/scheduler/job/DeleteJob.h +++ b/cpp/src/scheduler/job/DeleteJob.h @@ -14,6 +14,7 @@ // KIND, either express or implied. See the License for the // specific language governing permissions and limitations // under the License. +#pragma once #include #include diff --git a/cpp/src/scheduler/job/Job.h b/cpp/src/scheduler/job/Job.h index e79dd51e71..845a5dc165 100644 --- a/cpp/src/scheduler/job/Job.h +++ b/cpp/src/scheduler/job/Job.h @@ -14,6 +14,7 @@ // KIND, either express or implied. See the License for the // specific language governing permissions and limitations // under the License. +#pragma once #include #include diff --git a/cpp/src/scheduler/job/SearchJob.h b/cpp/src/scheduler/job/SearchJob.h index 4530c6485b..ed6531767c 100644 --- a/cpp/src/scheduler/job/SearchJob.h +++ b/cpp/src/scheduler/job/SearchJob.h @@ -14,6 +14,7 @@ // KIND, either express or implied. See the License for the // specific language governing permissions and limitations // under the License. +#pragma once #include #include @@ -78,6 +79,11 @@ public: return vectors_; } + Id2IndexMap & + index_files() { + return index_files_; + } + private: uint64_t topk_ = 0; uint64_t nq_ = 0; From 69ce92dcc58884af741320c38cc67e928a85fbc3 Mon Sep 17 00:00:00 2001 From: starlord Date: Wed, 18 Sep 2019 12:49:29 +0800 Subject: [PATCH 3/4] MS-558 refine status code Former-commit-id: ebb5ec55a58cfce4385f39d186e86d858eab1bf0 --- cpp/coverage.sh | 2 + cpp/src/db/Utils.cpp | 18 +- cpp/src/db/insert/MemTable.cpp | 4 +- cpp/src/db/insert/MemTableFile.cpp | 2 +- .../examples/grpcsimple/src/ClientTest.cpp | 24 +- cpp/src/sdk/include/Status.h | 14 +- cpp/src/sdk/interface/Status.cpp | 56 +-- cpp/src/server/DBWrapper.cpp | 24 +- cpp/src/server/DBWrapper.h | 6 +- cpp/src/server/Server.cpp | 21 +- cpp/src/server/Server.h | 4 +- cpp/src/server/ServerConfig.cpp | 108 ++--- cpp/src/server/ServerConfig.h | 18 +- cpp/src/server/grpc_impl/GrpcMilvusServer.cpp | 7 +- cpp/src/server/grpc_impl/GrpcMilvusServer.h | 6 +- .../server/grpc_impl/GrpcRequestScheduler.cpp | 55 ++- .../server/grpc_impl/GrpcRequestScheduler.h | 20 +- cpp/src/server/grpc_impl/GrpcRequestTask.cpp | 406 +++++++++--------- cpp/src/server/grpc_impl/GrpcRequestTask.h | 30 +- cpp/src/utils/CommonUtil.cpp | 29 +- cpp/src/utils/CommonUtil.h | 7 +- cpp/src/utils/Status.cpp | 44 +- cpp/src/utils/Status.h | 17 +- cpp/src/utils/StringHelpFunctions.cpp | 24 +- cpp/src/utils/StringHelpFunctions.h | 16 +- cpp/src/utils/ValidationUtil.cpp | 148 ++++--- cpp/src/utils/ValidationUtil.h | 32 +- cpp/unittest/server/config_test.cpp | 24 +- cpp/unittest/server/rpc_test.cpp | 16 +- cpp/unittest/server/util_test.cpp | 140 +++--- 30 files changed, 665 insertions(+), 657 deletions(-) diff --git a/cpp/coverage.sh b/cpp/coverage.sh index 64f60961a8..f1c42ede61 100755 --- a/cpp/coverage.sh +++ b/cpp/coverage.sh @@ -104,6 +104,8 @@ ${LCOV_CMD} -r "${FILE_INFO_OUTPUT}" -o "${FILE_INFO_OUTPUT_NEW}" \ "src/server/Server.cpp"\ "src/server/DBWrapper.cpp"\ "src/server/grpc_impl/GrpcMilvusServer.cpp"\ + "src/utils/easylogging++.h"\ + "src/utils/easylogging++.cc"\ # gen html report ${LCOV_GEN_CMD} "${FILE_INFO_OUTPUT_NEW}" --output-directory ${DIR_LCOV_OUTPUT}/ \ No newline at end of file diff --git a/cpp/src/db/Utils.cpp b/cpp/src/db/Utils.cpp index cbc3ba45b6..cbed4c6646 100644 --- a/cpp/src/db/Utils.cpp +++ b/cpp/src/db/Utils.cpp @@ -80,17 +80,17 @@ Status CreateTablePath(const DBMetaOptions& options, const std::string& table_id std::string db_path = options.path; std::string table_path = db_path + TABLES_FOLDER + table_id; auto status = server::CommonUtil::CreateDirectory(table_path); - if (status != 0) { - ENGINE_LOG_ERROR << "Create directory " << table_path << " Error"; - return Status(DB_ERROR, "Failed to create table path"); + if (!status.ok()) { + ENGINE_LOG_ERROR << status.message(); + return status; } for(auto& path : options.slave_paths) { table_path = path + TABLES_FOLDER + table_id; status = server::CommonUtil::CreateDirectory(table_path); - if (status != 0) { - ENGINE_LOG_ERROR << "Create directory " << table_path << " Error"; - return Status(DB_ERROR, "Failed to create table path"); + if (!status.ok()) { + ENGINE_LOG_ERROR << status.message(); + return status; } } @@ -120,9 +120,9 @@ Status CreateTableFilePath(const DBMetaOptions& options, meta::TableFileSchema& std::string parent_path = GetTableFileParentFolder(options, table_file); auto status = server::CommonUtil::CreateDirectory(parent_path); - if (status != 0) { - ENGINE_LOG_ERROR << "Create directory " << parent_path << " Error"; - return Status(DB_ERROR, "Failed to create partition directory"); + if (!status.ok()) { + ENGINE_LOG_ERROR << status.message(); + return status; } table_file.location_ = parent_path + "/" + table_file.file_id_; diff --git a/cpp/src/db/insert/MemTable.cpp b/cpp/src/db/insert/MemTable.cpp index 6bd5274d9e..7f4f497ba1 100644 --- a/cpp/src/db/insert/MemTable.cpp +++ b/cpp/src/db/insert/MemTable.cpp @@ -54,7 +54,7 @@ Status MemTable::Add(VectorSourcePtr &source, IDNumbers &vector_ids) { } if (!status.ok()) { - std::string err_msg = "MemTable::Add failed: " + status.ToString(); + std::string err_msg = "Insert failed: " + status.ToString(); ENGINE_LOG_ERROR << err_msg; return Status(DB_ERROR, err_msg); } @@ -74,7 +74,7 @@ Status MemTable::Serialize() { for (auto mem_table_file = mem_table_file_list_.begin(); mem_table_file != mem_table_file_list_.end();) { auto status = (*mem_table_file)->Serialize(); if (!status.ok()) { - std::string err_msg = "MemTable::Serialize failed: " + status.ToString(); + std::string err_msg = "Insert data serialize failed: " + status.ToString(); ENGINE_LOG_ERROR << err_msg; return Status(DB_ERROR, err_msg); } diff --git a/cpp/src/db/insert/MemTableFile.cpp b/cpp/src/db/insert/MemTableFile.cpp index 63a968986d..57ef39c670 100644 --- a/cpp/src/db/insert/MemTableFile.cpp +++ b/cpp/src/db/insert/MemTableFile.cpp @@ -67,7 +67,7 @@ Status MemTableFile::Add(const VectorSourcePtr &source, IDNumbers& vector_ids) { std::string err_msg = "MemTableFile::Add: table_file_schema dimension = " + std::to_string(table_file_schema_.dimension_) + ", table_id = " + table_file_schema_.table_id_; ENGINE_LOG_ERROR << err_msg; - return Status(DB_ERROR, "not able to create table file"); + return Status(DB_ERROR, "Not able to create table file"); } size_t single_vector_mem_size = table_file_schema_.dimension_ * VECTOR_TYPE_SIZE; diff --git a/cpp/src/sdk/examples/grpcsimple/src/ClientTest.cpp b/cpp/src/sdk/examples/grpcsimple/src/ClientTest.cpp index f0f7cf5292..ff3c6edaf5 100644 --- a/cpp/src/sdk/examples/grpcsimple/src/ClientTest.cpp +++ b/cpp/src/sdk/examples/grpcsimple/src/ClientTest.cpp @@ -190,7 +190,7 @@ void DoSearch(std::shared_ptr conn, { TimeRecorder rc(phase_name); Status stat = conn->Search(TABLE_NAME, record_array, query_range_array, TOP_K, 32, topk_query_result_array); - std::cout << "SearchVector function call status: " << stat.ToString() << std::endl; + std::cout << "SearchVector function call status: " << stat.message() << std::endl; } auto finish = std::chrono::high_resolution_clock::now(); std::cout << "SEARCHVECTOR COST: " << std::chrono::duration_cast>(finish - start).count() << "s\n"; @@ -207,7 +207,7 @@ ClientTest::Test(const std::string& address, const std::string& port) { {//connect server ConnectParam param = {address, port}; Status stat = conn->Connect(param); - std::cout << "Connect function call status: " << stat.ToString() << std::endl; + std::cout << "Connect function call status: " << stat.message() << std::endl; } {//server version @@ -223,7 +223,7 @@ ClientTest::Test(const std::string& address, const std::string& port) { { std::vector tables; Status stat = conn->ShowTables(tables); - std::cout << "ShowTables function call status: " << stat.ToString() << std::endl; + std::cout << "ShowTables function call status: " << stat.message() << std::endl; std::cout << "All tables: " << std::endl; for(auto& table : tables) { int64_t row_count = 0; @@ -236,7 +236,7 @@ ClientTest::Test(const std::string& address, const std::string& port) { {//create table TableSchema tb_schema = BuildTableSchema(); Status stat = conn->CreateTable(tb_schema); - std::cout << "CreateTable function call status: " << stat.ToString() << std::endl; + std::cout << "CreateTable function call status: " << stat.message() << std::endl; PrintTableSchema(tb_schema); bool has_table = conn->HasTable(tb_schema.table_name); @@ -248,7 +248,7 @@ ClientTest::Test(const std::string& address, const std::string& port) { {//describe table TableSchema tb_schema; Status stat = conn->DescribeTable(TABLE_NAME, tb_schema); - std::cout << "DescribeTable function call status: " << stat.ToString() << std::endl; + std::cout << "DescribeTable function call status: " << stat.message() << std::endl; PrintTableSchema(tb_schema); } @@ -279,7 +279,7 @@ ClientTest::Test(const std::string& address, const std::string& port) { std::cout << "InsertVector cost: " << std::chrono::duration_cast>(finish - start).count() << "s\n"; - std::cout << "InsertVector function call status: " << stat.ToString() << std::endl; + std::cout << "InsertVector function call status: " << stat.message() << std::endl; std::cout << "Returned id array count: " << record_ids.size() << std::endl; if(search_record_array.size() < NQ) { @@ -305,16 +305,16 @@ ClientTest::Test(const std::string& address, const std::string& port) { index.index_type = IndexType::gpu_ivfsq8; index.nlist = 16384; Status stat = conn->CreateIndex(index); - std::cout << "CreateIndex function call status: " << stat.ToString() << std::endl; + std::cout << "CreateIndex function call status: " << stat.message() << std::endl; IndexParam index2; stat = conn->DescribeIndex(TABLE_NAME, index2); - std::cout << "DescribeIndex function call status: " << stat.ToString() << std::endl; + std::cout << "DescribeIndex function call status: " << stat.message() << std::endl; } {//preload table Status stat = conn->PreloadTable(TABLE_NAME); - std::cout << "PreloadTable function call status: " << stat.ToString() << std::endl; + std::cout << "PreloadTable function call status: " << stat.message() << std::endl; } {//search vectors after build index finish @@ -326,7 +326,7 @@ ClientTest::Test(const std::string& address, const std::string& port) { {//delete index Status stat = conn->DropIndex(TABLE_NAME); - std::cout << "DropIndex function call status: " << stat.ToString() << std::endl; + std::cout << "DropIndex function call status: " << stat.message() << std::endl; int64_t row_count = 0; stat = conn->CountTable(TABLE_NAME, row_count); @@ -339,12 +339,12 @@ ClientTest::Test(const std::string& address, const std::string& port) { rg.end_value = CurrentTmDate(-3); Status stat = conn->DeleteByRange(rg, TABLE_NAME); - std::cout << "DeleteByRange function call status: " << stat.ToString() << std::endl; + std::cout << "DeleteByRange function call status: " << stat.message() << std::endl; } {//delete table Status stat = conn->DropTable(TABLE_NAME); - std::cout << "DeleteTable function call status: " << stat.ToString() << std::endl; + std::cout << "DeleteTable function call status: " << stat.message() << std::endl; } {//server status diff --git a/cpp/src/sdk/include/Status.h b/cpp/src/sdk/include/Status.h index 145d09d47a..cdb5f54ede 100644 --- a/cpp/src/sdk/include/Status.h +++ b/cpp/src/sdk/include/Status.h @@ -40,14 +40,12 @@ enum class StatusCode { ServerFailed, }; -using ErrorCode = StatusCode; - /** * @brief Status for SDK interface return */ class Status { public: - Status(ErrorCode code, const std::string &msg); + Status(StatusCode code, const std::string &msg); Status(); ~Status(); @@ -67,14 +65,14 @@ public: bool ok() const { return state_ == nullptr || code() == StatusCode::OK; } - std::string - ToString() const; - - ErrorCode + StatusCode code() const { - return (state_ == nullptr) ? StatusCode::OK : *(ErrorCode*)(state_); + return (state_ == nullptr) ? StatusCode::OK : *(StatusCode*)(state_); } + std::string + message() const; + private: inline void CopyFrom(const Status &s); diff --git a/cpp/src/sdk/interface/Status.cpp b/cpp/src/sdk/interface/Status.cpp index b04c6581fe..e6fd06c72e 100644 --- a/cpp/src/sdk/interface/Status.cpp +++ b/cpp/src/sdk/interface/Status.cpp @@ -21,9 +21,9 @@ namespace milvus { -constexpr int CODE_WIDTH = sizeof(ErrorCode); +constexpr int CODE_WIDTH = sizeof(StatusCode); -Status::Status(ErrorCode code, const std::string& msg) { +Status::Status(StatusCode code, const std::string& msg) { //4 bytes store code //4 bytes store message length //the left bytes store message string @@ -50,7 +50,8 @@ Status::Status(const Status &s) CopyFrom(s); } -Status &Status::operator=(const Status &s) { +Status& +Status::operator=(const Status &s) { CopyFrom(s); return *this; } @@ -60,12 +61,14 @@ Status::Status(Status &&s) MoveFrom(s); } -Status &Status::operator=(Status &&s) { +Status& +Status::operator=(Status &&s) { MoveFrom(s); return *this; } -void Status::CopyFrom(const Status &s) { +void +Status::CopyFrom(const Status &s) { delete state_; state_ = nullptr; if(s.state_ == nullptr) { @@ -73,58 +76,33 @@ void Status::CopyFrom(const Status &s) { } uint32_t length = 0; - std::memcpy(&length, s.state_ + CODE_WIDTH, sizeof(length)); + memcpy(&length, s.state_ + CODE_WIDTH, sizeof(length)); int buff_len = length + sizeof(length) + CODE_WIDTH; state_ = new char[buff_len]; memcpy((void*)state_, (void*)s.state_, buff_len); } -void Status::MoveFrom(Status &s) { +void +Status::MoveFrom(Status &s) { delete state_; state_ = s.state_; s.state_ = nullptr; } -std::string Status::ToString() const { +std::string +Status::message() const { if (state_ == nullptr) { - return "OK"; - } - - std::string result; - switch (code()) { - case StatusCode::OK: - result = "OK "; - break; - case StatusCode::UnknownError: - result = "Unknown error: "; - break; - case StatusCode::NotSupported: - result = "Not supported: "; - break; - case StatusCode::NotConnected: - result = "Not connected: "; - break; - case StatusCode::InvalidAgument: - result = "Invalid agument: "; - break; - case StatusCode::RPCFailed: - result = "Remote call failed: "; - break; - case StatusCode::ServerFailed: - result = "Service error: "; - break; - default: - result = "Error code(" + std::to_string((int)code()) + "): "; - break; + return ""; } + std::string msg; uint32_t length = 0; memcpy(&length, state_ + CODE_WIDTH, sizeof(length)); if(length > 0) { - result.append(state_ + sizeof(length) + CODE_WIDTH, length); + msg.append(state_ + sizeof(length) + CODE_WIDTH, length); } - return result; + return msg; } } diff --git a/cpp/src/server/DBWrapper.cpp b/cpp/src/server/DBWrapper.cpp index d4a145ff37..508d9552b1 100644 --- a/cpp/src/server/DBWrapper.cpp +++ b/cpp/src/server/DBWrapper.cpp @@ -33,7 +33,7 @@ DBWrapper::DBWrapper() { } -ErrorCode DBWrapper::StartService() { +Status DBWrapper::StartService() { //db config zilliz::milvus::engine::Options opt; ConfigNode& db_config = ServerConfig::GetInstance().GetConfig(CONFIG_DB); @@ -60,7 +60,7 @@ ErrorCode DBWrapper::StartService() { opt.mode = zilliz::milvus::engine::Options::MODE::READ_ONLY; } else { - std::cout << "ERROR: mode specified in server_config is not one of ['single', 'cluster', 'read_only']" << std::endl; + std::cerr << "ERROR: mode specified in server_config is not one of ['single', 'cluster', 'read_only']" << std::endl; kill(0, SIGUSR1); } @@ -91,16 +91,16 @@ ErrorCode DBWrapper::StartService() { opt.meta.archive_conf.SetCriterias(criterial); //create db root folder - ErrorCode err = CommonUtil::CreateDirectory(opt.meta.path); - if(err != SERVER_SUCCESS) { - std::cout << "ERROR! Failed to create database root path: " << opt.meta.path << std::endl; + Status status = CommonUtil::CreateDirectory(opt.meta.path); + if(!status.ok()) { + std::cerr << "ERROR! Failed to create database root path: " << opt.meta.path << std::endl; kill(0, SIGUSR1); } for(auto& path : opt.meta.slave_paths) { - err = CommonUtil::CreateDirectory(path); - if(err != SERVER_SUCCESS) { - std::cout << "ERROR! Failed to create database slave path: " << path << std::endl; + status = CommonUtil::CreateDirectory(path); + if(!status.ok()) { + std::cerr << "ERROR! Failed to create database slave path: " << path << std::endl; kill(0, SIGUSR1); } } @@ -114,21 +114,21 @@ ErrorCode DBWrapper::StartService() { } if(db_ == nullptr) { - std::cout << "ERROR! Failed to open database: " << msg << std::endl; + std::cerr << "ERROR! Failed to open database: " << msg << std::endl; kill(0, SIGUSR1); } db_->Start(); - return SERVER_SUCCESS; + return Status::OK(); } -ErrorCode DBWrapper::StopService() { +Status DBWrapper::StopService() { if(db_) { db_->Stop(); } - return SERVER_SUCCESS; + return Status::OK(); } } diff --git a/cpp/src/server/DBWrapper.h b/cpp/src/server/DBWrapper.h index a321e3cdae..99b8f90500 100644 --- a/cpp/src/server/DBWrapper.h +++ b/cpp/src/server/DBWrapper.h @@ -17,7 +17,7 @@ #pragma once -#include "utils/Error.h" +#include "utils/Status.h" #include "db/DB.h" #include @@ -41,8 +41,8 @@ public: return GetInstance().EngineDB(); } - ErrorCode StartService(); - ErrorCode StopService(); + Status StartService(); + Status StopService(); engine::DBPtr EngineDB() { return db_; diff --git a/cpp/src/server/Server.cpp b/cpp/src/server/Server.cpp index f64ec009b9..e1e67a112f 100644 --- a/cpp/src/server/Server.cpp +++ b/cpp/src/server/Server.cpp @@ -138,11 +138,11 @@ Server::Daemonize() { if (!pid_filename_.empty()) { pid_fd = open(pid_filename_.c_str(), O_RDWR | O_CREAT, 0640); if (pid_fd < 0) { - std::cout << "Can't open filename: " + pid_filename_ + ", Error: " + strerror(errno); + std::cerr << "Can't open filename: " + pid_filename_ + ", Error: " + strerror(errno); exit(EXIT_FAILURE); } if (lockf(pid_fd, F_TLOCK, 0) < 0) { - std::cout << "Can't lock filename: " + pid_filename_ + ", Error: " + strerror(errno); + std::cerr << "Can't lock filename: " + pid_filename_ + ", Error: " + strerror(errno); exit(EXIT_FAILURE); } @@ -216,18 +216,18 @@ Server::Start() { void Server::Stop() { - std::cout << "Milvus server is going to shutdown ..." << std::endl; + std::cerr << "Milvus server is going to shutdown ..." << std::endl; // Unlock and close lockfile if (pid_fd != -1) { int ret = lockf(pid_fd, F_ULOCK, 0); if (ret != 0) { - std::cout << "Can't lock file: " << strerror(errno) << std::endl; + std::cerr << "Can't lock file: " << strerror(errno) << std::endl; exit(0); } ret = close(pid_fd); if (ret != 0) { - std::cout << "Can't close file: " << strerror(errno) << std::endl; + std::cerr << "Can't close file: " << strerror(errno) << std::endl; exit(0); } } @@ -236,24 +236,23 @@ Server::Stop() { if (!pid_filename_.empty()) { int ret = unlink(pid_filename_.c_str()); if (ret != 0) { - std::cout << "Can't unlink file: " << strerror(errno) << std::endl; + std::cerr << "Can't unlink file: " << strerror(errno) << std::endl; exit(0); } } - running_ = 0; - StopService(); - std::cout << "Milvus server is closed!" << std::endl; + std::cerr << "Milvus server is closed!" << std::endl; } ErrorCode Server::LoadConfig() { ServerConfig::GetInstance().LoadConfigFile(config_filename_); - ErrorCode err = ServerConfig::GetInstance().ValidateConfig(); - if (err != SERVER_SUCCESS) { + auto status = ServerConfig::GetInstance().ValidateConfig(); + if (!status.ok()) { + std::cerr << "Failed to load config file: " << config_filename_ << std::endl; exit(0); } diff --git a/cpp/src/server/Server.h b/cpp/src/server/Server.h index f66c8a314b..43e8b28028 100644 --- a/cpp/src/server/Server.h +++ b/cpp/src/server/Server.h @@ -17,7 +17,7 @@ #pragma once -#include "utils/Error.h" +#include "utils/Status.h" #include #include @@ -40,7 +40,6 @@ class Server { void Daemonize(); - static void HandleSignal(int signal); ErrorCode LoadConfig(); void StartService(); @@ -48,7 +47,6 @@ class Server { private: int64_t daemonized_ = 0; - int64_t running_ = 1; int pid_fd = -1; std::string pid_filename_; std::string config_filename_; diff --git a/cpp/src/server/ServerConfig.cpp b/cpp/src/server/ServerConfig.cpp index 988ee707b6..687490cf5e 100644 --- a/cpp/src/server/ServerConfig.cpp +++ b/cpp/src/server/ServerConfig.cpp @@ -41,17 +41,17 @@ ServerConfig::GetInstance() { return config; } -ErrorCode +Status ServerConfig::LoadConfigFile(const std::string &config_filename) { std::string filename = config_filename; if (filename.empty()) { - std::cout << "ERROR: a config file is required" << std::endl; + std::cerr << "ERROR: a config file is required" << std::endl; exit(1);//directly exit program if config file not specified } struct stat directoryStat; int statOK = stat(filename.c_str(), &directoryStat); if (statOK != 0) { - std::cout << "ERROR: " << filename << " not found!" << std::endl; + std::cerr << "ERROR: " << filename << " not found!" << std::endl; exit(1);//directly exit program if config file not found } @@ -59,43 +59,44 @@ ServerConfig::LoadConfigFile(const std::string &config_filename) { ConfigMgr *mgr = const_cast(ConfigMgr::GetInstance()); ErrorCode err = mgr->LoadConfigFile(filename); if (err != 0) { - std::cout << "Server failed to load config file" << std::endl; + std::cerr << "Server failed to load config file" << std::endl; exit(1);//directly exit program if the config file is illegal } } catch (YAML::Exception &e) { - std::cout << "Server failed to load config file: " << std::endl; - return SERVER_UNEXPECTED_ERROR; + std::cerr << "Server failed to load config file: " << std::endl; + exit(1);//directly exit program if the config file is illegal } - return SERVER_SUCCESS; + return Status::OK(); } -ErrorCode ServerConfig::ValidateConfig() { +Status +ServerConfig::ValidateConfig() { bool okay = true; - if (CheckServerConfig() != SERVER_SUCCESS) { + if (!CheckServerConfig().ok()) { okay = false; } - if (CheckDBConfig() != SERVER_SUCCESS) { + if (!CheckDBConfig().ok()) { okay = false; } - if (CheckMetricConfig() != SERVER_SUCCESS) { + if (!CheckMetricConfig().ok()) { okay = false; } - if (CheckCacheConfig() != SERVER_SUCCESS) { + if (!CheckCacheConfig().ok()) { okay = false; } - if (CheckEngineConfig() != SERVER_SUCCESS) { + if (!CheckEngineConfig().ok()) { okay = false; } - if (CheckResourceConfig() != SERVER_SUCCESS) { + if (!CheckResourceConfig().ok()) { okay = false; } - return (okay ? SERVER_SUCCESS : SERVER_INVALID_ARGUMENT); + return (okay ? Status::OK() : Status(SERVER_INVALID_ARGUMENT, "Config validation not pass")); } -ErrorCode +Status ServerConfig::CheckServerConfig() { /* server_config: @@ -109,13 +110,13 @@ ServerConfig::CheckServerConfig() { ConfigNode server_config = GetConfig(CONFIG_SERVER); std::string ip_address = server_config.GetValue(CONFIG_SERVER_ADDRESS, "127.0.0.1"); - if (ValidationUtil::ValidateIpAddress(ip_address) != SERVER_SUCCESS) { + if (!ValidationUtil::ValidateIpAddress(ip_address).ok()) { std::cerr << "ERROR: invalid server IP address: " << ip_address << std::endl; okay = false; } std::string port_str = server_config.GetValue(CONFIG_SERVER_PORT, "19530"); - if (ValidationUtil::ValidateStringIsNumber(port_str) != SERVER_SUCCESS) { + if (!ValidationUtil::ValidateStringIsNumber(port_str).ok()) { std::cerr << "ERROR: port " << port_str << " is not a number" << std::endl; okay = false; } else { @@ -151,10 +152,10 @@ ServerConfig::CheckServerConfig() { okay = false; } - return (okay ? SERVER_SUCCESS : SERVER_INVALID_ARGUMENT); + return (okay ? Status::OK() : Status(SERVER_INVALID_ARGUMENT, "Server config is illegal")); } -ErrorCode +Status ServerConfig::CheckDBConfig() { /* db_config: @@ -182,25 +183,25 @@ ServerConfig::CheckDBConfig() { } std::string db_backend_url = db_config.GetValue(CONFIG_DB_URL); - if (ValidationUtil::ValidateDbURI(db_backend_url) != SERVER_SUCCESS) { + if (!ValidationUtil::ValidateDbURI(db_backend_url).ok()) { std::cerr << "ERROR: invalid db_backend_url: " << db_backend_url << std::endl; okay = false; } std::string archive_disk_threshold_str = db_config.GetValue(CONFIG_DB_INSERT_BUFFER_SIZE, "0"); - if (ValidationUtil::ValidateStringIsNumber(archive_disk_threshold_str) != SERVER_SUCCESS) { + if (!ValidationUtil::ValidateStringIsNumber(archive_disk_threshold_str).ok()) { std::cerr << "ERROR: archive_disk_threshold " << archive_disk_threshold_str << " is not a number" << std::endl; okay = false; } std::string archive_days_threshold_str = db_config.GetValue(CONFIG_DB_INSERT_BUFFER_SIZE, "0"); - if (ValidationUtil::ValidateStringIsNumber(archive_days_threshold_str) != SERVER_SUCCESS) { + if (!ValidationUtil::ValidateStringIsNumber(archive_days_threshold_str).ok()) { std::cerr << "ERROR: archive_days_threshold " << archive_days_threshold_str << " is not a number" << std::endl; okay = false; } std::string insert_buffer_size_str = db_config.GetValue(CONFIG_DB_INSERT_BUFFER_SIZE, "4"); - if (ValidationUtil::ValidateStringIsNumber(insert_buffer_size_str) != SERVER_SUCCESS) { + if (!ValidationUtil::ValidateStringIsNumber(insert_buffer_size_str).ok()) { std::cerr << "ERROR: insert_buffer_size " << insert_buffer_size_str << " is not a number" << std::endl; okay = false; } @@ -216,21 +217,21 @@ ServerConfig::CheckDBConfig() { } std::string gpu_index_str = db_config.GetValue(CONFIG_DB_BUILD_INDEX_GPU, "0"); - if (ValidationUtil::ValidateStringIsNumber(gpu_index_str) != SERVER_SUCCESS) { + if (!ValidationUtil::ValidateStringIsNumber(gpu_index_str).ok()) { std::cerr << "ERROR: gpu_index " << gpu_index_str << " is not a number" << std::endl; okay = false; } else { int32_t gpu_index = std::stol(gpu_index_str); - if (ValidationUtil::ValidateGpuIndex(gpu_index) != SERVER_SUCCESS) { + if (!ValidationUtil::ValidateGpuIndex(gpu_index).ok()) { std::cerr << "ERROR: invalid gpu_index " << gpu_index_str << std::endl; okay = false; } } - return (okay ? SERVER_SUCCESS : SERVER_INVALID_ARGUMENT); + return (okay ? Status::OK() : Status(SERVER_INVALID_ARGUMENT, "DB config is illegal")); } -ErrorCode +Status ServerConfig::CheckMetricConfig() { /* metric_config: @@ -245,21 +246,21 @@ ServerConfig::CheckMetricConfig() { ConfigNode metric_config = GetConfig(CONFIG_METRIC); std::string is_startup_str = metric_config.GetValue(CONFIG_METRIC_IS_STARTUP, "off"); - if (ValidationUtil::ValidateStringIsBool(is_startup_str) != SERVER_SUCCESS) { + if (!ValidationUtil::ValidateStringIsBool(is_startup_str).ok()) { std::cerr << "ERROR: invalid is_startup config: " << is_startup_str << std::endl; okay = false; } std::string port_str = metric_config.GetChild(CONFIG_PROMETHEUS).GetValue(CONFIG_METRIC_PROMETHEUS_PORT, "8080"); - if (ValidationUtil::ValidateStringIsNumber(port_str) != SERVER_SUCCESS) { + if (!ValidationUtil::ValidateStringIsNumber(port_str).ok()) { std::cerr << "ERROR: port specified in prometheus_config " << port_str << " is not a number" << std::endl; okay = false; } - return (okay ? SERVER_SUCCESS : SERVER_INVALID_ARGUMENT); + return (okay ? Status::OK() : Status(SERVER_INVALID_ARGUMENT, "Metric config is illegal")); } -ErrorCode +Status ServerConfig::CheckCacheConfig() { /* cache_config: @@ -274,7 +275,7 @@ ServerConfig::CheckCacheConfig() { ConfigNode cache_config = GetConfig(CONFIG_CACHE); std::string cpu_cache_capacity_str = cache_config.GetValue(CONFIG_CPU_CACHE_CAPACITY, "16"); - if (ValidationUtil::ValidateStringIsNumber(cpu_cache_capacity_str) != SERVER_SUCCESS) { + if (!ValidationUtil::ValidateStringIsNumber(cpu_cache_capacity_str).ok()) { std::cerr << "ERROR: cpu_cache_capacity " << cpu_cache_capacity_str << " is not a number" << std::endl; okay = false; } @@ -301,7 +302,7 @@ ServerConfig::CheckCacheConfig() { std::string cpu_cache_free_percent_str = cache_config.GetValue(CACHE_FREE_PERCENT, "0.85"); double cpu_cache_free_percent; - if (ValidationUtil::ValidateStringIsDouble(cpu_cache_free_percent_str, cpu_cache_free_percent) != SERVER_SUCCESS) { + if (!ValidationUtil::ValidateStringIsDouble(cpu_cache_free_percent_str, cpu_cache_free_percent).ok()) { std::cerr << "ERROR: cpu_cache_free_percent " << cpu_cache_free_percent_str << " is not a double" << std::endl; okay = false; } @@ -311,13 +312,13 @@ ServerConfig::CheckCacheConfig() { } std::string insert_cache_immediately_str = cache_config.GetValue(CONFIG_INSERT_CACHE_IMMEDIATELY, "false"); - if (ValidationUtil::ValidateStringIsBool(insert_cache_immediately_str) != SERVER_SUCCESS) { + if (!ValidationUtil::ValidateStringIsBool(insert_cache_immediately_str).ok()) { std::cerr << "ERROR: invalid insert_cache_immediately config: " << insert_cache_immediately_str << std::endl; okay = false; } std::string gpu_cache_capacity_str = cache_config.GetValue(CONFIG_GPU_CACHE_CAPACITY, "0"); - if (ValidationUtil::ValidateStringIsNumber(gpu_cache_capacity_str) != SERVER_SUCCESS) { + if (!ValidationUtil::ValidateStringIsNumber(gpu_cache_capacity_str).ok()) { std::cerr << "ERROR: gpu_cache_capacity " << gpu_cache_capacity_str << " is not a number" << std::endl; okay = false; } @@ -326,7 +327,7 @@ ServerConfig::CheckCacheConfig() { gpu_cache_capacity *= GB; int gpu_index = GetConfig(CONFIG_DB).GetInt32Value(CONFIG_DB_BUILD_INDEX_GPU, 0); size_t gpu_memory; - if (ValidationUtil::GetGpuMemory(gpu_index, gpu_memory) != SERVER_SUCCESS) { + if (!ValidationUtil::GetGpuMemory(gpu_index, gpu_memory).ok()) { std::cerr << "ERROR: could not get gpu memory for device " << gpu_index << std::endl; okay = false; } @@ -342,7 +343,7 @@ ServerConfig::CheckCacheConfig() { std::string gpu_cache_free_percent_str = cache_config.GetValue(GPU_CACHE_FREE_PERCENT, "0.85"); double gpu_cache_free_percent; - if (ValidationUtil::ValidateStringIsDouble(gpu_cache_free_percent_str, gpu_cache_free_percent) != SERVER_SUCCESS) { + if (!ValidationUtil::ValidateStringIsDouble(gpu_cache_free_percent_str, gpu_cache_free_percent).ok()) { std::cerr << "ERROR: gpu_cache_free_percent " << gpu_cache_free_percent_str << " is not a double" << std::endl; okay = false; } @@ -351,10 +352,10 @@ ServerConfig::CheckCacheConfig() { okay = false; } - return (okay ? SERVER_SUCCESS : SERVER_INVALID_ARGUMENT); + return (okay ? Status::OK() : Status(SERVER_INVALID_ARGUMENT, "Cache config is illegal")); } -ErrorCode +Status ServerConfig::CheckEngineConfig() { /* engine_config: @@ -365,13 +366,13 @@ ServerConfig::CheckEngineConfig() { ConfigNode engine_config = GetConfig(CONFIG_ENGINE); std::string use_blas_threshold_str = engine_config.GetValue(CONFIG_DCBT, "20"); - if (ValidationUtil::ValidateStringIsNumber(use_blas_threshold_str) != SERVER_SUCCESS) { + if (!ValidationUtil::ValidateStringIsNumber(use_blas_threshold_str).ok()) { std::cerr << "ERROR: use_blas_threshold " << use_blas_threshold_str << " is not a number" << std::endl; okay = false; } std::string omp_thread_num_str = engine_config.GetValue(CONFIG_OMP_THREAD_NUM, "0"); - if (ValidationUtil::ValidateStringIsNumber(omp_thread_num_str) != SERVER_SUCCESS) { + if (!ValidationUtil::ValidateStringIsNumber(omp_thread_num_str).ok()) { std::cerr << "ERROR: omp_thread_num " << omp_thread_num_str << " is not a number" << std::endl; okay = false; } else { @@ -384,10 +385,10 @@ ServerConfig::CheckEngineConfig() { } } - return (okay ? SERVER_SUCCESS : SERVER_INVALID_ARGUMENT); + return (okay ? Status::OK() : Status(SERVER_INVALID_ARGUMENT, "Engine config is illegal")); } -ErrorCode +Status ServerConfig::CheckResourceConfig() { /* resource_config: @@ -410,10 +411,10 @@ ServerConfig::CheckResourceConfig() { okay = false; } - return (okay ? SERVER_SUCCESS : SERVER_INVALID_ARGUMENT); + return (okay ? Status::OK() : Status(SERVER_INVALID_ARGUMENT, "Resource config is illegal")); } -//ErrorCode +//Status //ServerConfig::CheckResourceConfig() { /* resource_config: @@ -484,7 +485,7 @@ ServerConfig::CheckResourceConfig() { // // std::string device_id_str = resource_conf.GetValue(CONFIG_RESOURCE_DEVICE_ID, "0"); // int32_t device_id = -1; -// if (ValidationUtil::ValidateStringIsNumber(device_id_str) != SERVER_SUCCESS) { +// if (!ValidationUtil::ValidateStringIsNumber(device_id_str).ok()) { // std::cerr << "ERROR: device_id " << device_id_str << " is not a number" << std::endl; // okay = false; // } else { @@ -492,7 +493,7 @@ ServerConfig::CheckResourceConfig() { // } // // std::string enable_executor_str = resource_conf.GetValue(CONFIG_RESOURCE_ENABLE_EXECUTOR, "off"); -// if (ValidationUtil::ValidateStringIsBool(enable_executor_str) != SERVER_SUCCESS) { +// if (!ValidationUtil::ValidateStringIsBool(enable_executor_str).ok()) { // std::cerr << "ERROR: invalid enable_executor config: " << enable_executor_str << std::endl; // okay = false; // } @@ -514,26 +515,26 @@ ServerConfig::CheckResourceConfig() { // hasExecutor = true; // } // std::string gpu_resource_num_str = resource_conf.GetValue(CONFIG_RESOURCE_NUM, "2"); -// if (ValidationUtil::ValidateStringIsNumber(gpu_resource_num_str) != SERVER_SUCCESS) { +// if (!ValidationUtil::ValidateStringIsNumber(gpu_resource_num_str).ok()) { // std::cerr << "ERROR: gpu_resource_num " << gpu_resource_num_str << " is not a number" << std::endl; // okay = false; // } // bool mem_valid = true; // std::string pinned_memory_str = resource_conf.GetValue(CONFIG_RESOURCE_PIN_MEMORY, "300"); -// if (ValidationUtil::ValidateStringIsNumber(pinned_memory_str) != SERVER_SUCCESS) { +// if (!ValidationUtil::ValidateStringIsNumber(pinned_memory_str).ok()) { // std::cerr << "ERROR: pinned_memory " << pinned_memory_str << " is not a number" << std::endl; // okay = false; // mem_valid = false; // } // std::string temp_memory_str = resource_conf.GetValue(CONFIG_RESOURCE_TEMP_MEMORY, "300"); -// if (ValidationUtil::ValidateStringIsNumber(temp_memory_str) != SERVER_SUCCESS) { +// if (!ValidationUtil::ValidateStringIsNumber(temp_memory_str).ok()) { // std::cerr << "ERROR: temp_memory " << temp_memory_str << " is not a number" << std::endl; // okay = false; // mem_valid = false; // } // if (mem_valid) { // size_t gpu_memory; -// if (ValidationUtil::GetGpuMemory(device_id, gpu_memory) != SERVER_SUCCESS) { +// if (!ValidationUtil::GetGpuMemory(device_id, gpu_memory).ok()) { // std::cerr << "ERROR: could not get gpu memory for device " << device_id << std::endl; // okay = false; // } @@ -592,8 +593,7 @@ ServerConfig::CheckResourceConfig() { // } // } // -// return (okay ? SERVER_SUCCESS : SERVER_INVALID_ARGUMENT); -// return SERVER_SUCCESS; +// return (okay ? Status::OK() : Status(SERVER_INVALID_ARGUMENT, "Resource config is illegal")); //} void diff --git a/cpp/src/server/ServerConfig.h b/cpp/src/server/ServerConfig.h index d1f414b47b..d0f3e30d6b 100644 --- a/cpp/src/server/ServerConfig.h +++ b/cpp/src/server/ServerConfig.h @@ -17,7 +17,7 @@ #pragma once -#include "utils/Error.h" +#include "utils/Status.h" #include "config/ConfigNode.h" #include "yaml-cpp/yaml.h" @@ -78,20 +78,20 @@ class ServerConfig { public: static ServerConfig &GetInstance(); - ErrorCode LoadConfigFile(const std::string& config_filename); - ErrorCode ValidateConfig(); + Status LoadConfigFile(const std::string& config_filename); + Status ValidateConfig(); void PrintAll() const; ConfigNode GetConfig(const std::string& name) const; ConfigNode& GetConfig(const std::string& name); private: - ErrorCode CheckServerConfig(); - ErrorCode CheckDBConfig(); - ErrorCode CheckMetricConfig(); - ErrorCode CheckCacheConfig(); - ErrorCode CheckEngineConfig(); - ErrorCode CheckResourceConfig(); + Status CheckServerConfig(); + Status CheckDBConfig(); + Status CheckMetricConfig(); + Status CheckCacheConfig(); + Status CheckEngineConfig(); + Status CheckResourceConfig(); }; } diff --git a/cpp/src/server/grpc_impl/GrpcMilvusServer.cpp b/cpp/src/server/grpc_impl/GrpcMilvusServer.cpp index 7b17cd6e56..73f8a7573e 100644 --- a/cpp/src/server/grpc_impl/GrpcMilvusServer.cpp +++ b/cpp/src/server/grpc_impl/GrpcMilvusServer.cpp @@ -58,7 +58,7 @@ class NoReusePortOption : public ::grpc::ServerBuilderOption { }; -void +Status GrpcMilvusServer::StartService() { if (server != nullptr) { std::cout << "stop service!\n"; @@ -92,13 +92,16 @@ GrpcMilvusServer::StartService() { server = builder.BuildAndStart(); server->Wait(); + return Status::OK(); } -void +Status GrpcMilvusServer::StopService() { if (server != nullptr) { server->Shutdown(); } + + return Status::OK(); } } diff --git a/cpp/src/server/grpc_impl/GrpcMilvusServer.h b/cpp/src/server/grpc_impl/GrpcMilvusServer.h index 2ddc5ee2fb..751a2cd2a3 100644 --- a/cpp/src/server/grpc_impl/GrpcMilvusServer.h +++ b/cpp/src/server/grpc_impl/GrpcMilvusServer.h @@ -17,6 +17,8 @@ #pragma once +#include "utils/Status.h" + #include #include @@ -27,10 +29,10 @@ namespace grpc { class GrpcMilvusServer { public: - static void + static Status StartService(); - static void + static Status StopService(); }; diff --git a/cpp/src/server/grpc_impl/GrpcRequestScheduler.cpp b/cpp/src/server/grpc_impl/GrpcRequestScheduler.cpp index 81ef8d81ad..fb8daa3540 100644 --- a/cpp/src/server/grpc_impl/GrpcRequestScheduler.cpp +++ b/cpp/src/server/grpc_impl/GrpcRequestScheduler.cpp @@ -75,8 +75,7 @@ namespace { GrpcBaseTask::GrpcBaseTask(const std::string &task_group, bool async) : task_group_(task_group), async_(async), - done_(false), - error_code_(SERVER_SUCCESS) { + done_(false) { } @@ -84,10 +83,10 @@ GrpcBaseTask::~GrpcBaseTask() { WaitToFinish(); } -ErrorCode GrpcBaseTask::Execute() { - error_code_ = OnExecute(); +Status GrpcBaseTask::Execute() { + status_ = OnExecute(); Done(); - return error_code_; + return status_; } void GrpcBaseTask::Done() { @@ -95,19 +94,17 @@ void GrpcBaseTask::Done() { finish_cond_.notify_all(); } -ErrorCode GrpcBaseTask::SetError(ErrorCode error_code, const std::string &error_msg) { - error_code_ = error_code; - error_msg_ = error_msg; - - SERVER_LOG_ERROR << error_msg_; - return error_code_; +Status GrpcBaseTask::SetStatus(ErrorCode error_code, const std::string &error_msg) { + status_ = Status(error_code, error_msg); + SERVER_LOG_ERROR << error_msg; + return status_; } -ErrorCode GrpcBaseTask::WaitToFinish() { +Status GrpcBaseTask::WaitToFinish() { std::unique_lock lock(finish_mtx_); finish_cond_.wait(lock, [this] { return done_; }); - return error_code_; + return status_; } //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -130,10 +127,10 @@ void GrpcRequestScheduler::ExecTask(BaseTaskPtr &task_ptr, ::milvus::grpc::Statu if (!task_ptr->IsAsync()) { task_ptr->WaitToFinish(); - ErrorCode err = task_ptr->ErrorID(); - if (err != SERVER_SUCCESS) { - grpc_status->set_reason(task_ptr->ErrorMsg()); - grpc_status->set_error_code(ErrorMap(err)); + const Status& status = task_ptr->status(); + if (!status.ok()) { + grpc_status->set_reason(status.message()); + grpc_status->set_error_code(ErrorMap(status.code())); } } } @@ -171,19 +168,19 @@ void GrpcRequestScheduler::Stop() { SERVER_LOG_INFO << "Scheduler stopped"; } -ErrorCode GrpcRequestScheduler::ExecuteTask(const BaseTaskPtr &task_ptr) { +Status GrpcRequestScheduler::ExecuteTask(const BaseTaskPtr &task_ptr) { if (task_ptr == nullptr) { - return SERVER_NULL_POINTER; + return Status::OK(); } - ErrorCode err = PutTaskToQueue(task_ptr); - if (err != SERVER_SUCCESS) { - SERVER_LOG_ERROR << "Put task to queue failed with code: " << err; - return err; + auto status = PutTaskToQueue(task_ptr); + if (!status.ok()) { + SERVER_LOG_ERROR << "Put task to queue failed with code: " << status.ToString(); + return status; } if (task_ptr->IsAsync()) { - return SERVER_SUCCESS;//async execution, caller need to call WaitToFinish at somewhere + return Status::OK(); //async execution, caller need to call WaitToFinish at somewhere } return task_ptr->WaitToFinish();//sync execution @@ -203,9 +200,9 @@ void GrpcRequestScheduler::TakeTaskToExecute(TaskQueuePtr task_queue) { } try { - ErrorCode err = task->Execute(); - if (err != SERVER_SUCCESS) { - SERVER_LOG_ERROR << "Task failed with code: " << err; + auto status = task->Execute(); + if (!status.ok()) { + SERVER_LOG_ERROR << "Task failed with code: " << status.ToString(); } } catch (std::exception &ex) { SERVER_LOG_ERROR << "Task failed to execute: " << ex.what(); @@ -213,7 +210,7 @@ void GrpcRequestScheduler::TakeTaskToExecute(TaskQueuePtr task_queue) { } } -ErrorCode GrpcRequestScheduler::PutTaskToQueue(const BaseTaskPtr &task_ptr) { +Status GrpcRequestScheduler::PutTaskToQueue(const BaseTaskPtr &task_ptr) { std::lock_guard lock(queue_mtx_); std::string group_name = task_ptr->TaskGroup(); @@ -230,7 +227,7 @@ ErrorCode GrpcRequestScheduler::PutTaskToQueue(const BaseTaskPtr &task_ptr) { SERVER_LOG_INFO << "Create new thread for task group: " << group_name; } - return SERVER_SUCCESS; + return Status::OK(); } } diff --git a/cpp/src/server/grpc_impl/GrpcRequestScheduler.h b/cpp/src/server/grpc_impl/GrpcRequestScheduler.h index f8f287b1e6..e1217540fc 100644 --- a/cpp/src/server/grpc_impl/GrpcRequestScheduler.h +++ b/cpp/src/server/grpc_impl/GrpcRequestScheduler.h @@ -17,6 +17,7 @@ #pragma once +#include "utils/Status.h" #include "utils/BlockingQueue.h" #include "status.grpc.pb.h" #include "status.pb.h" @@ -37,24 +38,22 @@ protected: virtual ~GrpcBaseTask(); public: - ErrorCode Execute(); + Status Execute(); void Done(); - ErrorCode WaitToFinish(); + Status WaitToFinish(); std::string TaskGroup() const { return task_group_; } - ErrorCode ErrorID() const { return error_code_; } - - std::string ErrorMsg() const { return error_msg_; } + const Status& status() const { return status_; } bool IsAsync() const { return async_; } protected: - virtual ErrorCode OnExecute() = 0; + virtual Status OnExecute() = 0; - ErrorCode SetError(ErrorCode error_code, const std::string &msg); + Status SetStatus(ErrorCode error_code, const std::string &msg); protected: mutable std::mutex finish_mtx_; @@ -63,8 +62,7 @@ protected: std::string task_group_; bool async_; bool done_; - ErrorCode error_code_; - std::string error_msg_; + Status status_; }; using BaseTaskPtr = std::shared_ptr; @@ -83,7 +81,7 @@ public: void Stop(); - ErrorCode ExecuteTask(const BaseTaskPtr &task_ptr); + Status ExecuteTask(const BaseTaskPtr &task_ptr); static void ExecTask(BaseTaskPtr &task_ptr, ::milvus::grpc::Status *grpc_status); @@ -94,7 +92,7 @@ protected: void TakeTaskToExecute(TaskQueuePtr task_queue); - ErrorCode PutTaskToQueue(const BaseTaskPtr &task_ptr); + Status PutTaskToQueue(const BaseTaskPtr &task_ptr); private: mutable std::mutex queue_mtx_; diff --git a/cpp/src/server/grpc_impl/GrpcRequestTask.cpp b/cpp/src/server/grpc_impl/GrpcRequestTask.cpp index d7b39bea05..e49ea48236 100644 --- a/cpp/src/server/grpc_impl/GrpcRequestTask.cpp +++ b/cpp/src/server/grpc_impl/GrpcRequestTask.cpp @@ -77,33 +77,26 @@ namespace { constexpr long DAY_SECONDS = 24 * 60 * 60; - void + Status ConvertTimeRangeToDBDates(const std::vector<::milvus::grpc::Range> &range_array, - std::vector &dates, - ErrorCode &error_code, - std::string &error_msg) { + std::vector &dates) { dates.clear(); for (auto &range : range_array) { time_t tt_start, tt_end; tm tm_start, tm_end; if (!CommonUtil::TimeStrToTime(range.start_value(), tt_start, tm_start)) { - error_code = SERVER_INVALID_TIME_RANGE; - error_msg = "Invalid time range: " + range.start_value(); - return; + return Status(SERVER_INVALID_TIME_RANGE, "Invalid time range: " + range.start_value()); } if (!CommonUtil::TimeStrToTime(range.end_value(), tt_end, tm_end)) { - error_code = SERVER_INVALID_TIME_RANGE; - error_msg = "Invalid time range: " + range.start_value(); - return; + return Status(SERVER_INVALID_TIME_RANGE, "Invalid time range: " + range.start_value()); } long days = (tt_end > tt_start) ? (tt_end - tt_start) / DAY_SECONDS : (tt_start - tt_end) / DAY_SECONDS; if (days == 0) { - error_code = SERVER_INVALID_TIME_RANGE; - error_msg = "Invalid time range: " + range.start_value() + " to " + range.end_value(); - return; + return Status(SERVER_INVALID_TIME_RANGE, + "Invalid time range: " + range.start_value() + " to " + range.end_value()); } //range: [start_day, end_day) @@ -117,6 +110,8 @@ namespace { dates.push_back(date); } } + + return Status::OK(); } } @@ -136,30 +131,30 @@ CreateTableTask::Create(const ::milvus::grpc::TableSchema *schema) { return std::shared_ptr(new CreateTableTask(schema)); } -ErrorCode +Status CreateTableTask::OnExecute() { TimeRecorder rc("CreateTableTask"); try { //step 1: check arguments - ErrorCode res = ValidationUtil::ValidateTableName(schema_->table_name().table_name()); - if (res != SERVER_SUCCESS) { - return SetError(res, "Invalid table name: " + schema_->table_name().table_name()); + auto status = ValidationUtil::ValidateTableName(schema_->table_name().table_name()); + if (!status.ok()) { + return status; } - res = ValidationUtil::ValidateTableDimension(schema_->dimension()); - if (res != SERVER_SUCCESS) { - return SetError(res, "Invalid table dimension: " + std::to_string(schema_->dimension())); + status = ValidationUtil::ValidateTableDimension(schema_->dimension()); + if (!status.ok()) { + return status; } - res = ValidationUtil::ValidateTableIndexFileSize(schema_->index_file_size()); - if(res != SERVER_SUCCESS) { - return SetError(res, "Invalid index file size: " + std::to_string(schema_->index_file_size())); + status = ValidationUtil::ValidateTableIndexFileSize(schema_->index_file_size()); + if (!status.ok()) { + return status; } - res = ValidationUtil::ValidateTableIndexMetricType(schema_->metric_type()); - if(res != SERVER_SUCCESS) { - return SetError(res, "Invalid index metric type: " + std::to_string(schema_->metric_type())); + status = ValidationUtil::ValidateTableIndexMetricType(schema_->metric_type()); + if (!status.ok()) { + return status; } //step 2: construct table schema @@ -170,22 +165,22 @@ CreateTableTask::OnExecute() { table_info.metric_type_ = schema_->metric_type(); //step 3: create table - auto stat = DBWrapper::DB()->CreateTable(table_info); - if (!stat.ok()) { + status = DBWrapper::DB()->CreateTable(table_info); + if (!status.ok()) { //table could exist - if(stat.code() == DB_ALREADY_EXIST) { - return SetError(SERVER_INVALID_TABLE_NAME, stat.ToString()); + if(status.code() == DB_ALREADY_EXIST) { + return Status(SERVER_INVALID_TABLE_NAME, status.message()); } - return SetError(DB_META_TRANSACTION_FAILED, stat.ToString()); + return status; } } catch (std::exception &ex) { - return SetError(SERVER_UNEXPECTED_ERROR, ex.what()); + return Status(SERVER_UNEXPECTED_ERROR, ex.what()); } rc.ElapseFromBegin("totally cost"); - return SERVER_SUCCESS; + return Status::OK(); } //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -200,23 +195,23 @@ DescribeTableTask::Create(const std::string &table_name, ::milvus::grpc::TableSc return std::shared_ptr(new DescribeTableTask(table_name, schema)); } -ErrorCode +Status DescribeTableTask::OnExecute() { TimeRecorder rc("DescribeTableTask"); try { //step 1: check arguments - ErrorCode res = ValidationUtil::ValidateTableName(table_name_); - if (res != SERVER_SUCCESS) { - return SetError(res, "Invalid table name: " + table_name_); + auto status = ValidationUtil::ValidateTableName(table_name_); + if (!status.ok()) { + return status; } //step 2: get table info engine::meta::TableSchema table_info; table_info.table_id_ = table_name_; - auto stat = DBWrapper::DB()->DescribeTable(table_info); - if (!stat.ok()) { - return SetError(DB_META_TRANSACTION_FAILED, stat.ToString()); + status = DBWrapper::DB()->DescribeTable(table_info); + if (!status.ok()) { + return status; } schema_->mutable_table_name()->set_table_name(table_info.table_id_); @@ -225,12 +220,12 @@ DescribeTableTask::OnExecute() { schema_->set_metric_type(table_info.metric_type_); } catch (std::exception &ex) { - return SetError(SERVER_UNEXPECTED_ERROR, ex.what()); + return Status(SERVER_UNEXPECTED_ERROR, ex.what()); } rc.ElapseFromBegin("totally cost"); - return SERVER_SUCCESS; + return Status::OK(); } //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -248,54 +243,54 @@ CreateIndexTask::Create(const ::milvus::grpc::IndexParam *index_param) { return std::shared_ptr(new CreateIndexTask(index_param)); } -ErrorCode +Status CreateIndexTask::OnExecute() { try { TimeRecorder rc("CreateIndexTask"); //step 1: check arguments std::string table_name_ = index_param_->table_name().table_name(); - ErrorCode res = ValidationUtil::ValidateTableName(table_name_); - if (res != SERVER_SUCCESS) { - return SetError(res, "Invalid table name: " + table_name_); + auto status = ValidationUtil::ValidateTableName(table_name_); + if (!status.ok()) { + return status; } bool has_table = false; - auto stat = DBWrapper::DB()->HasTable(table_name_, has_table); - if (!stat.ok()) { - return SetError(DB_META_TRANSACTION_FAILED, stat.ToString()); + status = DBWrapper::DB()->HasTable(table_name_, has_table); + if (!status.ok()) { + return status; } if (!has_table) { - return SetError(SERVER_TABLE_NOT_EXIST, "Table " + table_name_ + " not exists"); + return Status(SERVER_TABLE_NOT_EXIST, "Table " + table_name_ + " not exists"); } auto &grpc_index = index_param_->index(); - res = ValidationUtil::ValidateTableIndexType(grpc_index.index_type()); - if(res != SERVER_SUCCESS) { - return SetError(res, "Invalid index type: " + std::to_string(grpc_index.index_type())); + status = ValidationUtil::ValidateTableIndexType(grpc_index.index_type()); + if (!status.ok()) { + return status; } - res = ValidationUtil::ValidateTableIndexNlist(grpc_index.nlist()); - if(res != SERVER_SUCCESS) { - return SetError(res, "Invalid index nlist: " + std::to_string(grpc_index.nlist())); + status = ValidationUtil::ValidateTableIndexNlist(grpc_index.nlist()); + if (!status.ok()) { + return status; } //step 2: check table existence engine::TableIndex index; index.engine_type_ = grpc_index.index_type(); index.nlist_ = grpc_index.nlist(); - stat = DBWrapper::DB()->CreateIndex(table_name_, index); - if (!stat.ok()) { - return SetError(SERVER_BUILD_INDEX_ERROR, stat.ToString()); + status = DBWrapper::DB()->CreateIndex(table_name_, index); + if (!status.ok()) { + return status; } rc.ElapseFromBegin("totally cost"); } catch (std::exception &ex) { - return SetError(SERVER_UNEXPECTED_ERROR, ex.what()); + return Status(SERVER_UNEXPECTED_ERROR, ex.what()); } - return SERVER_SUCCESS; + return Status::OK(); } //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -311,29 +306,29 @@ HasTableTask::Create(const std::string &table_name, bool &has_table) { return std::shared_ptr(new HasTableTask(table_name, has_table)); } -ErrorCode +Status HasTableTask::OnExecute() { try { TimeRecorder rc("HasTableTask"); //step 1: check arguments - ErrorCode res = ValidationUtil::ValidateTableName(table_name_); - if (res != SERVER_SUCCESS) { - return SetError(res, "Invalid table name: " + table_name_); + auto status = ValidationUtil::ValidateTableName(table_name_); + if (!status.ok()) { + return status; } //step 2: check table existence - auto stat = DBWrapper::DB()->HasTable(table_name_, has_table_); - if (!stat.ok()) { - return SetError(DB_META_TRANSACTION_FAILED, stat.ToString()); + status = DBWrapper::DB()->HasTable(table_name_, has_table_); + if (!status.ok()) { + return status; } rc.ElapseFromBegin("totally cost"); } catch (std::exception &ex) { - return SetError(SERVER_UNEXPECTED_ERROR, ex.what()); + return Status(SERVER_UNEXPECTED_ERROR, ex.what()); } - return SERVER_SUCCESS; + return Status::OK(); } //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -348,26 +343,26 @@ DropTableTask::Create(const std::string &table_name) { return std::shared_ptr(new DropTableTask(table_name)); } -ErrorCode +Status DropTableTask::OnExecute() { try { TimeRecorder rc("DropTableTask"); //step 1: check arguments - ErrorCode res = ValidationUtil::ValidateTableName(table_name_); - if (res != SERVER_SUCCESS) { - return SetError(res, "Invalid table name: " + table_name_); + auto status = ValidationUtil::ValidateTableName(table_name_); + if (!status.ok()) { + return status; } //step 2: check table existence engine::meta::TableSchema table_info; table_info.table_id_ = table_name_; - auto stat = DBWrapper::DB()->DescribeTable(table_info); - if (!stat.ok()) { - if (stat.code() == DB_NOT_FOUND) { - return SetError(SERVER_TABLE_NOT_EXIST, "Table " + table_name_ + " not exists"); + status = DBWrapper::DB()->DescribeTable(table_info); + if (!status.ok()) { + if (status.code() == DB_NOT_FOUND) { + return Status(SERVER_TABLE_NOT_EXIST, "Table " + table_name_ + " not exists"); } else { - return SetError(DB_META_TRANSACTION_FAILED, stat.ToString()); + return status; } } @@ -375,17 +370,17 @@ DropTableTask::OnExecute() { //step 3: Drop table std::vector dates; - stat = DBWrapper::DB()->DeleteTable(table_name_, dates); - if (!stat.ok()) { - return SetError(DB_META_TRANSACTION_FAILED, stat.ToString()); + status = DBWrapper::DB()->DeleteTable(table_name_, dates); + if (!status.ok()) { + return status; } rc.ElapseFromBegin("total cost"); } catch (std::exception &ex) { - return SetError(SERVER_UNEXPECTED_ERROR, ex.what()); + return Status(SERVER_UNEXPECTED_ERROR, ex.what()); } - return SERVER_SUCCESS; + return Status::OK(); } //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -400,22 +395,22 @@ ShowTablesTask::Create(::grpc::ServerWriter<::milvus::grpc::TableName> *writer) return std::shared_ptr(new ShowTablesTask(writer)); } -ErrorCode +Status ShowTablesTask::OnExecute() { std::vector schema_array; - auto stat = DBWrapper::DB()->AllTables(schema_array); - if (!stat.ok()) { - return SetError(DB_META_TRANSACTION_FAILED, stat.ToString()); + auto statuts = DBWrapper::DB()->AllTables(schema_array); + if (!statuts.ok()) { + return statuts; } for (auto &schema : schema_array) { ::milvus::grpc::TableName tableName; tableName.set_table_name(schema.table_id_); if (!writer_->Write(tableName)) { - return SetError(SERVER_WRITE_ERROR, "Write table name failed!"); + return Status(SERVER_WRITE_ERROR, "Write table name failed!"); } } - return SERVER_SUCCESS; + return Status::OK(); } //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -436,23 +431,23 @@ InsertTask::Create(const ::milvus::grpc::InsertParam *insert_param, return std::shared_ptr(new InsertTask(insert_param, record_ids)); } -ErrorCode +Status InsertTask::OnExecute() { try { TimeRecorder rc("InsertVectorTask"); //step 1: check arguments - ErrorCode res = ValidationUtil::ValidateTableName(insert_param_->table_name()); - if (res != SERVER_SUCCESS) { - return SetError(res, "Invalid table name: " + insert_param_->table_name()); + auto status = ValidationUtil::ValidateTableName(insert_param_->table_name()); + if (!status.ok()) { + return status; } if (insert_param_->row_record_array().empty()) { - return SetError(SERVER_INVALID_ROWRECORD_ARRAY, "Row record array is empty"); + return Status(SERVER_INVALID_ROWRECORD_ARRAY, "Row record array is empty"); } if (!record_ids_->vector_id_array().empty()) { if (record_ids_->vector_id_array().size() != insert_param_->row_record_array_size()) { - return SetError(SERVER_ILLEGAL_VECTOR_ID, + return Status(SERVER_ILLEGAL_VECTOR_ID, "Size of vector ids is not equal to row record array size"); } } @@ -460,13 +455,13 @@ InsertTask::OnExecute() { //step 2: check table existence engine::meta::TableSchema table_info; table_info.table_id_ = insert_param_->table_name(); - auto stat = DBWrapper::DB()->DescribeTable(table_info); - if (!stat.ok()) { - if (stat.code() == DB_NOT_FOUND) { - return SetError(SERVER_TABLE_NOT_EXIST, + status = DBWrapper::DB()->DescribeTable(table_info); + if (!status.ok()) { + if (status.code() == DB_NOT_FOUND) { + return Status(SERVER_TABLE_NOT_EXIST, "Table " + insert_param_->table_name() + " not exists"); } else { - return SetError(DB_META_TRANSACTION_FAILED, stat.ToString()); + return status; } } @@ -475,12 +470,12 @@ InsertTask::OnExecute() { bool user_provide_ids = !insert_param_->row_id_array().empty(); //user already provided id before, all insert action require user id if((table_info.flag_ & engine::meta::FLAG_MASK_HAS_USERID) && !user_provide_ids) { - return SetError(SERVER_ILLEGAL_VECTOR_ID, "Table vector ids are user defined, please provide id for this batch"); + return Status(SERVER_ILLEGAL_VECTOR_ID, "Table vector ids are user defined, please provide id for this batch"); } //user didn't provided id before, no need to provide user id if((table_info.flag_ & engine::meta::FLAG_MASK_NO_USERID) && user_provide_ids) { - return SetError(SERVER_ILLEGAL_VECTOR_ID, "Table vector ids are auto generated, no need to provide id for this batch"); + return Status(SERVER_ILLEGAL_VECTOR_ID, "Table vector ids are auto generated, no need to provide id for this batch"); } rc.RecordSection("check validation"); @@ -496,7 +491,7 @@ InsertTask::OnExecute() { // TODO: change to one dimension array in protobuf or use multiple-thread to copy the data for (size_t i = 0; i < insert_param_->row_record_array_size(); i++) { if (insert_param_->row_record_array(i).vector_data().empty()) { - return SetError(SERVER_INVALID_ROWRECORD_ARRAY, "Row record array data is empty"); + return Status(SERVER_INVALID_ROWRECORD_ARRAY, "Row record array data is empty"); } uint64_t vec_dim = insert_param_->row_record_array(i).vector_data().size(); if (vec_dim != table_info.dimension_) { @@ -504,7 +499,7 @@ InsertTask::OnExecute() { std::string error_msg = "Invalid row record dimension: " + std::to_string(vec_dim) + " vs. table dimension:" + std::to_string(table_info.dimension_); - return SetError(error_code, error_msg); + return Status(error_code, error_msg); } memcpy(&vec_f[i * table_info.dimension_], insert_param_->row_record_array(i).vector_data().data(), @@ -522,10 +517,10 @@ InsertTask::OnExecute() { memcpy(target_data, src_data, (size_t)(sizeof(int64_t)*insert_param_->row_id_array_size())); } - stat = DBWrapper::DB()->InsertVectors(insert_param_->table_name(), vec_count, vec_f.data(), vec_ids); + status = DBWrapper::DB()->InsertVectors(insert_param_->table_name(), vec_count, vec_f.data(), vec_ids); rc.ElapseFromBegin("add vectors to engine"); - if (!stat.ok()) { - return SetError(SERVER_CACHE_ERROR, "Cache error: " + stat.ToString()); + if (!status.ok()) { + return status; } for (int64_t id : vec_ids) { record_ids_->add_vector_id_array(id); @@ -535,13 +530,13 @@ InsertTask::OnExecute() { if (ids_size != vec_count) { std::string msg = "Add " + std::to_string(vec_count) + " vectors but only return " + std::to_string(ids_size) + " id"; - return SetError(SERVER_ILLEGAL_VECTOR_ID, msg); + return Status(SERVER_ILLEGAL_VECTOR_ID, msg); } //step 6: update table flag user_provide_ids ? table_info.flag_ |= engine::meta::FLAG_MASK_HAS_USERID : table_info.flag_ |= engine::meta::FLAG_MASK_NO_USERID; - stat = DBWrapper::DB()->UpdateTableFlag(insert_param_->table_name(), table_info.flag_); + status = DBWrapper::DB()->UpdateTableFlag(insert_param_->table_name(), table_info.flag_); #ifdef MILVUS_ENABLE_PROFILING ProfilerStop(); @@ -551,10 +546,10 @@ InsertTask::OnExecute() { rc.ElapseFromBegin("total cost"); } catch (std::exception &ex) { - return SetError(SERVER_UNEXPECTED_ERROR, ex.what()); + return Status(SERVER_UNEXPECTED_ERROR, ex.what()); } - return SERVER_SUCCESS; + return Status::OK(); } //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -580,7 +575,7 @@ SearchTask::Create(const ::milvus::grpc::SearchParam *search_vector_infos, response)); } -ErrorCode +Status SearchTask::OnExecute() { try { int64_t top_k = search_param_->topk(); @@ -591,53 +586,51 @@ SearchTask::OnExecute() { //step 1: check table name std::string table_name_ = search_param_->table_name(); - ErrorCode res = ValidationUtil::ValidateTableName(table_name_); - if (res != SERVER_SUCCESS) { - return SetError(res, "Invalid table name: " + table_name_); + auto status = ValidationUtil::ValidateTableName(table_name_); + if (!status.ok()) { + return status; } //step 2: check table existence engine::meta::TableSchema table_info; table_info.table_id_ = table_name_; - auto stat = DBWrapper::DB()->DescribeTable(table_info); - if (!stat.ok()) { - if (stat.code() == DB_NOT_FOUND) { - return SetError(SERVER_TABLE_NOT_EXIST, "Table " + table_name_ + " not exists"); + status = DBWrapper::DB()->DescribeTable(table_info); + if (!status.ok()) { + if (status.code() == DB_NOT_FOUND) { + return Status(SERVER_TABLE_NOT_EXIST, "Table " + table_name_ + " not exists"); } else { - return SetError(DB_META_TRANSACTION_FAILED, stat.ToString()); + return status; } } //step 3: check search parameter - res = ValidationUtil::ValidateSearchTopk(top_k, table_info); - if (res != SERVER_SUCCESS) { - return SetError(res, "Invalid topk: " + std::to_string(top_k)); + status = ValidationUtil::ValidateSearchTopk(top_k, table_info); + if (!status.ok()) { + return status; } - res = ValidationUtil::ValidateSearchNprobe(nprobe, table_info); - if (res != SERVER_SUCCESS) { - return SetError(res, "Invalid nprobe: " + std::to_string(nprobe)); + status = ValidationUtil::ValidateSearchNprobe(nprobe, table_info); + if (!status.ok()) { + return status; } if (search_param_->query_record_array().empty()) { - return SetError(SERVER_INVALID_ROWRECORD_ARRAY, "Row record array is empty"); + return Status(SERVER_INVALID_ROWRECORD_ARRAY, "Row record array is empty"); } //step 4: check date range, and convert to db dates std::vector dates; - ErrorCode error_code = SERVER_SUCCESS; - std::string error_msg; - std::vector<::milvus::grpc::Range> range_array; for (size_t i = 0; i < search_param_->query_range_array_size(); i++) { range_array.emplace_back(search_param_->query_range_array(i)); } - ConvertTimeRangeToDBDates(range_array, dates, error_code, error_msg); - if (error_code != SERVER_SUCCESS) { - return SetError(error_code, error_msg); + + status = ConvertTimeRangeToDBDates(range_array, dates); + if (!status.ok()) { + return status; } - double span_check = rc.RecordSection("check validation"); + rc.RecordSection("check validation"); //step 5: prepare float data @@ -645,14 +638,14 @@ SearchTask::OnExecute() { std::vector vec_f(record_array_size * table_info.dimension_, 0); for (size_t i = 0; i < record_array_size; i++) { if (search_param_->query_record_array(i).vector_data().empty()) { - return SetError(SERVER_INVALID_ROWRECORD_ARRAY, "Row record array data is empty"); + return Status(SERVER_INVALID_ROWRECORD_ARRAY, "Row record array data is empty"); } uint64_t query_vec_dim = search_param_->query_record_array(i).vector_data().size(); if (query_vec_dim != table_info.dimension_) { ErrorCode error_code = SERVER_INVALID_VECTOR_DIMENSION; std::string error_msg = "Invalid row record dimension: " + std::to_string(query_vec_dim) + " vs. table dimension:" + std::to_string(table_info.dimension_); - return SetError(error_code, error_msg); + return Status(error_code, error_msg); } memcpy(&vec_f[i * table_info.dimension_], @@ -671,11 +664,11 @@ SearchTask::OnExecute() { #endif if (file_id_array_.empty()) { - stat = DBWrapper::DB()->Query(table_name_, (size_t) top_k, record_count, nprobe, vec_f.data(), - dates, results); + status = DBWrapper::DB()->Query(table_name_, (size_t) top_k, record_count, nprobe, + vec_f.data(), dates, results); } else { - stat = DBWrapper::DB()->Query(table_name_, file_id_array_, (size_t) top_k, - record_count, nprobe, vec_f.data(), dates, results); + status = DBWrapper::DB()->Query(table_name_, file_id_array_, (size_t) top_k, + record_count, nprobe, vec_f.data(), dates, results); } #ifdef MILVUS_ENABLE_PROFILING @@ -683,18 +676,18 @@ SearchTask::OnExecute() { #endif rc.RecordSection("search vectors from engine"); - if (!stat.ok()) { - return SetError(stat.code(), stat.ToString()); + if (!status.ok()) { + return status; } if (results.empty()) { - return SERVER_SUCCESS; //empty table + return Status::OK(); //empty table } if (results.size() != record_count) { std::string msg = "Search " + std::to_string(record_count) + " vectors but only return " + std::to_string(results.size()) + " results"; - return SetError(SERVER_ILLEGAL_SEARCH_RESULT, msg); + return Status(SERVER_ILLEGAL_SEARCH_RESULT, msg); } //step 7: construct result array @@ -713,10 +706,10 @@ SearchTask::OnExecute() { } catch (std::exception &ex) { - return SetError(SERVER_UNEXPECTED_ERROR, ex.what()); + return Status(SERVER_UNEXPECTED_ERROR, ex.what()); } - return SERVER_SUCCESS; + return Status::OK(); } //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -732,23 +725,22 @@ CountTableTask::Create(const std::string &table_name, int64_t &row_count) { return std::shared_ptr(new CountTableTask(table_name, row_count)); } -ErrorCode +Status CountTableTask::OnExecute() { try { TimeRecorder rc("GetTableRowCountTask"); //step 1: check arguments - ErrorCode res = SERVER_SUCCESS; - res = ValidationUtil::ValidateTableName(table_name_); - if (res != SERVER_SUCCESS) { - return SetError(res, "Invalid table name: " + table_name_); + auto status = ValidationUtil::ValidateTableName(table_name_); + if (!status.ok()) { + return status; } //step 2: get row count uint64_t row_count = 0; - auto stat = DBWrapper::DB()->GetTableRowCount(table_name_, row_count); - if (!stat.ok()) { - return SetError(DB_META_TRANSACTION_FAILED, stat.ToString()); + status = DBWrapper::DB()->GetTableRowCount(table_name_, row_count); + if (!status.ok()) { + return status; } row_count_ = (int64_t) row_count; @@ -756,10 +748,10 @@ CountTableTask::OnExecute() { rc.ElapseFromBegin("total cost"); } catch (std::exception &ex) { - return SetError(SERVER_UNEXPECTED_ERROR, ex.what()); + return Status(SERVER_UNEXPECTED_ERROR, ex.what()); } - return SERVER_SUCCESS; + return Status::OK(); } //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -775,7 +767,7 @@ CmdTask::Create(const std::string &cmd, std::string &result) { return std::shared_ptr(new CmdTask(cmd, result)); } -ErrorCode +Status CmdTask::OnExecute() { if (cmd_ == "version") { result_ = MILVUS_VERSION; @@ -786,7 +778,7 @@ CmdTask::OnExecute() { result_ = "OK"; } - return SERVER_SUCCESS; + return Status::OK(); } //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -804,27 +796,27 @@ DeleteByRangeTask::Create(const ::milvus::grpc::DeleteByRangeParam *delete_by_ra return std::shared_ptr(new DeleteByRangeTask(delete_by_range_param)); } -ErrorCode +Status DeleteByRangeTask::OnExecute() { try { TimeRecorder rc("DeleteByRangeTask"); //step 1: check arguments std::string table_name = delete_by_range_param_->table_name(); - ErrorCode res = ValidationUtil::ValidateTableName(table_name); - if (res != SERVER_SUCCESS) { - return SetError(res, "Invalid table name: " + table_name); + auto status = ValidationUtil::ValidateTableName(table_name); + if (!status.ok()) { + return status; } //step 2: check table existence engine::meta::TableSchema table_info; table_info.table_id_ = table_name; - auto stat = DBWrapper::DB()->DescribeTable(table_info); - if (!stat.ok()) { - if (stat.code(), DB_NOT_FOUND) { - return SetError(SERVER_TABLE_NOT_EXIST, "Table " + table_name + " not exists"); + status = DBWrapper::DB()->DescribeTable(table_info); + if (!status.ok()) { + if (status.code(), DB_NOT_FOUND) { + return Status(SERVER_TABLE_NOT_EXIST, "Table " + table_name + " not exists"); } else { - return SetError(DB_META_TRANSACTION_FAILED, stat.ToString()); + return status; } } @@ -837,25 +829,25 @@ DeleteByRangeTask::OnExecute() { std::vector<::milvus::grpc::Range> range_array; range_array.emplace_back(delete_by_range_param_->range()); - ConvertTimeRangeToDBDates(range_array, dates, error_code, error_msg); - if (error_code != SERVER_SUCCESS) { - return SetError(error_code, error_msg); + status = ConvertTimeRangeToDBDates(range_array, dates); + if (!status.ok()) { + return status; } #ifdef MILVUS_ENABLE_PROFILING std::string fname = "/tmp/search_nq_" + this->delete_by_range_param_->table_name() + ".profiling"; ProfilerStart(fname.c_str()); #endif - stat = DBWrapper::DB()->DeleteTable(table_name, dates); - if (!stat.ok()) { - return SetError(DB_META_TRANSACTION_FAILED, stat.ToString()); + status = DBWrapper::DB()->DeleteTable(table_name, dates); + if (!status.ok()) { + return status; } } catch (std::exception &ex) { - return SetError(SERVER_UNEXPECTED_ERROR, ex.what()); + return Status(SERVER_UNEXPECTED_ERROR, ex.what()); } - return SERVER_SUCCESS; + return Status::OK(); } //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -870,29 +862,29 @@ PreloadTableTask::Create(const std::string &table_name){ return std::shared_ptr(new PreloadTableTask(table_name)); } -ErrorCode +Status PreloadTableTask::OnExecute() { try { TimeRecorder rc("PreloadTableTask"); //step 1: check arguments - ErrorCode res = ValidationUtil::ValidateTableName(table_name_); - if (res != SERVER_SUCCESS) { - return SetError(res, "Invalid table name: " + table_name_); + auto status = ValidationUtil::ValidateTableName(table_name_); + if (!status.ok()) { + return status; } //step 2: check table existence - auto stat = DBWrapper::DB()->PreloadTable(table_name_); - if (!stat.ok()) { - return SetError(DB_META_TRANSACTION_FAILED, stat.ToString()); + status = DBWrapper::DB()->PreloadTable(table_name_); + if (!status.ok()) { + return status; } rc.ElapseFromBegin("totally cost"); } catch (std::exception &ex) { - return SetError(SERVER_UNEXPECTED_ERROR, ex.what()); + return Status(SERVER_UNEXPECTED_ERROR, ex.what()); } - return SERVER_SUCCESS; + return Status::OK(); } //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -910,22 +902,22 @@ DescribeIndexTask::Create(const std::string &table_name, return std::shared_ptr(new DescribeIndexTask(table_name, index_param)); } -ErrorCode +Status DescribeIndexTask::OnExecute() { try { TimeRecorder rc("DescribeIndexTask"); //step 1: check arguments - ErrorCode res = ValidationUtil::ValidateTableName(table_name_); - if (res != SERVER_SUCCESS) { - return SetError(res, "Invalid table name: " + table_name_); + auto status = ValidationUtil::ValidateTableName(table_name_); + if (!status.ok()) { + return status; } //step 2: check table existence engine::TableIndex index; - auto stat = DBWrapper::DB()->DescribeIndex(table_name_, index); - if (!stat.ok()) { - return SetError(DB_META_TRANSACTION_FAILED, stat.ToString()); + status = DBWrapper::DB()->DescribeIndex(table_name_, index); + if (!status.ok()) { + return status; } index_param_->mutable_table_name()->set_table_name(table_name_); @@ -934,10 +926,10 @@ DescribeIndexTask::OnExecute() { rc.ElapseFromBegin("totally cost"); } catch (std::exception &ex) { - return SetError(SERVER_UNEXPECTED_ERROR, ex.what()); + return Status(SERVER_UNEXPECTED_ERROR, ex.what()); } - return SERVER_SUCCESS; + return Status::OK(); } //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -952,39 +944,39 @@ DropIndexTask::Create(const std::string &table_name){ return std::shared_ptr(new DropIndexTask(table_name)); } -ErrorCode +Status DropIndexTask::OnExecute() { try { TimeRecorder rc("DropIndexTask"); //step 1: check arguments - ErrorCode res = ValidationUtil::ValidateTableName(table_name_); - if (res != SERVER_SUCCESS) { - return SetError(res, "Invalid table name: " + table_name_); + auto status = ValidationUtil::ValidateTableName(table_name_); + if (!status.ok()) { + return status; } bool has_table = false; - auto stat = DBWrapper::DB()->HasTable(table_name_, has_table); - if (!stat.ok()) { - return SetError(DB_META_TRANSACTION_FAILED, stat.ToString()); + status = DBWrapper::DB()->HasTable(table_name_, has_table); + if (!status.ok()) { + return status; } if (!has_table) { - return SetError(SERVER_TABLE_NOT_EXIST, "Table " + table_name_ + " not exists"); + return Status(SERVER_TABLE_NOT_EXIST, "Table " + table_name_ + " not exists"); } //step 2: check table existence - stat = DBWrapper::DB()->DropIndex(table_name_); - if (!stat.ok()) { - return SetError(DB_META_TRANSACTION_FAILED, stat.ToString()); + status = DBWrapper::DB()->DropIndex(table_name_); + if (!status.ok()) { + return status; } rc.ElapseFromBegin("totally cost"); } catch (std::exception &ex) { - return SetError(SERVER_UNEXPECTED_ERROR, ex.what()); + return Status(SERVER_UNEXPECTED_ERROR, ex.what()); } - return SERVER_SUCCESS; + return Status::OK(); } } diff --git a/cpp/src/server/grpc_impl/GrpcRequestTask.h b/cpp/src/server/grpc_impl/GrpcRequestTask.h index 42a1048124..b7aa4af1d8 100644 --- a/cpp/src/server/grpc_impl/GrpcRequestTask.h +++ b/cpp/src/server/grpc_impl/GrpcRequestTask.h @@ -17,7 +17,7 @@ #pragma once #include "GrpcRequestScheduler.h" -#include "utils/Error.h" +#include "utils/Status.h" #include "db/Types.h" #include "milvus.grpc.pb.h" @@ -41,7 +41,7 @@ protected: explicit CreateTableTask(const ::milvus::grpc::TableSchema *request); - ErrorCode + Status OnExecute() override; private: @@ -57,7 +57,7 @@ public: protected: HasTableTask(const std::string &request, bool &has_table); - ErrorCode + Status OnExecute() override; @@ -75,7 +75,7 @@ public: protected: DescribeTableTask(const std::string &table_name, ::milvus::grpc::TableSchema *schema); - ErrorCode + Status OnExecute() override; @@ -94,7 +94,7 @@ protected: explicit DropTableTask(const std::string &table_name); - ErrorCode + Status OnExecute() override; @@ -112,7 +112,7 @@ protected: explicit CreateIndexTask(const ::milvus::grpc::IndexParam *index_Param); - ErrorCode + Status OnExecute() override; @@ -130,7 +130,7 @@ protected: explicit ShowTablesTask(::grpc::ServerWriter<::milvus::grpc::TableName> *writer); - ErrorCode + Status OnExecute() override; private: @@ -148,7 +148,7 @@ protected: InsertTask(const ::milvus::grpc::InsertParam *insert_Param, ::milvus::grpc::VectorIds *record_ids_); - ErrorCode + Status OnExecute() override; private: @@ -169,7 +169,7 @@ protected: const std::vector &file_id_array, ::milvus::grpc::TopKQueryResultList *response); - ErrorCode + Status OnExecute() override; private: @@ -187,7 +187,7 @@ public: protected: CountTableTask(const std::string &table_name, int64_t &row_count); - ErrorCode + Status OnExecute() override; private: @@ -204,7 +204,7 @@ public: protected: CmdTask(const std::string &cmd, std::string &result); - ErrorCode + Status OnExecute() override; private: @@ -221,7 +221,7 @@ public: protected: DeleteByRangeTask(const ::milvus::grpc::DeleteByRangeParam *delete_by_range_param); - ErrorCode + Status OnExecute() override; private: @@ -237,7 +237,7 @@ public: protected: PreloadTableTask(const std::string &table_name); - ErrorCode + Status OnExecute() override; private: @@ -255,7 +255,7 @@ protected: DescribeIndexTask(const std::string &table_name, ::milvus::grpc::IndexParam *index_param); - ErrorCode + Status OnExecute() override; private: @@ -272,7 +272,7 @@ public: protected: DropIndexTask(const std::string &table_name); - ErrorCode + Status OnExecute() override; private: diff --git a/cpp/src/utils/CommonUtil.cpp b/cpp/src/utils/CommonUtil.cpp index 0f3f7a229e..19a63006cd 100644 --- a/cpp/src/utils/CommonUtil.cpp +++ b/cpp/src/utils/CommonUtil.cpp @@ -73,35 +73,35 @@ bool CommonUtil::IsDirectoryExist(const std::string &path) { return true; } -ErrorCode CommonUtil::CreateDirectory(const std::string &path) { +Status CommonUtil::CreateDirectory(const std::string &path) { if(path.empty()) { - return SERVER_SUCCESS; + return Status::OK(); } struct stat directory_stat; int status = stat(path.c_str(), &directory_stat); if (status == 0) { - return SERVER_SUCCESS;//already exist + return Status::OK();//already exist } fs::path fs_path(path); fs::path parent_path = fs_path.parent_path(); - ErrorCode err = CreateDirectory(parent_path.string()); - if(err != SERVER_SUCCESS){ - return err; + Status err_status = CreateDirectory(parent_path.string()); + if(!err_status.ok()){ + return err_status; } status = stat(path.c_str(), &directory_stat); if (status == 0) { - return SERVER_SUCCESS;//already exist + return Status::OK();//already exist } int makeOK = mkdir(path.c_str(), S_IRWXU|S_IRGRP|S_IROTH); if (makeOK != 0) { - return SERVER_UNEXPECTED_ERROR; + return Status(SERVER_UNEXPECTED_ERROR, "failed to create directory: " + path); } - return SERVER_SUCCESS; + return Status::OK(); } namespace { @@ -134,18 +134,19 @@ namespace { } } -ErrorCode CommonUtil::DeleteDirectory(const std::string &path) { +Status CommonUtil::DeleteDirectory(const std::string &path) { if(path.empty()) { - return SERVER_SUCCESS; + return Status::OK(); } struct stat directory_stat; int statOK = stat(path.c_str(), &directory_stat); - if (statOK != 0) - return SERVER_SUCCESS; + if (statOK != 0) { + return Status::OK(); + } RemoveDirectory(path); - return SERVER_SUCCESS; + return Status::OK(); } bool CommonUtil::IsFileExist(const std::string &path) { diff --git a/cpp/src/utils/CommonUtil.h b/cpp/src/utils/CommonUtil.h index 8c825f70fe..aee57f0446 100755 --- a/cpp/src/utils/CommonUtil.h +++ b/cpp/src/utils/CommonUtil.h @@ -17,10 +17,11 @@ #pragma once +#include "utils/Status.h" + #include #include -#include "Error.h" namespace zilliz { namespace milvus { @@ -34,8 +35,8 @@ class CommonUtil { static bool IsFileExist(const std::string &path); static uint64_t GetFileSize(const std::string &path); static bool IsDirectoryExist(const std::string &path); - static ErrorCode CreateDirectory(const std::string &path); - static ErrorCode DeleteDirectory(const std::string &path); + static Status CreateDirectory(const std::string &path); + static Status DeleteDirectory(const std::string &path); static std::string GetExePath(); diff --git a/cpp/src/utils/Status.cpp b/cpp/src/utils/Status.cpp index 0232c823f0..e21a279a4c 100644 --- a/cpp/src/utils/Status.cpp +++ b/cpp/src/utils/Status.cpp @@ -21,9 +21,9 @@ namespace zilliz { namespace milvus { -constexpr int CODE_WIDTH = sizeof(ErrorCode); +constexpr int CODE_WIDTH = sizeof(StatusCode); -Status::Status(ErrorCode code, const std::string& msg) { +Status::Status(StatusCode code, const std::string& msg) { //4 bytes store code //4 bytes store message length //the left bytes store message string @@ -50,7 +50,8 @@ Status::Status(const Status &s) CopyFrom(s); } -Status &Status::operator=(const Status &s) { +Status& +Status::operator=(const Status &s) { CopyFrom(s); return *this; } @@ -60,12 +61,14 @@ Status::Status(Status &&s) MoveFrom(s); } -Status &Status::operator=(Status &&s) { +Status& +Status::operator=(Status &&s) { MoveFrom(s); return *this; } -void Status::CopyFrom(const Status &s) { +void +Status::CopyFrom(const Status &s) { delete state_; state_ = nullptr; if(s.state_ == nullptr) { @@ -73,19 +76,37 @@ void Status::CopyFrom(const Status &s) { } uint32_t length = 0; - std::memcpy(&length, s.state_ + CODE_WIDTH, sizeof(length)); + memcpy(&length, s.state_ + CODE_WIDTH, sizeof(length)); int buff_len = length + sizeof(length) + CODE_WIDTH; state_ = new char[buff_len]; memcpy((void*)state_, (void*)s.state_, buff_len); } -void Status::MoveFrom(Status &s) { +void +Status::MoveFrom(Status &s) { delete state_; state_ = s.state_; s.state_ = nullptr; } -std::string Status::ToString() const { +std::string +Status::message() const { + if (state_ == nullptr) { + return ""; + } + + std::string msg; + uint32_t length = 0; + memcpy(&length, state_ + CODE_WIDTH, sizeof(length)); + if(length > 0) { + msg.append(state_ + sizeof(length) + CODE_WIDTH, length); + } + + return msg; +} + +std::string +Status::ToString() const { if (state_ == nullptr) { return "OK"; } @@ -115,12 +136,7 @@ std::string Status::ToString() const { break; } - uint32_t length = 0; - memcpy(&length, state_ + CODE_WIDTH, sizeof(length)); - if(length > 0) { - result.append(state_ + sizeof(length) + CODE_WIDTH, length); - } - + result += message(); return result; } diff --git a/cpp/src/utils/Status.h b/cpp/src/utils/Status.h index 153ff61dc5..fe06c8029d 100644 --- a/cpp/src/utils/Status.h +++ b/cpp/src/utils/Status.h @@ -25,9 +25,11 @@ namespace zilliz { namespace milvus { +using StatusCode = ErrorCode; + class Status { public: - Status(ErrorCode code, const std::string &msg); + Status(StatusCode code, const std::string &msg); Status(); ~Status(); @@ -47,14 +49,17 @@ class Status { bool ok() const { return state_ == nullptr || code() == 0; } + StatusCode + code() const { + return (state_ == nullptr) ? 0 : *(StatusCode*)(state_); + } + + std::string + message() const; + std::string ToString() const; - ErrorCode - code() const { - return (state_ == nullptr) ? 0 : *(ErrorCode*)(state_); - } - private: inline void CopyFrom(const Status &s); diff --git a/cpp/src/utils/StringHelpFunctions.cpp b/cpp/src/utils/StringHelpFunctions.cpp index 9c4a6d3a3a..068c376ea0 100644 --- a/cpp/src/utils/StringHelpFunctions.cpp +++ b/cpp/src/utils/StringHelpFunctions.cpp @@ -36,11 +36,11 @@ void StringHelpFunctions::TrimStringQuote(std::string &string, const std::string } } -ErrorCode StringHelpFunctions::SplitStringByDelimeter(const std::string &str, - const std::string &delimeter, - std::vector &result) { +Status StringHelpFunctions::SplitStringByDelimeter(const std::string &str, + const std::string &delimeter, + std::vector &result) { if(str.empty()) { - return SERVER_SUCCESS; + return Status::OK(); } size_t last = 0; @@ -55,13 +55,13 @@ ErrorCode StringHelpFunctions::SplitStringByDelimeter(const std::string &str, result.emplace_back(temp); } - return SERVER_SUCCESS; + return Status::OK(); } - ErrorCode StringHelpFunctions::SplitStringByQuote(const std::string &str, - const std::string &delimeter, - const std::string "e, - std::vector &result) { +Status StringHelpFunctions::SplitStringByQuote(const std::string &str, + const std::string &delimeter, + const std::string "e, + std::vector &result) { if (quote.empty()) { return SplitStringByDelimeter(str, delimeter, result); } @@ -88,7 +88,7 @@ ErrorCode StringHelpFunctions::SplitStringByDelimeter(const std::string &str, std::string postfix = process_str.substr(last); index = postfix.find_first_of(quote, 0); if (index == std::string::npos) { - return SERVER_UNEXPECTED_ERROR; + return Status(SERVER_UNEXPECTED_ERROR, ""); } std::string quoted_text = postfix.substr(0, index); append_prefix += quoted_text; @@ -105,7 +105,7 @@ ErrorCode StringHelpFunctions::SplitStringByDelimeter(const std::string &str, result.emplace_back(append_prefix); if (last == postfix.length()) { - return SERVER_SUCCESS; + return Status::OK(); } process_str = postfix.substr(index + 1); @@ -117,7 +117,7 @@ ErrorCode StringHelpFunctions::SplitStringByDelimeter(const std::string &str, return SplitStringByDelimeter(process_str, delimeter, result); } - return SERVER_SUCCESS; + return Status::OK(); } } diff --git a/cpp/src/utils/StringHelpFunctions.h b/cpp/src/utils/StringHelpFunctions.h index 1fa45322da..d50b62d8a3 100644 --- a/cpp/src/utils/StringHelpFunctions.h +++ b/cpp/src/utils/StringHelpFunctions.h @@ -17,7 +17,7 @@ #pragma once -#include "./Error.h" +#include "utils/Status.h" #include @@ -41,9 +41,9 @@ public: // ,b, | b | // ,, | | // a a - static ErrorCode SplitStringByDelimeter(const std::string &str, - const std::string &delimeter, - std::vector &result); + static Status SplitStringByDelimeter(const std::string &str, + const std::string &delimeter, + std::vector &result); //assume the table has two columns, quote='\"', delimeter=',' // a,b a | b @@ -52,10 +52,10 @@ public: // "aa,bb" aa,bb // 55,1122\"aa,bb\",yyy,\"kkk\" 55 | 1122aa,bb | yyy | kkk // "55,1122"aa,bb",yyy,"kkk" illegal - static ErrorCode SplitStringByQuote(const std::string &str, - const std::string &delimeter, - const std::string "e, - std::vector &result); + static Status SplitStringByQuote(const std::string &str, + const std::string &delimeter, + const std::string "e, + std::vector &result); }; diff --git a/cpp/src/utils/ValidationUtil.cpp b/cpp/src/utils/ValidationUtil.cpp index 2572cbd8bd..c993a4ae14 100644 --- a/cpp/src/utils/ValidationUtil.cpp +++ b/cpp/src/utils/ValidationUtil.cpp @@ -21,9 +21,7 @@ #include "Log.h" #include - #include - #include #include @@ -36,135 +34,156 @@ constexpr size_t TABLE_NAME_SIZE_LIMIT = 255; constexpr int64_t TABLE_DIMENSION_LIMIT = 16384; constexpr int32_t INDEX_FILE_SIZE_LIMIT = 4096; //index trigger size max = 4096 MB -ErrorCode +Status ValidationUtil::ValidateTableName(const std::string &table_name) { // Table name shouldn't be empty. if (table_name.empty()) { - SERVER_LOG_ERROR << "Empty table name"; - return SERVER_INVALID_TABLE_NAME; + std::string msg = "Empty table name"; + SERVER_LOG_ERROR << msg; + return Status(SERVER_INVALID_TABLE_NAME, msg); } // Table name size shouldn't exceed 16384. if (table_name.size() > TABLE_NAME_SIZE_LIMIT) { - SERVER_LOG_ERROR << "Table name size exceed the limitation"; - return SERVER_INVALID_TABLE_NAME; + std::string msg = "Table name size exceed the limitation"; + SERVER_LOG_ERROR << msg; + return Status(SERVER_INVALID_TABLE_NAME, msg); } // Table name first character should be underscore or character. char first_char = table_name[0]; if (first_char != '_' && std::isalpha(first_char) == 0) { - SERVER_LOG_ERROR << "Table name first character isn't underscore or character: " << first_char; - return SERVER_INVALID_TABLE_NAME; + std::string msg = "Table name first character isn't underscore or character"; + SERVER_LOG_ERROR << msg; + return Status(SERVER_INVALID_TABLE_NAME, msg); } int64_t table_name_size = table_name.size(); for (int64_t i = 1; i < table_name_size; ++i) { char name_char = table_name[i]; if (name_char != '_' && std::isalnum(name_char) == 0) { - SERVER_LOG_ERROR << "Table name character isn't underscore or alphanumber: " << name_char; - return SERVER_INVALID_TABLE_NAME; + std::string msg = "Table name character isn't underscore or alphanumber"; + SERVER_LOG_ERROR << msg; + return Status(SERVER_INVALID_TABLE_NAME, msg); } } - return SERVER_SUCCESS; + return Status::OK(); } -ErrorCode +Status ValidationUtil::ValidateTableDimension(int64_t dimension) { if (dimension <= 0 || dimension > TABLE_DIMENSION_LIMIT) { - SERVER_LOG_ERROR << "Table dimension excceed the limitation: " << TABLE_DIMENSION_LIMIT; - return SERVER_INVALID_VECTOR_DIMENSION; + std::string msg = "Table dimension excceed the limitation: " + std::to_string(TABLE_DIMENSION_LIMIT); + SERVER_LOG_ERROR << msg; + return Status(SERVER_INVALID_VECTOR_DIMENSION, msg); } else { - return SERVER_SUCCESS; + return Status::OK(); } } -ErrorCode +Status ValidationUtil::ValidateTableIndexType(int32_t index_type) { int engine_type = (int) engine::EngineType(index_type); if (engine_type <= 0 || engine_type > (int) engine::EngineType::MAX_VALUE) { - return SERVER_INVALID_INDEX_TYPE; + std::string msg = "Invalid index type: " + std::to_string(index_type); + SERVER_LOG_ERROR << msg; + return Status(SERVER_INVALID_INDEX_TYPE, msg); } - return SERVER_SUCCESS; + return Status::OK(); } -ErrorCode +Status ValidationUtil::ValidateTableIndexNlist(int32_t nlist) { if (nlist <= 0) { - return SERVER_INVALID_INDEX_NLIST; + std::string msg = "Invalid nlist value: " + std::to_string(nlist); + SERVER_LOG_ERROR << msg; + return Status(SERVER_INVALID_INDEX_NLIST, msg); } - return SERVER_SUCCESS; + return Status::OK(); } -ErrorCode +Status ValidationUtil::ValidateTableIndexFileSize(int64_t index_file_size) { if (index_file_size <= 0 || index_file_size > INDEX_FILE_SIZE_LIMIT) { - return SERVER_INVALID_INDEX_FILE_SIZE; + std::string msg = "Invalid index file size: " + std::to_string(index_file_size); + SERVER_LOG_ERROR << msg; + return Status(SERVER_INVALID_INDEX_FILE_SIZE, msg); } - return SERVER_SUCCESS; + return Status::OK(); } -ErrorCode +Status ValidationUtil::ValidateTableIndexMetricType(int32_t metric_type) { if (metric_type != (int32_t) engine::MetricType::L2 && metric_type != (int32_t) engine::MetricType::IP) { - return SERVER_INVALID_INDEX_METRIC_TYPE; + std::string msg = "Invalid metric type: " + std::to_string(metric_type); + SERVER_LOG_ERROR << msg; + return Status(SERVER_INVALID_INDEX_METRIC_TYPE, msg); } - return SERVER_SUCCESS; + return Status::OK(); } -ErrorCode +Status ValidationUtil::ValidateSearchTopk(int64_t top_k, const engine::meta::TableSchema &table_schema) { if (top_k <= 0 || top_k > 2048) { - return SERVER_INVALID_TOPK; + std::string msg = "Invalid top k value: " + std::to_string(top_k); + SERVER_LOG_ERROR << msg; + return Status(SERVER_INVALID_TOPK, msg); } - return SERVER_SUCCESS; + return Status::OK(); } -ErrorCode +Status ValidationUtil::ValidateSearchNprobe(int64_t nprobe, const engine::meta::TableSchema &table_schema) { if (nprobe <= 0 || nprobe > table_schema.nlist_) { - return SERVER_INVALID_NPROBE; + std::string msg = "Invalid nprobe value: " + std::to_string(nprobe); + SERVER_LOG_ERROR << msg; + return Status(SERVER_INVALID_NPROBE, msg); } - return SERVER_SUCCESS; + return Status::OK(); } -ErrorCode +Status ValidationUtil::ValidateGpuIndex(uint32_t gpu_index) { int num_devices = 0; auto cuda_err = cudaGetDeviceCount(&num_devices); if (cuda_err) { - SERVER_LOG_ERROR << "Failed to count video card: " << std::to_string(cuda_err); - return SERVER_UNEXPECTED_ERROR; + std::string msg = "Failed to get gpu card number, cuda error:" + std::to_string(cuda_err); + SERVER_LOG_ERROR << msg; + return Status(SERVER_UNEXPECTED_ERROR, msg); } if (gpu_index >= num_devices) { - return SERVER_INVALID_ARGUMENT; + std::string msg = "Invalid gpu index: " + std::to_string(gpu_index); + SERVER_LOG_ERROR << msg; + return Status(SERVER_INVALID_ARGUMENT, msg); } - return SERVER_SUCCESS; + return Status::OK(); } -ErrorCode +Status ValidationUtil::GetGpuMemory(uint32_t gpu_index, size_t &memory) { cudaDeviceProp deviceProp; auto cuda_err = cudaGetDeviceProperties(&deviceProp, gpu_index); if (cuda_err) { - SERVER_LOG_ERROR << "Failed to get video card properties: " << std::to_string(cuda_err); - return SERVER_UNEXPECTED_ERROR; + std::string msg = "Failed to get gpu properties, cuda error:" + std::to_string(cuda_err); + SERVER_LOG_ERROR << msg; + return Status(SERVER_UNEXPECTED_ERROR, msg); } memory = deviceProp.totalGlobalMem; - return SERVER_SUCCESS; + return Status::OK(); } -ErrorCode +Status ValidationUtil::ValidateIpAddress(const std::string &ip_address) { struct in_addr address; @@ -172,50 +191,56 @@ ValidationUtil::ValidateIpAddress(const std::string &ip_address) { int result = inet_pton(AF_INET, ip_address.c_str(), &address); switch (result) { - case 1:return SERVER_SUCCESS; - case 0:SERVER_LOG_ERROR << "Invalid IP address: " << ip_address; - return SERVER_INVALID_ARGUMENT; - default:SERVER_LOG_ERROR << "inet_pton conversion error"; - return SERVER_UNEXPECTED_ERROR; + case 1:return Status::OK(); + case 0: { + std::string msg = "Invalid IP address: " + ip_address; + SERVER_LOG_ERROR << msg; + return Status(SERVER_INVALID_ARGUMENT, msg); + } + default: { + std::string msg = "IP address conversion error: " + ip_address; + SERVER_LOG_ERROR << msg; + return Status(SERVER_UNEXPECTED_ERROR, msg); + } } } -ErrorCode +Status ValidationUtil::ValidateStringIsNumber(const std::string &string) { if (!string.empty() && std::all_of(string.begin(), string.end(), ::isdigit)) { - return SERVER_SUCCESS; + return Status::OK(); } else { - return SERVER_INVALID_ARGUMENT; + return Status(SERVER_INVALID_ARGUMENT, "Not a number"); } } -ErrorCode +Status ValidationUtil::ValidateStringIsBool(std::string &str) { std::transform(str.begin(), str.end(), str.begin(), ::tolower); if (str == "true" || str == "on" || str == "yes" || str == "1" || str == "false" || str == "off" || str == "no" || str == "0" || str.empty()) { - return SERVER_SUCCESS; + return Status::OK(); } else { - return SERVER_INVALID_ARGUMENT; + return Status(SERVER_INVALID_ARGUMENT, "Not a boolean: " + str); } } -ErrorCode +Status ValidationUtil::ValidateStringIsDouble(const std::string &str, double &val) { char *end = nullptr; val = std::strtod(str.c_str(), &end); if (end != str.c_str() && *end == '\0' && val != HUGE_VAL) { - return SERVER_SUCCESS; + return Status::OK(); } else { - return SERVER_INVALID_ARGUMENT; + return Status(SERVER_INVALID_ARGUMENT, "Not a double value: " + str); } } -ErrorCode +Status ValidationUtil::ValidateDbURI(const std::string &uri) { std::string dialectRegex = "(.*)"; std::string usernameRegex = "(.*)"; @@ -256,7 +281,8 @@ ValidationUtil::ValidateDbURI(const std::string &uri) { std::string port = pieces_match[5].str(); if (!port.empty()) { - if (ValidateStringIsNumber(port) != SERVER_SUCCESS) { + auto status = ValidateStringIsNumber(port); + if (!status.ok()) { SERVER_LOG_ERROR << "Invalid port in uri = " << port; okay = false; } @@ -267,7 +293,7 @@ ValidationUtil::ValidateDbURI(const std::string &uri) { okay = false; } - return (okay ? SERVER_SUCCESS : SERVER_INVALID_ARGUMENT); + return (okay ? Status::OK() : Status(SERVER_INVALID_ARGUMENT, "Invalid db backend uri")); } } diff --git a/cpp/src/utils/ValidationUtil.h b/cpp/src/utils/ValidationUtil.h index ed443e9165..0c109e0e25 100644 --- a/cpp/src/utils/ValidationUtil.h +++ b/cpp/src/utils/ValidationUtil.h @@ -19,7 +19,7 @@ #pragma once #include "db/meta/MetaTypes.h" -#include "Error.h" +#include "utils/Status.h" namespace zilliz { namespace milvus { @@ -27,49 +27,49 @@ namespace server { class ValidationUtil { public: - static ErrorCode + static Status ValidateTableName(const std::string &table_name); - static ErrorCode + static Status ValidateTableDimension(int64_t dimension); - static ErrorCode + static Status ValidateTableIndexType(int32_t index_type); - static ErrorCode + static Status ValidateTableIndexNlist(int32_t nlist); - static ErrorCode + static Status ValidateTableIndexFileSize(int64_t index_file_size); - static ErrorCode + static Status ValidateTableIndexMetricType(int32_t metric_type); - static ErrorCode + static Status ValidateSearchTopk(int64_t top_k, const engine::meta::TableSchema& table_schema); - static ErrorCode + static Status ValidateSearchNprobe(int64_t nprobe, const engine::meta::TableSchema& table_schema); - static ErrorCode + static Status ValidateGpuIndex(uint32_t gpu_index); - static ErrorCode + static Status GetGpuMemory(uint32_t gpu_index, size_t &memory); - static ErrorCode + static Status ValidateIpAddress(const std::string &ip_address); - static ErrorCode + static Status ValidateStringIsNumber(const std::string &str); - static ErrorCode + static Status ValidateStringIsBool(std::string &str); - static ErrorCode + static Status ValidateStringIsDouble(const std::string &str, double &val); - static ErrorCode + static Status ValidateDbURI(const std::string &uri); }; diff --git a/cpp/unittest/server/config_test.cpp b/cpp/unittest/server/config_test.cpp index d53d674b37..13089e914b 100644 --- a/cpp/unittest/server/config_test.cpp +++ b/cpp/unittest/server/config_test.cpp @@ -107,11 +107,11 @@ TEST(ConfigTest, CONFIG_TEST) { TEST(ConfigTest, SERVER_CONFIG_TEST) { server::ServerConfig& config = server::ServerConfig::GetInstance(); - ErrorCode err = config.LoadConfigFile(CONFIG_FILE_PATH); - ASSERT_EQ(err, SERVER_SUCCESS); + auto status = config.LoadConfigFile(CONFIG_FILE_PATH); + ASSERT_TRUE(status.ok()); - err = server::ServerConfig::GetInstance().ValidateConfig(); - ASSERT_EQ(err, SERVER_SUCCESS); + status = server::ServerConfig::GetInstance().ValidateConfig(); + ASSERT_TRUE(status.ok()); const server::ServerConfig& config_const = config; server::ConfigNode node1 = config_const.GetConfig("server_config"); @@ -137,23 +137,23 @@ TEST(ConfigTest, SERVER_CONFIG_TEST) { server::ConfigNode& db_config = config.GetConfig("db_config"); server::ConfigNode& cache_config = config.GetConfig(server::CONFIG_CACHE); cache_config.SetValue(server::CACHE_FREE_PERCENT, "2.0"); - err = config.ValidateConfig(); - ASSERT_NE(err, SERVER_SUCCESS); + status = config.ValidateConfig(); + ASSERT_FALSE(status.ok()); size_t cache_cap = 16; size_t insert_buffer_size = (total_mem - cache_cap*GB + 1*GB)/GB; db_config.SetValue(server::CONFIG_DB_INSERT_BUFFER_SIZE, std::to_string(insert_buffer_size)); cache_config.SetValue(server::CONFIG_CPU_CACHE_CAPACITY, std::to_string(cache_cap)); - err = config.ValidateConfig(); - ASSERT_NE(err, SERVER_SUCCESS); + status = config.ValidateConfig(); + ASSERT_FALSE(status.ok()); cache_cap = total_mem/GB + 2; cache_config.SetValue(server::CONFIG_CPU_CACHE_CAPACITY, std::to_string(cache_cap)); - err = config.ValidateConfig(); - ASSERT_NE(err, SERVER_SUCCESS); + status = config.ValidateConfig(); + ASSERT_FALSE(status.ok()); insert_buffer_size = total_mem/GB + 2; db_config.SetValue(server::CONFIG_DB_INSERT_BUFFER_SIZE, std::to_string(insert_buffer_size)); - err = config.ValidateConfig(); - ASSERT_NE(err, SERVER_SUCCESS); + status = config.ValidateConfig(); + ASSERT_FALSE(status.ok()); } \ No newline at end of file diff --git a/cpp/unittest/server/rpc_test.cpp b/cpp/unittest/server/rpc_test.cpp index 09a8a1e461..b87ead099a 100644 --- a/cpp/unittest/server/rpc_test.cpp +++ b/cpp/unittest/server/rpc_test.cpp @@ -431,9 +431,9 @@ TEST_F(RpcHandlerTest, DeleteByRangeTest) { ////////////////////////////////////////////////////////////////////// class DummyTask : public GrpcBaseTask { public: - ErrorCode + Status OnExecute() override { - return 0; + return Status::OK(); } static BaseTaskPtr @@ -441,11 +441,6 @@ class DummyTask : public GrpcBaseTask { return std::shared_ptr(new DummyTask(dummy)); } - ErrorCode - DummySetError(ErrorCode error_code, const std::string &msg) { - return SetError(error_code, msg); - } - public: explicit DummyTask(std::string &dummy) : GrpcBaseTask(dummy) { @@ -464,11 +459,8 @@ class RpcSchedulerTest : public testing::Test { }; TEST_F(RpcSchedulerTest, BaseTaskTest){ - ErrorCode error_code = task_ptr->Execute(); - ASSERT_EQ(error_code, 0); - - error_code = task_ptr->DummySetError(0, "test error"); - ASSERT_EQ(error_code, 0); + auto status = task_ptr->Execute(); + ASSERT_TRUE(status.ok()); GrpcRequestScheduler::GetInstance().Start(); ::milvus::grpc::Status grpc_status; diff --git a/cpp/unittest/server/util_test.cpp b/cpp/unittest/server/util_test.cpp index cfbcbab683..00be2f3f13 100644 --- a/cpp/unittest/server/util_test.cpp +++ b/cpp/unittest/server/util_test.cpp @@ -64,19 +64,19 @@ TEST(UtilTest, COMMON_TEST) { std::string path1 = "/tmp/milvus_test/"; std::string path2 = path1 + "common_test_12345/"; std::string path3 = path2 + "abcdef"; - ErrorCode err = server::CommonUtil::CreateDirectory(path3); - ASSERT_EQ(err, SERVER_SUCCESS); + Status status = server::CommonUtil::CreateDirectory(path3); + ASSERT_TRUE(status.ok()); //test again - err = server::CommonUtil::CreateDirectory(path3); - ASSERT_EQ(err, SERVER_SUCCESS); + status = server::CommonUtil::CreateDirectory(path3); + ASSERT_TRUE(status.ok()); ASSERT_TRUE(server::CommonUtil::IsDirectoryExist(path3)); - err = server::CommonUtil::DeleteDirectory(path1); - ASSERT_EQ(err, SERVER_SUCCESS); + status = server::CommonUtil::DeleteDirectory(path1); + ASSERT_TRUE(status.ok()); //test again - err = server::CommonUtil::DeleteDirectory(path1); - ASSERT_EQ(err, SERVER_SUCCESS); + status = server::CommonUtil::DeleteDirectory(path1); + ASSERT_TRUE(status.ok()); ASSERT_FALSE(server::CommonUtil::IsDirectoryExist(path1)); ASSERT_FALSE(server::CommonUtil::IsFileExist(path1)); @@ -114,24 +114,24 @@ TEST(UtilTest, STRINGFUNCTIONS_TEST) { str = "a,b,c"; std::vector result; - ErrorCode err = server::StringHelpFunctions::SplitStringByDelimeter(str , ",", result); - ASSERT_EQ(err, SERVER_SUCCESS); + auto status = server::StringHelpFunctions::SplitStringByDelimeter(str , ",", result); + ASSERT_TRUE(status.ok()); ASSERT_EQ(result.size(), 3UL); result.clear(); - err = server::StringHelpFunctions::SplitStringByQuote(str , ",", "\"", result); - ASSERT_EQ(err, SERVER_SUCCESS); + status = server::StringHelpFunctions::SplitStringByQuote(str , ",", "\"", result); + ASSERT_TRUE(status.ok()); ASSERT_EQ(result.size(), 3UL); result.clear(); - err = server::StringHelpFunctions::SplitStringByQuote(str , ",", "", result); - ASSERT_EQ(err, SERVER_SUCCESS); + status = server::StringHelpFunctions::SplitStringByQuote(str , ",", "", result); + ASSERT_TRUE(status.ok()); ASSERT_EQ(result.size(), 3UL); str = "55,\"aa,gg,yy\",b"; result.clear(); - err = server::StringHelpFunctions::SplitStringByQuote(str , ",", "\"", result); - ASSERT_EQ(err, SERVER_SUCCESS); + status = server::StringHelpFunctions::SplitStringByQuote(str , ",", "\"", result); + ASSERT_TRUE(status.ok()); ASSERT_EQ(result.size(), 3UL); @@ -227,117 +227,117 @@ TEST(UtilTest, STATUS_TEST) { TEST(ValidationUtilTest, VALIDATE_TABLENAME_TEST) { std::string table_name = "Normal123_"; - ErrorCode res = server::ValidationUtil::ValidateTableName(table_name); - ASSERT_EQ(res, SERVER_SUCCESS); + auto status = server::ValidationUtil::ValidateTableName(table_name); + ASSERT_TRUE(status.ok()); table_name = "12sds"; - res = server::ValidationUtil::ValidateTableName(table_name); - ASSERT_EQ(res, SERVER_INVALID_TABLE_NAME); + status = server::ValidationUtil::ValidateTableName(table_name); + ASSERT_EQ(status.code(), SERVER_INVALID_TABLE_NAME); table_name = ""; - res = server::ValidationUtil::ValidateTableName(table_name); - ASSERT_EQ(res, SERVER_INVALID_TABLE_NAME); + status = server::ValidationUtil::ValidateTableName(table_name); + ASSERT_EQ(status.code(), SERVER_INVALID_TABLE_NAME); table_name = "_asdasd"; - res = server::ValidationUtil::ValidateTableName(table_name); - ASSERT_EQ(res, SERVER_SUCCESS); + status = server::ValidationUtil::ValidateTableName(table_name); + ASSERT_EQ(status.code(), SERVER_SUCCESS); table_name = "!@#!@"; - res = server::ValidationUtil::ValidateTableName(table_name); - ASSERT_EQ(res, SERVER_INVALID_TABLE_NAME); + status = server::ValidationUtil::ValidateTableName(table_name); + ASSERT_EQ(status.code(), SERVER_INVALID_TABLE_NAME); table_name = "_!@#!@"; - res = server::ValidationUtil::ValidateTableName(table_name); - ASSERT_EQ(res, SERVER_INVALID_TABLE_NAME); + status = server::ValidationUtil::ValidateTableName(table_name); + ASSERT_EQ(status.code(), SERVER_INVALID_TABLE_NAME); table_name = "中文"; - res = server::ValidationUtil::ValidateTableName(table_name); - ASSERT_EQ(res, SERVER_INVALID_TABLE_NAME); + status = server::ValidationUtil::ValidateTableName(table_name); + ASSERT_EQ(status.code(), SERVER_INVALID_TABLE_NAME); table_name = std::string(10000, 'a'); - res = server::ValidationUtil::ValidateTableName(table_name); - ASSERT_EQ(res, SERVER_INVALID_TABLE_NAME); + status = server::ValidationUtil::ValidateTableName(table_name); + ASSERT_EQ(status.code(), SERVER_INVALID_TABLE_NAME); } TEST(ValidationUtilTest, VALIDATE_DIMENSION_TEST) { - ASSERT_EQ(server::ValidationUtil::ValidateTableDimension(-1), SERVER_INVALID_VECTOR_DIMENSION); - ASSERT_EQ(server::ValidationUtil::ValidateTableDimension(0), SERVER_INVALID_VECTOR_DIMENSION); - ASSERT_EQ(server::ValidationUtil::ValidateTableDimension(16385), SERVER_INVALID_VECTOR_DIMENSION); - ASSERT_EQ(server::ValidationUtil::ValidateTableDimension(16384), SERVER_SUCCESS); - ASSERT_EQ(server::ValidationUtil::ValidateTableDimension(1), SERVER_SUCCESS); + ASSERT_EQ(server::ValidationUtil::ValidateTableDimension(-1).code(), SERVER_INVALID_VECTOR_DIMENSION); + ASSERT_EQ(server::ValidationUtil::ValidateTableDimension(0).code(), SERVER_INVALID_VECTOR_DIMENSION); + ASSERT_EQ(server::ValidationUtil::ValidateTableDimension(16385).code(), SERVER_INVALID_VECTOR_DIMENSION); + ASSERT_EQ(server::ValidationUtil::ValidateTableDimension(16384).code(), SERVER_SUCCESS); + ASSERT_EQ(server::ValidationUtil::ValidateTableDimension(1).code(), SERVER_SUCCESS); } TEST(ValidationUtilTest, VALIDATE_INDEX_TEST) { - ASSERT_EQ(server::ValidationUtil::ValidateTableIndexType((int)engine::EngineType::INVALID), SERVER_INVALID_INDEX_TYPE); + ASSERT_EQ(server::ValidationUtil::ValidateTableIndexType((int)engine::EngineType::INVALID).code(), SERVER_INVALID_INDEX_TYPE); for(int i = 1; i <= (int)engine::EngineType::MAX_VALUE; i++) { - ASSERT_EQ(server::ValidationUtil::ValidateTableIndexType(i), SERVER_SUCCESS); + ASSERT_EQ(server::ValidationUtil::ValidateTableIndexType(i).code(), SERVER_SUCCESS); } - ASSERT_EQ(server::ValidationUtil::ValidateTableIndexType((int)engine::EngineType::MAX_VALUE + 1), SERVER_INVALID_INDEX_TYPE); + ASSERT_EQ(server::ValidationUtil::ValidateTableIndexType((int)engine::EngineType::MAX_VALUE + 1).code(), SERVER_INVALID_INDEX_TYPE); - ASSERT_EQ(server::ValidationUtil::ValidateTableIndexNlist(0), SERVER_INVALID_INDEX_NLIST); - ASSERT_EQ(server::ValidationUtil::ValidateTableIndexNlist(100), SERVER_SUCCESS); + ASSERT_EQ(server::ValidationUtil::ValidateTableIndexNlist(0).code(), SERVER_INVALID_INDEX_NLIST); + ASSERT_EQ(server::ValidationUtil::ValidateTableIndexNlist(100).code(), SERVER_SUCCESS); - ASSERT_EQ(server::ValidationUtil::ValidateTableIndexFileSize(0), SERVER_INVALID_INDEX_FILE_SIZE); - ASSERT_EQ(server::ValidationUtil::ValidateTableIndexFileSize(100), SERVER_SUCCESS); + ASSERT_EQ(server::ValidationUtil::ValidateTableIndexFileSize(0).code(), SERVER_INVALID_INDEX_FILE_SIZE); + ASSERT_EQ(server::ValidationUtil::ValidateTableIndexFileSize(100).code(), SERVER_SUCCESS); - ASSERT_EQ(server::ValidationUtil::ValidateTableIndexMetricType(0), SERVER_INVALID_INDEX_METRIC_TYPE); - ASSERT_EQ(server::ValidationUtil::ValidateTableIndexMetricType(1), SERVER_SUCCESS); - ASSERT_EQ(server::ValidationUtil::ValidateTableIndexMetricType(2), SERVER_SUCCESS); + ASSERT_EQ(server::ValidationUtil::ValidateTableIndexMetricType(0).code(), SERVER_INVALID_INDEX_METRIC_TYPE); + ASSERT_EQ(server::ValidationUtil::ValidateTableIndexMetricType(1).code(), SERVER_SUCCESS); + ASSERT_EQ(server::ValidationUtil::ValidateTableIndexMetricType(2).code(), SERVER_SUCCESS); } TEST(ValidationUtilTest, VALIDATE_TOPK_TEST) { engine::meta::TableSchema schema; - ASSERT_EQ(server::ValidationUtil::ValidateSearchTopk(10, schema), SERVER_SUCCESS); - ASSERT_NE(server::ValidationUtil::ValidateSearchTopk(65536, schema), SERVER_SUCCESS); - ASSERT_NE(server::ValidationUtil::ValidateSearchTopk(0, schema), SERVER_SUCCESS); + ASSERT_EQ(server::ValidationUtil::ValidateSearchTopk(10, schema).code(), SERVER_SUCCESS); + ASSERT_NE(server::ValidationUtil::ValidateSearchTopk(65536, schema).code(), SERVER_SUCCESS); + ASSERT_NE(server::ValidationUtil::ValidateSearchTopk(0, schema).code(), SERVER_SUCCESS); } TEST(ValidationUtilTest, VALIDATE_NPROBE_TEST) { engine::meta::TableSchema schema; schema.nlist_ = 100; - ASSERT_EQ(server::ValidationUtil::ValidateSearchNprobe(10, schema), SERVER_SUCCESS); - ASSERT_NE(server::ValidationUtil::ValidateSearchNprobe(0, schema), SERVER_SUCCESS); - ASSERT_NE(server::ValidationUtil::ValidateSearchNprobe(101, schema), SERVER_SUCCESS); + ASSERT_EQ(server::ValidationUtil::ValidateSearchNprobe(10, schema).code(), SERVER_SUCCESS); + ASSERT_NE(server::ValidationUtil::ValidateSearchNprobe(0, schema).code(), SERVER_SUCCESS); + ASSERT_NE(server::ValidationUtil::ValidateSearchNprobe(101, schema).code(), SERVER_SUCCESS); } TEST(ValidationUtilTest, VALIDATE_GPU_TEST) { - ASSERT_EQ(server::ValidationUtil::ValidateGpuIndex(0), SERVER_SUCCESS); - ASSERT_NE(server::ValidationUtil::ValidateGpuIndex(100), SERVER_SUCCESS); + ASSERT_EQ(server::ValidationUtil::ValidateGpuIndex(0).code(), SERVER_SUCCESS); + ASSERT_NE(server::ValidationUtil::ValidateGpuIndex(100).code(), SERVER_SUCCESS); size_t memory = 0; - ASSERT_EQ(server::ValidationUtil::GetGpuMemory(0, memory), SERVER_SUCCESS); - ASSERT_NE(server::ValidationUtil::GetGpuMemory(100, memory), SERVER_SUCCESS); + ASSERT_EQ(server::ValidationUtil::GetGpuMemory(0, memory).code(), SERVER_SUCCESS); + ASSERT_NE(server::ValidationUtil::GetGpuMemory(100, memory).code(), SERVER_SUCCESS); } TEST(ValidationUtilTest, VALIDATE_IPADDRESS_TEST) { - ASSERT_EQ(server::ValidationUtil::ValidateIpAddress("127.0.0.1"), SERVER_SUCCESS); - ASSERT_NE(server::ValidationUtil::ValidateIpAddress("not ip"), SERVER_SUCCESS); + ASSERT_EQ(server::ValidationUtil::ValidateIpAddress("127.0.0.1").code(), SERVER_SUCCESS); + ASSERT_NE(server::ValidationUtil::ValidateIpAddress("not ip").code(), SERVER_SUCCESS); } TEST(ValidationUtilTest, VALIDATE_NUMBER_TEST) { - ASSERT_EQ(server::ValidationUtil::ValidateStringIsNumber("1234"), SERVER_SUCCESS); - ASSERT_NE(server::ValidationUtil::ValidateStringIsNumber("not number"), SERVER_SUCCESS); + ASSERT_EQ(server::ValidationUtil::ValidateStringIsNumber("1234").code(), SERVER_SUCCESS); + ASSERT_NE(server::ValidationUtil::ValidateStringIsNumber("not number").code(), SERVER_SUCCESS); } TEST(ValidationUtilTest, VALIDATE_BOOL_TEST) { std::string str = "true"; - ASSERT_EQ(server::ValidationUtil::ValidateStringIsBool(str), SERVER_SUCCESS); + ASSERT_EQ(server::ValidationUtil::ValidateStringIsBool(str).code(), SERVER_SUCCESS); str = "not bool"; - ASSERT_NE(server::ValidationUtil::ValidateStringIsBool(str), SERVER_SUCCESS); + ASSERT_NE(server::ValidationUtil::ValidateStringIsBool(str).code(), SERVER_SUCCESS); } TEST(ValidationUtilTest, VALIDATE_DOUBLE_TEST) { double ret = 0.0; - ASSERT_EQ(server::ValidationUtil::ValidateStringIsDouble("2.5", ret), SERVER_SUCCESS); - ASSERT_NE(server::ValidationUtil::ValidateStringIsDouble("not double", ret), SERVER_SUCCESS); + ASSERT_EQ(server::ValidationUtil::ValidateStringIsDouble("2.5", ret).code(), SERVER_SUCCESS); + ASSERT_NE(server::ValidationUtil::ValidateStringIsDouble("not double", ret).code(), SERVER_SUCCESS); } TEST(ValidationUtilTest, VALIDATE_DBURI_TEST) { - ASSERT_EQ(server::ValidationUtil::ValidateDbURI("sqlite://:@:/"), SERVER_SUCCESS); - ASSERT_NE(server::ValidationUtil::ValidateDbURI("xxx://:@:/"), SERVER_SUCCESS); - ASSERT_NE(server::ValidationUtil::ValidateDbURI("not uri"), SERVER_SUCCESS); - ASSERT_EQ(server::ValidationUtil::ValidateDbURI("mysql://root:123456@127.0.0.1:3303/milvus"), SERVER_SUCCESS); - ASSERT_NE(server::ValidationUtil::ValidateDbURI("mysql://root:123456@127.0.0.1:port/milvus"), SERVER_SUCCESS); + ASSERT_EQ(server::ValidationUtil::ValidateDbURI("sqlite://:@:/").code(), SERVER_SUCCESS); + ASSERT_NE(server::ValidationUtil::ValidateDbURI("xxx://:@:/").code(), SERVER_SUCCESS); + ASSERT_NE(server::ValidationUtil::ValidateDbURI("not uri").code(), SERVER_SUCCESS); + ASSERT_EQ(server::ValidationUtil::ValidateDbURI("mysql://root:123456@127.0.0.1:3303/milvus").code(), SERVER_SUCCESS); + ASSERT_NE(server::ValidationUtil::ValidateDbURI("mysql://root:123456@127.0.0.1:port/milvus").code(), SERVER_SUCCESS); } TEST(UtilTest, ROLLOUTHANDLER_TEST){ From 8eaf03a279b1873f6df7791ec39445a640085c2f Mon Sep 17 00:00:00 2001 From: starlord Date: Wed, 18 Sep 2019 15:05:51 +0800 Subject: [PATCH 4/4] refine code Former-commit-id: 710c122428d52f56a7bf41903afa4a7f8c6db01e --- cpp/src/db/DBFactory.cpp | 6 +- cpp/src/db/DBFactory.h | 4 +- cpp/src/db/DBImpl.cpp | 8 +- cpp/src/db/DBImpl.h | 40 +++++----- cpp/src/db/Options.cpp | 3 - cpp/src/db/Options.h | 5 +- cpp/src/db/insert/MemManagerImpl.h | 4 +- cpp/src/db/insert/MemMenagerFactory.cpp | 3 +- cpp/src/db/insert/MemMenagerFactory.h | 2 +- cpp/src/db/insert/MemTable.cpp | 2 +- cpp/src/db/insert/MemTable.h | 4 +- cpp/src/db/insert/MemTableFile.cpp | 2 +- cpp/src/db/insert/MemTableFile.h | 4 +- cpp/src/db/meta/MySQLMetaImpl.cpp | 4 +- cpp/src/main.cpp | 6 +- cpp/src/server/DBWrapper.cpp | 11 ++- cpp/src/server/Server.cpp | 4 +- cpp/src/server/Server.h | 15 ++-- cpp/src/server/grpc_impl/GrpcMilvusServer.cpp | 8 +- cpp/src/utils/BlockingQueue.inl | 28 +------ cpp/src/utils/CommonUtil.cpp | 5 ++ cpp/src/utils/CommonUtil.h | 1 + cpp/src/utils/LogUtil.cpp | 40 +--------- cpp/src/utils/LogUtil.h | 10 +-- cpp/src/utils/SignalUtil.cpp | 18 ++--- cpp/src/utils/ValidationUtil.h | 3 + cpp/unittest/db/mysql_meta_test.cpp | 4 +- cpp/unittest/db/utils.cpp | 8 +- cpp/unittest/db/utils.h | 8 +- cpp/unittest/metrics/utils.cpp | 2 +- cpp/unittest/metrics/utils.h | 2 +- cpp/unittest/server/rpc_test.cpp | 80 +++++++++---------- cpp/unittest/server/util_test.cpp | 11 ++- 33 files changed, 150 insertions(+), 205 deletions(-) diff --git a/cpp/src/db/DBFactory.cpp b/cpp/src/db/DBFactory.cpp index eb13c309fc..91e03278d6 100644 --- a/cpp/src/db/DBFactory.cpp +++ b/cpp/src/db/DBFactory.cpp @@ -33,14 +33,14 @@ namespace zilliz { namespace milvus { namespace engine { -Options DBFactory::BuildOption() { +DBOptions DBFactory::BuildOption() { auto meta = MetaFactory::BuildOption(); - Options options; + DBOptions options; options.meta = meta; return options; } -DBPtr DBFactory::Build(const Options& options) { +DBPtr DBFactory::Build(const DBOptions& options) { return std::make_shared(options); } diff --git a/cpp/src/db/DBFactory.h b/cpp/src/db/DBFactory.h index 7de4396846..bb363118ed 100644 --- a/cpp/src/db/DBFactory.h +++ b/cpp/src/db/DBFactory.h @@ -29,9 +29,9 @@ namespace engine { class DBFactory { public: - static Options BuildOption(); + static DBOptions BuildOption(); - static DBPtr Build(const Options& options); + static DBPtr Build(const DBOptions& options); }; diff --git a/cpp/src/db/DBImpl.cpp b/cpp/src/db/DBImpl.cpp index 648038de25..0dec98b92b 100644 --- a/cpp/src/db/DBImpl.cpp +++ b/cpp/src/db/DBImpl.cpp @@ -51,7 +51,7 @@ constexpr uint64_t INDEX_ACTION_INTERVAL = 1; } -DBImpl::DBImpl(const Options& options) +DBImpl::DBImpl(const DBOptions& options) : options_(options), shutting_down_(true), compact_thread_pool_(1, 1), @@ -77,7 +77,7 @@ Status DBImpl::Start() { shutting_down_.store(false, std::memory_order_release); //for distribute version, some nodes are read only - if (options_.mode != Options::MODE::READ_ONLY) { + if (options_.mode != DBOptions::MODE::READ_ONLY) { ENGINE_LOG_TRACE << "StartTimerTasks"; bg_timer_thread_ = std::thread(&DBImpl::BackgroundTimerTask, this); } @@ -98,7 +98,7 @@ Status DBImpl::Stop() { //wait compaction/buildindex finish bg_timer_thread_.join(); - if (options_.mode != Options::MODE::READ_ONLY) { + if (options_.mode != DBOptions::MODE::READ_ONLY) { meta_ptr_->CleanUp(); } @@ -687,7 +687,7 @@ void DBImpl::BackgroundCompaction(std::set table_ids) { meta_ptr_->Archive(); int ttl = 5*meta::M_SEC;//default: file will be deleted after 5 minutes - if (options_.mode == Options::MODE::CLUSTER) { + if (options_.mode == DBOptions::MODE::CLUSTER) { ttl = meta::D_SEC; } meta_ptr_->CleanUpFilesWithTTL(ttl); diff --git a/cpp/src/db/DBImpl.h b/cpp/src/db/DBImpl.h index 85803b763f..30e9667994 100644 --- a/cpp/src/db/DBImpl.h +++ b/cpp/src/db/DBImpl.h @@ -44,7 +44,7 @@ class Meta; class DBImpl : public DB { public: - explicit DBImpl(const Options &options); + explicit DBImpl(const DBOptions &options); ~DBImpl(); Status Start() override; @@ -76,28 +76,28 @@ class DBImpl : public DB { Status DropIndex(const std::string& table_id) override; Status Query(const std::string &table_id, - uint64_t k, - uint64_t nq, - uint64_t nprobe, - const float *vectors, - QueryResults &results) override; + uint64_t k, + uint64_t nq, + uint64_t nprobe, + const float *vectors, + QueryResults &results) override; Status Query(const std::string &table_id, - uint64_t k, - uint64_t nq, - uint64_t nprobe, - const float *vectors, - const meta::DatesT &dates, - QueryResults &results) override; + uint64_t k, + uint64_t nq, + uint64_t nprobe, + const float *vectors, + const meta::DatesT &dates, + QueryResults &results) override; Status Query(const std::string &table_id, - const std::vector &file_ids, - uint64_t k, - uint64_t nq, - uint64_t nprobe, - const float *vectors, - const meta::DatesT &dates, - QueryResults &results) override; + const std::vector &file_ids, + uint64_t k, + uint64_t nq, + uint64_t nprobe, + const float *vectors, + const meta::DatesT &dates, + QueryResults &results) override; Status Size(uint64_t &result) override; @@ -130,7 +130,7 @@ class DBImpl : public DB { Status MemSerialize(); private: - const Options options_; + const DBOptions options_; std::atomic shutting_down_; diff --git a/cpp/src/db/Options.cpp b/cpp/src/db/Options.cpp index 9abc8e66ec..5796b781b2 100644 --- a/cpp/src/db/Options.cpp +++ b/cpp/src/db/Options.cpp @@ -27,9 +27,6 @@ namespace zilliz { namespace milvus { namespace engine { -Options::Options() { -} - ArchiveConf::ArchiveConf(const std::string& type, const std::string& criterias) { ParseType(type); ParseCritirias(criterias); diff --git a/cpp/src/db/Options.h b/cpp/src/db/Options.h index c5b4648c6d..663ac3fd09 100644 --- a/cpp/src/db/Options.h +++ b/cpp/src/db/Options.h @@ -58,16 +58,13 @@ struct DBMetaOptions { ArchiveConf archive_conf = ArchiveConf("delete"); }; // DBMetaOptions -struct Options { - +struct DBOptions { typedef enum { SINGLE, CLUSTER, READ_ONLY } MODE; - Options(); - uint16_t merge_trigger_number = 2; DBMetaOptions meta; int mode = MODE::SINGLE; diff --git a/cpp/src/db/insert/MemManagerImpl.h b/cpp/src/db/insert/MemManagerImpl.h index 51e8f1044e..767dc0eef0 100644 --- a/cpp/src/db/insert/MemManagerImpl.h +++ b/cpp/src/db/insert/MemManagerImpl.h @@ -38,7 +38,7 @@ class MemManagerImpl : public MemManager { public: using Ptr = std::shared_ptr; - MemManagerImpl(const meta::MetaPtr &meta, const Options &options) + MemManagerImpl(const meta::MetaPtr &meta, const DBOptions &options) : meta_(meta), options_(options) {} Status InsertVectors(const std::string &table_id, @@ -66,7 +66,7 @@ class MemManagerImpl : public MemManager { MemIdMap mem_id_map_; MemList immu_mem_list_; meta::MetaPtr meta_; - Options options_; + DBOptions options_; std::mutex mutex_; std::mutex serialization_mtx_; }; // NewMemManager diff --git a/cpp/src/db/insert/MemMenagerFactory.cpp b/cpp/src/db/insert/MemMenagerFactory.cpp index d8620e908c..d0ef51f61f 100644 --- a/cpp/src/db/insert/MemMenagerFactory.cpp +++ b/cpp/src/db/insert/MemMenagerFactory.cpp @@ -31,8 +31,7 @@ namespace zilliz { namespace milvus { namespace engine { -MemManagerPtr MemManagerFactory::Build(const std::shared_ptr& meta, - const Options& options) { +MemManagerPtr MemManagerFactory::Build(const std::shared_ptr& meta, const DBOptions& options) { return std::make_shared(meta, options); } diff --git a/cpp/src/db/insert/MemMenagerFactory.h b/cpp/src/db/insert/MemMenagerFactory.h index 4bf4267c87..d38404f5c0 100644 --- a/cpp/src/db/insert/MemMenagerFactory.h +++ b/cpp/src/db/insert/MemMenagerFactory.h @@ -26,7 +26,7 @@ namespace engine { class MemManagerFactory { public: - static MemManagerPtr Build(const std::shared_ptr &meta, const Options &options); + static MemManagerPtr Build(const std::shared_ptr &meta, const DBOptions &options); }; } diff --git a/cpp/src/db/insert/MemTable.cpp b/cpp/src/db/insert/MemTable.cpp index 7f4f497ba1..4be0695572 100644 --- a/cpp/src/db/insert/MemTable.cpp +++ b/cpp/src/db/insert/MemTable.cpp @@ -26,7 +26,7 @@ namespace engine { MemTable::MemTable(const std::string &table_id, const meta::MetaPtr &meta, - const Options &options) : + const DBOptions &options) : table_id_(table_id), meta_(meta), options_(options) { diff --git a/cpp/src/db/insert/MemTable.h b/cpp/src/db/insert/MemTable.h index def4a6dcca..c1b4125331 100644 --- a/cpp/src/db/insert/MemTable.h +++ b/cpp/src/db/insert/MemTable.h @@ -33,7 +33,7 @@ class MemTable { public: using MemTableFileList = std::vector; - MemTable(const std::string &table_id, const meta::MetaPtr &meta, const Options &options); + MemTable(const std::string &table_id, const meta::MetaPtr &meta, const DBOptions &options); Status Add(VectorSourcePtr &source, IDNumbers &vector_ids); @@ -56,7 +56,7 @@ class MemTable { meta::MetaPtr meta_; - Options options_; + DBOptions options_; std::mutex mutex_; diff --git a/cpp/src/db/insert/MemTableFile.cpp b/cpp/src/db/insert/MemTableFile.cpp index 57ef39c670..12dbbcf49f 100644 --- a/cpp/src/db/insert/MemTableFile.cpp +++ b/cpp/src/db/insert/MemTableFile.cpp @@ -31,7 +31,7 @@ namespace engine { MemTableFile::MemTableFile(const std::string &table_id, const meta::MetaPtr &meta, - const Options &options) : + const DBOptions &options) : table_id_(table_id), meta_(meta), options_(options) { diff --git a/cpp/src/db/insert/MemTableFile.h b/cpp/src/db/insert/MemTableFile.h index 5ac3e15ad2..168fabe7e9 100644 --- a/cpp/src/db/insert/MemTableFile.h +++ b/cpp/src/db/insert/MemTableFile.h @@ -31,7 +31,7 @@ namespace engine { class MemTableFile { public: - MemTableFile(const std::string &table_id, const meta::MetaPtr &meta, const Options &options); + MemTableFile(const std::string &table_id, const meta::MetaPtr &meta, const DBOptions &options); Status Add(const VectorSourcePtr &source, IDNumbers& vector_ids); @@ -53,7 +53,7 @@ class MemTableFile { meta::MetaPtr meta_; - Options options_; + DBOptions options_; size_t current_mem_; diff --git a/cpp/src/db/meta/MySQLMetaImpl.cpp b/cpp/src/db/meta/MySQLMetaImpl.cpp index ccd7e79ada..a07d4c82cf 100644 --- a/cpp/src/db/meta/MySQLMetaImpl.cpp +++ b/cpp/src/db/meta/MySQLMetaImpl.cpp @@ -135,7 +135,7 @@ Status MySQLMetaImpl::Initialize() { ENGINE_LOG_DEBUG << "MySQL connection pool: maximum pool size = " << std::to_string(maxPoolSize); try { - if (mode_ != Options::MODE::READ_ONLY) { + if (mode_ != DBOptions::MODE::READ_ONLY) { CleanUp(); } @@ -621,7 +621,7 @@ Status MySQLMetaImpl::DeleteTable(const std::string &table_id) { } //Scoped Connection - if (mode_ == Options::MODE::CLUSTER) { + if (mode_ == DBOptions::MODE::CLUSTER) { DeleteTableFiles(table_id); } diff --git a/cpp/src/main.cpp b/cpp/src/main.cpp index d9b160d6e0..3065e7d684 100644 --- a/cpp/src/main.cpp +++ b/cpp/src/main.cpp @@ -109,9 +109,9 @@ main(int argc, char *argv[]) { } } - server::Server* server_ptr = server::Server::Instance(); - server_ptr->Init(start_daemonized, pid_filename, config_filename, log_config_file); - return server_ptr->Start(); + server::Server& server = server::Server::Instance(); + server.Init(start_daemonized, pid_filename, config_filename, log_config_file); + return server.Start(); } void diff --git a/cpp/src/server/DBWrapper.cpp b/cpp/src/server/DBWrapper.cpp index 508d9552b1..115a636621 100644 --- a/cpp/src/server/DBWrapper.cpp +++ b/cpp/src/server/DBWrapper.cpp @@ -24,6 +24,7 @@ #include "utils/StringHelpFunctions.h" #include +#include namespace zilliz { namespace milvus { @@ -35,7 +36,7 @@ DBWrapper::DBWrapper() { Status DBWrapper::StartService() { //db config - zilliz::milvus::engine::Options opt; + engine::DBOptions opt; ConfigNode& db_config = ServerConfig::GetInstance().GetConfig(CONFIG_DB); opt.meta.backend_uri = db_config.GetValue(CONFIG_DB_URL); std::string db_path = db_config.GetValue(CONFIG_DB_PATH); @@ -51,13 +52,13 @@ Status DBWrapper::StartService() { ConfigNode& serverConfig = ServerConfig::GetInstance().GetConfig(CONFIG_SERVER); std::string mode = serverConfig.GetValue(CONFIG_CLUSTER_MODE, "single"); if (mode == "single") { - opt.mode = zilliz::milvus::engine::Options::MODE::SINGLE; + opt.mode = engine::DBOptions::MODE::SINGLE; } else if (mode == "cluster") { - opt.mode = zilliz::milvus::engine::Options::MODE::CLUSTER; + opt.mode = engine::DBOptions::MODE::CLUSTER; } else if (mode == "read_only") { - opt.mode = zilliz::milvus::engine::Options::MODE::READ_ONLY; + opt.mode = engine::DBOptions::MODE::READ_ONLY; } else { std::cerr << "ERROR: mode specified in server_config is not one of ['single', 'cluster', 'read_only']" << std::endl; @@ -78,6 +79,8 @@ Status DBWrapper::StartService() { } } + faiss::distance_compute_blas_threshold = engine_config.GetInt32Value(CONFIG_DCBT, 20);//init faiss global variable + //set archive config engine::ArchiveConf::CriteriaT criterial; int64_t disk = db_config.GetInt64Value(CONFIG_DB_ARCHIVE_DISK, 0); diff --git a/cpp/src/server/Server.cpp b/cpp/src/server/Server.cpp index e1e67a112f..252f92a5f0 100644 --- a/cpp/src/server/Server.cpp +++ b/cpp/src/server/Server.cpp @@ -42,10 +42,10 @@ namespace zilliz { namespace milvus { namespace server { -Server * +Server& Server::Instance() { static Server server; - return &server; + return server; } Server::Server() { diff --git a/cpp/src/server/Server.h b/cpp/src/server/Server.h index 43e8b28028..eaf19d6b15 100644 --- a/cpp/src/server/Server.h +++ b/cpp/src/server/Server.h @@ -27,15 +27,19 @@ namespace milvus { namespace server { class Server { - public: - static Server* Instance(); +public: + static Server &Instance(); + + void Init(int64_t daemonized, const std::string &pid_filename, const std::string &config_filename, + const std::string &log_config_file); - void Init(int64_t daemonized, const std::string& pid_filename, const std::string& config_filename, const std::string &log_config_file); int Start(); + void Stop(); - private: +private: Server(); + ~Server(); void Daemonize(); @@ -43,9 +47,10 @@ class Server { ErrorCode LoadConfig(); void StartService(); + void StopService(); - private: +private: int64_t daemonized_ = 0; int pid_fd = -1; std::string pid_filename_; diff --git a/cpp/src/server/grpc_impl/GrpcMilvusServer.cpp b/cpp/src/server/grpc_impl/GrpcMilvusServer.cpp index 73f8a7573e..6770e3111a 100644 --- a/cpp/src/server/grpc_impl/GrpcMilvusServer.cpp +++ b/cpp/src/server/grpc_impl/GrpcMilvusServer.cpp @@ -17,10 +17,9 @@ #include "milvus.grpc.pb.h" #include "GrpcMilvusServer.h" -#include "../ServerConfig.h" -#include "../DBWrapper.h" +#include "server/ServerConfig.h" +#include "server/DBWrapper.h" #include "utils/Log.h" -#include "faiss/utils.h" #include "GrpcRequestHandler.h" #include @@ -47,6 +46,7 @@ static std::unique_ptr<::grpc::Server> server; constexpr long MESSAGE_SIZE = -1; +//this class is to check port occupation during server start class NoReusePortOption : public ::grpc::ServerBuilderOption { public: void UpdateArguments(::grpc::ChannelArguments *args) override { @@ -71,8 +71,6 @@ GrpcMilvusServer::StartService() { std::string address = server_config.GetValue(CONFIG_SERVER_ADDRESS, "127.0.0.1"); int32_t port = server_config.GetInt32Value(CONFIG_SERVER_PORT, 19530); - faiss::distance_compute_blas_threshold = engine_config.GetInt32Value(CONFIG_DCBT, 20); - std::string server_address(address + ":" + std::to_string(port)); ::grpc::ServerBuilder builder; diff --git a/cpp/src/utils/BlockingQueue.inl b/cpp/src/utils/BlockingQueue.inl index 8df38dd547..86237f3322 100644 --- a/cpp/src/utils/BlockingQueue.inl +++ b/cpp/src/utils/BlockingQueue.inl @@ -18,8 +18,6 @@ #pragma once -//#include "Log.h" -#include "Error.h" namespace zilliz { namespace milvus { @@ -31,14 +29,6 @@ BlockingQueue::Put(const T &task) { std::unique_lock lock(mtx); full_.wait(lock, [this] { return (queue_.size() < capacity_); }); - if (queue_.size() >= capacity_) { - std::string error_msg = - "blocking queue is full, capacity: " + std::to_string(capacity_) + " queue_size: " + - std::to_string(queue_.size()); - //SERVER_LOG_ERROR << error_msg; - throw ServerException(SERVER_BLOCKING_QUEUE_EMPTY, error_msg); - } - queue_.push(task); empty_.notify_all(); } @@ -49,12 +39,6 @@ BlockingQueue::Take() { std::unique_lock lock(mtx); empty_.wait(lock, [this] { return !queue_.empty(); }); - if (queue_.empty()) { - std::string error_msg = "blocking queue empty"; - //SERVER_LOG_ERROR << error_msg; - throw ServerException(SERVER_BLOCKING_QUEUE_EMPTY, error_msg); - } - T front(queue_.front()); queue_.pop(); full_.notify_all(); @@ -73,11 +57,7 @@ T BlockingQueue::Front() { std::unique_lock lock(mtx); empty_.wait(lock, [this] { return !queue_.empty(); }); - if (queue_.empty()) { - std::string error_msg = "blocking queue empty"; - //SERVER_LOG_ERROR << error_msg; - throw ServerException(SERVER_BLOCKING_QUEUE_EMPTY, error_msg); - } + T front(queue_.front()); return front; } @@ -88,12 +68,6 @@ BlockingQueue::Back() { std::unique_lock lock(mtx); empty_.wait(lock, [this] { return !queue_.empty(); }); - if (queue_.empty()) { - std::string error_msg = "blocking queue empty"; - //SERVER_LOG_ERROR << error_msg; - throw ServerException(SERVER_BLOCKING_QUEUE_EMPTY, error_msg); - } - T back(queue_.back()); return back; } diff --git a/cpp/src/utils/CommonUtil.cpp b/cpp/src/utils/CommonUtil.cpp index 19a63006cd..ff985dcd7d 100644 --- a/cpp/src/utils/CommonUtil.cpp +++ b/cpp/src/utils/CommonUtil.cpp @@ -162,6 +162,11 @@ uint64_t CommonUtil::GetFileSize(const std::string &path) { } } +std::string CommonUtil::GetFileName(std::string filename) { + int pos = filename.find_last_of('/'); + return filename.substr(pos + 1); +} + std::string CommonUtil::GetExePath() { const size_t buf_len = 1024; char buf[buf_len]; diff --git a/cpp/src/utils/CommonUtil.h b/cpp/src/utils/CommonUtil.h index aee57f0446..7e1349bf9e 100755 --- a/cpp/src/utils/CommonUtil.h +++ b/cpp/src/utils/CommonUtil.h @@ -38,6 +38,7 @@ class CommonUtil { static Status CreateDirectory(const std::string &path); static Status DeleteDirectory(const std::string &path); + static std::string GetFileName(std::string filename); static std::string GetExePath(); static bool TimeStrToTime(const std::string& time_str, diff --git a/cpp/src/utils/LogUtil.cpp b/cpp/src/utils/LogUtil.cpp index 6224c854a5..e4af1104db 100644 --- a/cpp/src/utils/LogUtil.cpp +++ b/cpp/src/utils/LogUtil.cpp @@ -84,49 +84,15 @@ void RolloutHandler(const char *filename, std::size_t size, el::Level level) { } } -int32_t InitLog(const std::string &log_config_file) { -#if 0 - ServerConfig &config = ServerConfig::GetInstance(); - ConfigNode log_config = config.GetConfig(CONFIG_LOG); - const std::map& settings = log_config.GetChildren(); - - std::string str_config; - for(auto iter : settings) { - str_config += "* "; - str_config += iter.first; - str_config += ":"; - str_config.append("\n"); - - auto sub_configs = iter.second.GetConfig(); - for(auto it_sub : sub_configs) { - str_config += " "; - str_config += it_sub.first; - str_config += " = "; - std::string temp = it_sub.first; - std::transform(temp.begin(), temp.end(), temp.begin(), ::tolower); - bool is_text = (temp == "format" || temp == "filename"); - if(is_text){ - str_config += "\""; - } - str_config += it_sub.second; - if(is_text){ - str_config += "\""; - } - str_config.append("\n"); - } - } - - el::Configurations conf; - conf.parseFromText(str_config); -#else +Status InitLog(const std::string &log_config_file) { el::Configurations conf(log_config_file); -#endif el::Loggers::reconfigureAllLoggers(conf); el::Loggers::addFlag(el::LoggingFlag::StrictLogFileSizeCheck); el::Helpers::installPreRollOutCallback(RolloutHandler); el::Loggers::addFlag(el::LoggingFlag::DisableApplicationAbortOnFatalLog); - return 0; + + return Status::OK(); } diff --git a/cpp/src/utils/LogUtil.h b/cpp/src/utils/LogUtil.h index ebeb117aa2..d86b301d9f 100644 --- a/cpp/src/utils/LogUtil.h +++ b/cpp/src/utils/LogUtil.h @@ -17,6 +17,8 @@ #pragma once +#include "utils/Status.h" + #include #include #include "easylogging++.h" @@ -24,11 +26,9 @@ namespace zilliz { namespace milvus { namespace server { -int32_t InitLog(const std::string& log_config_file); -inline std::string GetFileName(std::string filename) { - int pos = filename.find_last_of('/'); - return filename.substr(pos + 1); -} + +Status InitLog(const std::string& log_config_file); + void RolloutHandler(const char *filename, std::size_t size, el::Level level); #define SHOW_LOCATION diff --git a/cpp/src/utils/SignalUtil.cpp b/cpp/src/utils/SignalUtil.cpp index 07c1001044..e380dc6c85 100644 --- a/cpp/src/utils/SignalUtil.cpp +++ b/cpp/src/utils/SignalUtil.cpp @@ -31,23 +31,19 @@ void SignalUtil::HandleSignal(int signum){ switch(signum){ case SIGINT: case SIGUSR2:{ - server::Server* server_ptr = server::Server::Instance(); - server_ptr->Stop(); + SERVER_LOG_INFO << "Server received signal:" << std::to_string(signum); + + server::Server& server_ptr = server::Server::Instance(); + server_ptr.Stop(); exit(0); } default:{ - SERVER_LOG_INFO << "Server received signal:" << std::to_string(signum); + SERVER_LOG_INFO << "Server received critical signal:" << std::to_string(signum); SignalUtil::PrintStacktrace(); - std::string info = "Server encounter critical signal:"; - info += std::to_string(signum); -// SendSignalMessage(signum, info); - - SERVER_LOG_INFO << info; - - server::Server* server_ptr = server::Server::Instance(); - server_ptr->Stop(); + server::Server& server_ptr = server::Server::Instance(); + server_ptr.Stop(); exit(1); } diff --git a/cpp/src/utils/ValidationUtil.h b/cpp/src/utils/ValidationUtil.h index 0c109e0e25..ad8ba5fee9 100644 --- a/cpp/src/utils/ValidationUtil.h +++ b/cpp/src/utils/ValidationUtil.h @@ -26,6 +26,9 @@ namespace milvus { namespace server { class ValidationUtil { +private: + ValidationUtil() = default; + public: static Status ValidateTableName(const std::string &table_name); diff --git a/cpp/unittest/db/mysql_meta_test.cpp b/cpp/unittest/db/mysql_meta_test.cpp index ab94c817fe..e99565886d 100644 --- a/cpp/unittest/db/mysql_meta_test.cpp +++ b/cpp/unittest/db/mysql_meta_test.cpp @@ -130,7 +130,7 @@ TEST_F(MySqlMetaTest, ARCHIVE_TEST_DAYS) { std::stringstream ss; ss << "days:" << days_num; options.archive_conf = ArchiveConf("delete", ss.str()); - int mode = Options::MODE::SINGLE; + int mode = DBOptions::MODE::SINGLE; meta::MySQLMetaImpl impl(options, mode); auto table_id = "meta_test_table"; @@ -190,7 +190,7 @@ TEST_F(MySqlMetaTest, ARCHIVE_TEST_DISK) { DBMetaOptions options = GetOptions().meta; options.archive_conf = ArchiveConf("delete", "disk:11"); - int mode = Options::MODE::SINGLE; + int mode = DBOptions::MODE::SINGLE; auto impl = meta::MySQLMetaImpl(options, mode); auto table_id = "meta_test_group"; diff --git a/cpp/unittest/db/utils.cpp b/cpp/unittest/db/utils.cpp index 4bc9699e05..aededfd0bd 100644 --- a/cpp/unittest/db/utils.cpp +++ b/cpp/unittest/db/utils.cpp @@ -66,7 +66,7 @@ void BaseTest::TearDown() { zilliz::knowhere::FaissGpuResourceMgr::GetInstance().Free(); } -engine::Options BaseTest::GetOptions() { +engine::DBOptions BaseTest::GetOptions() { auto options = engine::DBFactory::BuildOption(); options.meta.path = "/tmp/milvus_test"; options.meta.backend_uri = "sqlite://:@:/"; @@ -108,7 +108,7 @@ void DBTest::TearDown() { } ///////////////////////////////////////////////////////////////////////////////////////////////////////////////// -engine::Options DBTest2::GetOptions() { +engine::DBOptions DBTest2::GetOptions() { auto options = engine::DBFactory::BuildOption(); options.meta.path = "/tmp/milvus_test"; options.meta.archive_conf = engine::ArchiveConf("delete", "disk:1"); @@ -134,7 +134,7 @@ void MetaTest::TearDown() { } ///////////////////////////////////////////////////////////////////////////////////////////////////////////////// -engine::Options MySqlDBTest::GetOptions() { +engine::DBOptions MySqlDBTest::GetOptions() { auto options = engine::DBFactory::BuildOption(); options.meta.path = "/tmp/milvus_test"; options.meta.backend_uri = DBTestEnvironment::getURI(); @@ -163,7 +163,7 @@ void MySqlMetaTest::TearDown() { boost::filesystem::remove_all(options.meta.path); } -zilliz::milvus::engine::Options MySqlMetaTest::GetOptions() { +engine::DBOptions MySqlMetaTest::GetOptions() { auto options = engine::DBFactory::BuildOption(); options.meta.path = "/tmp/milvus_test"; options.meta.backend_uri = DBTestEnvironment::getURI(); diff --git a/cpp/unittest/db/utils.h b/cpp/unittest/db/utils.h index c1dc730089..7a1320ef6b 100644 --- a/cpp/unittest/db/utils.h +++ b/cpp/unittest/db/utils.h @@ -50,7 +50,7 @@ protected: virtual void SetUp() override; virtual void TearDown() override; - virtual zilliz::milvus::engine::Options GetOptions(); + virtual zilliz::milvus::engine::DBOptions GetOptions(); }; ///////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -65,7 +65,7 @@ class DBTest : public BaseTest { ///////////////////////////////////////////////////////////////////////////////////////////////////////////////// class DBTest2 : public DBTest { protected: - virtual zilliz::milvus::engine::Options GetOptions() override; + virtual zilliz::milvus::engine::DBOptions GetOptions() override; }; ///////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -84,7 +84,7 @@ class MetaTest : public BaseTest { ///////////////////////////////////////////////////////////////////////////////////////////////////////////////// class MySqlDBTest : public DBTest { protected: - zilliz::milvus::engine::Options GetOptions(); + zilliz::milvus::engine::DBOptions GetOptions(); }; ///////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -94,7 +94,7 @@ class MySqlMetaTest : public BaseTest { virtual void SetUp() override; virtual void TearDown() override; - zilliz::milvus::engine::Options GetOptions(); + zilliz::milvus::engine::DBOptions GetOptions(); }; diff --git a/cpp/unittest/metrics/utils.cpp b/cpp/unittest/metrics/utils.cpp index 4dc370c952..49ccc53d30 100644 --- a/cpp/unittest/metrics/utils.cpp +++ b/cpp/unittest/metrics/utils.cpp @@ -52,7 +52,7 @@ void MetricTest::InitLog() { el::Loggers::reconfigureLogger("default", defaultConf); } -engine::Options MetricTest::GetOptions() { +engine::DBOptions MetricTest::GetOptions() { auto options = engine::DBFactory::BuildOption(); options.meta.path = "/tmp/milvus_test"; options.meta.backend_uri = "sqlite://:@:/"; diff --git a/cpp/unittest/metrics/utils.h b/cpp/unittest/metrics/utils.h index 7f093dd091..37c21c82f9 100644 --- a/cpp/unittest/metrics/utils.h +++ b/cpp/unittest/metrics/utils.h @@ -72,5 +72,5 @@ protected: void InitLog(); virtual void SetUp() override; virtual void TearDown() override; - virtual zilliz::milvus::engine::Options GetOptions(); + virtual zilliz::milvus::engine::DBOptions GetOptions(); }; \ No newline at end of file diff --git a/cpp/unittest/server/rpc_test.cpp b/cpp/unittest/server/rpc_test.cpp index b87ead099a..f89c98eb9d 100644 --- a/cpp/unittest/server/rpc_test.cpp +++ b/cpp/unittest/server/rpc_test.cpp @@ -34,11 +34,9 @@ #include "scheduler/ResourceFactory.h" #include "utils/CommonUtil.h" +using namespace zilliz::milvus; -namespace zilliz { -namespace milvus { -namespace server { -namespace grpc { +namespace { static const char *TABLE_NAME = "test_grpc"; static constexpr int64_t TABLE_DIM = 256; @@ -65,35 +63,35 @@ class RpcHandlerTest : public testing::Test { res_mgr->Start(); engine::SchedInst::GetInstance()->Start(); - zilliz::milvus::engine::Options opt; + engine::DBOptions opt; - ConfigNode &db_config = ServerConfig::GetInstance().GetConfig(CONFIG_DB); - db_config.SetValue(CONFIG_DB_URL, "sqlite://:@:/"); - db_config.SetValue(CONFIG_DB_PATH, "/tmp/milvus_test"); - db_config.SetValue(CONFIG_DB_SLAVE_PATH, ""); - db_config.SetValue(CONFIG_DB_ARCHIVE_DISK, ""); - db_config.SetValue(CONFIG_DB_ARCHIVE_DAYS, ""); + server::ConfigNode &db_config = server::ServerConfig::GetInstance().GetConfig(server::CONFIG_DB); + db_config.SetValue(server::CONFIG_DB_URL, "sqlite://:@:/"); + db_config.SetValue(server::CONFIG_DB_PATH, "/tmp/milvus_test"); + db_config.SetValue(server::CONFIG_DB_SLAVE_PATH, ""); + db_config.SetValue(server::CONFIG_DB_ARCHIVE_DISK, ""); + db_config.SetValue(server::CONFIG_DB_ARCHIVE_DAYS, ""); - ConfigNode &cache_config = ServerConfig::GetInstance().GetConfig(CONFIG_CACHE); - cache_config.SetValue(CONFIG_INSERT_CACHE_IMMEDIATELY, ""); + server::ConfigNode &cache_config = server::ServerConfig::GetInstance().GetConfig(server::CONFIG_CACHE); + cache_config.SetValue(server::CONFIG_INSERT_CACHE_IMMEDIATELY, ""); - ConfigNode &engine_config = ServerConfig::GetInstance().GetConfig(CONFIG_ENGINE); - engine_config.SetValue(CONFIG_OMP_THREAD_NUM, ""); + server::ConfigNode &engine_config = server::ServerConfig::GetInstance().GetConfig(server::CONFIG_ENGINE); + engine_config.SetValue(server::CONFIG_OMP_THREAD_NUM, ""); - ConfigNode &serverConfig = ServerConfig::GetInstance().GetConfig(CONFIG_SERVER); -// serverConfig.SetValue(CONFIG_CLUSTER_MODE, "cluster"); + server::ConfigNode &serverConfig = server::ServerConfig::GetInstance().GetConfig(server::CONFIG_SERVER); +// serverConfig.SetValue(server::CONFIG_CLUSTER_MODE, "cluster"); // DBWrapper::GetInstance().GetInstance().StartService(); // DBWrapper::GetInstance().GetInstance().StopService(); // -// serverConfig.SetValue(CONFIG_CLUSTER_MODE, "read_only"); +// serverConfig.SetValue(server::CONFIG_CLUSTER_MODE, "read_only"); // DBWrapper::GetInstance().GetInstance().StartService(); // DBWrapper::GetInstance().GetInstance().StopService(); - serverConfig.SetValue(CONFIG_CLUSTER_MODE, "single"); - DBWrapper::GetInstance().GetInstance().StartService(); + serverConfig.SetValue(server::CONFIG_CLUSTER_MODE, "single"); + server::DBWrapper::GetInstance().StartService(); //initialize handler, create table - handler = std::make_shared(); + handler = std::make_shared(); ::grpc::ServerContext context; ::milvus::grpc::TableSchema request; ::milvus::grpc::Status status; @@ -106,16 +104,15 @@ class RpcHandlerTest : public testing::Test { void TearDown() override { - DBWrapper::GetInstance().StopService(); + server::DBWrapper::GetInstance().StopService(); engine::ResMgrInst::GetInstance()->Stop(); engine::SchedInst::GetInstance()->Stop(); boost::filesystem::remove_all("/tmp/milvus_test"); } protected: - std::shared_ptr handler; + std::shared_ptr handler; }; -namespace { void BuildVectors(int64_t from, int64_t to, std::vector> &vector_record_array) { if (to <= from) { @@ -146,6 +143,7 @@ std::string CurrentTmDate(int64_t offset_day = 0) { return str; } + } TEST_F(RpcHandlerTest, HasTableTest) { @@ -429,26 +427,27 @@ TEST_F(RpcHandlerTest, DeleteByRangeTest) { } ////////////////////////////////////////////////////////////////////// -class DummyTask : public GrpcBaseTask { - public: +namespace { +class DummyTask : public server::grpc::GrpcBaseTask { +public: Status OnExecute() override { return Status::OK(); } - static BaseTaskPtr - Create(std::string& dummy) { - return std::shared_ptr(new DummyTask(dummy)); + static server::grpc::BaseTaskPtr + Create(std::string &dummy) { + return std::shared_ptr(new DummyTask(dummy)); } - public: +public: explicit DummyTask(std::string &dummy) : GrpcBaseTask(dummy) { } }; class RpcSchedulerTest : public testing::Test { - protected: +protected: void SetUp() override { std::string dummy = "dql"; @@ -458,25 +457,22 @@ class RpcSchedulerTest : public testing::Test { std::shared_ptr task_ptr; }; +} + TEST_F(RpcSchedulerTest, BaseTaskTest){ auto status = task_ptr->Execute(); ASSERT_TRUE(status.ok()); - GrpcRequestScheduler::GetInstance().Start(); + server::grpc::GrpcRequestScheduler::GetInstance().Start(); ::milvus::grpc::Status grpc_status; std::string dummy = "dql"; - BaseTaskPtr base_task_ptr = DummyTask::Create(dummy); - GrpcRequestScheduler::GetInstance().ExecTask(base_task_ptr, &grpc_status); + server::grpc::BaseTaskPtr base_task_ptr = DummyTask::Create(dummy); + server::grpc::GrpcRequestScheduler::GetInstance().ExecTask(base_task_ptr, &grpc_status); - GrpcRequestScheduler::GetInstance().ExecuteTask(task_ptr); + server::grpc::GrpcRequestScheduler::GetInstance().ExecuteTask(task_ptr); task_ptr = nullptr; - GrpcRequestScheduler::GetInstance().ExecuteTask(task_ptr); + server::grpc::GrpcRequestScheduler::GetInstance().ExecuteTask(task_ptr); - GrpcRequestScheduler::GetInstance().Stop(); -} - -} -} -} + server::grpc::GrpcRequestScheduler::GetInstance().Stop(); } diff --git a/cpp/unittest/server/util_test.cpp b/cpp/unittest/server/util_test.cpp index 00be2f3f13..b582c9a17a 100644 --- a/cpp/unittest/server/util_test.cpp +++ b/cpp/unittest/server/util_test.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include "utils/CommonUtil.h" #include "utils/Error.h" @@ -51,6 +52,10 @@ TEST(UtilTest, EXCEPTION_TEST) { ASSERT_EQ(msg, err_msg); } +TEST(UtilTest, SIGNAL_TEST) { + server::SignalUtil::PrintStacktrace(); +} + TEST(UtilTest, COMMON_TEST) { unsigned long total_mem = 0, free_mem = 0; server::CommonUtil::GetSystemMemInfo(total_mem, free_mem); @@ -167,13 +172,13 @@ TEST(UtilTest, BLOCKINGQUEUE_TEST) { } TEST(UtilTest, LOG_TEST) { - int32_t res = server::InitLog(LOG_FILE_PATH); - ASSERT_EQ(res, 0); + auto status = server::InitLog(LOG_FILE_PATH); + ASSERT_TRUE(status.ok()); EXPECT_FALSE(el::Loggers::hasFlag(el::LoggingFlag::NewLineForContainer)); EXPECT_FALSE(el::Loggers::hasFlag(el::LoggingFlag::LogDetailedCrashReason)); - std::string fname = server::GetFileName(LOG_FILE_PATH); + std::string fname = server::CommonUtil::GetFileName(LOG_FILE_PATH); ASSERT_EQ(fname, "log_config.conf"); }