From 92a21ec3dab5b8c8ee50548808fc0bcc6103e6a5 Mon Sep 17 00:00:00 2001 From: starlord Date: Fri, 18 Oct 2019 14:40:15 +0800 Subject: [PATCH 1/2] #30 Some troubleshoot messages in Milvus do not provide enough information Former-commit-id: 49b03511fc7abbc323c511fad28a568d71b518d5 --- CHANGELOG.md | 1 + core/src/server/grpc_impl/GrpcRequestTask.cpp | 40 ++++++++++--------- core/src/utils/ValidationUtil.cpp | 38 ++++++++++-------- 3 files changed, 44 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f2e22d0112..257011cc03 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ Please mark all change in change log and use the ticket from JIRA. - \#23 - Add unittest to improve code coverage - \#31 - make clang-format failed after run build.sh -l - \#39 - Create SQ8H index hang if using github server version +- \#30 - Some troubleshoot messages in Milvus do not provide enough information ## Improvement - MS-552 - Add and change the easylogging library diff --git a/core/src/server/grpc_impl/GrpcRequestTask.cpp b/core/src/server/grpc_impl/GrpcRequestTask.cpp index 02cb24175a..9e14ecece8 100644 --- a/core/src/server/grpc_impl/GrpcRequestTask.cpp +++ b/core/src/server/grpc_impl/GrpcRequestTask.cpp @@ -113,6 +113,12 @@ ConvertTimeRangeToDBDates(const std::vector<::milvus::grpc::Range>& range_array, return Status::OK(); } + +std::string TableNotExistMsg(const std::string& table_name) { + return "Table " + table_name + + " not exist. Use milvus.has_table to verify whether the table exists. You also can check if the table name exists."; +} + } // namespace //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -255,7 +261,7 @@ CreateIndexTask::OnExecute() { } if (!has_table) { - return Status(SERVER_TABLE_NOT_EXIST, "Table " + table_name_ + " not exists"); + return Status(SERVER_TABLE_NOT_EXIST, TableNotExistMsg(table_name_)); } auto& grpc_index = index_param_->index(); @@ -348,7 +354,7 @@ DropTableTask::OnExecute() { 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"); + return Status(SERVER_TABLE_NOT_EXIST, TableNotExistMsg(table_name_)); } else { return status; } @@ -420,12 +426,12 @@ InsertTask::OnExecute() { return status; } if (insert_param_->row_record_array().empty()) { - return Status(SERVER_INVALID_ROWRECORD_ARRAY, "Row record array is empty"); + return Status(SERVER_INVALID_ROWRECORD_ARRAY, "The vector array is empty. Make sure you have entered vector records."); } if (!insert_param_->row_id_array().empty()) { if (insert_param_->row_id_array().size() != insert_param_->row_record_array_size()) { - return Status(SERVER_ILLEGAL_VECTOR_ID, "Size of vector ids is not equal to row record array size"); + return Status(SERVER_ILLEGAL_VECTOR_ID, "The size of vector ID array must be equal to the size of the vector."); } } @@ -435,7 +441,7 @@ InsertTask::OnExecute() { 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"); + return Status(SERVER_TABLE_NOT_EXIST, TableNotExistMsg(insert_param_->table_name())); } else { return status; } @@ -447,13 +453,13 @@ InsertTask::OnExecute() { // user already provided id before, all insert action require user id if ((table_info.flag_ & engine::meta::FLAG_MASK_HAS_USERID) != 0 && !user_provide_ids) { return Status(SERVER_ILLEGAL_VECTOR_ID, - "Table vector ids are user defined, please provide id for this batch"); + "Table vector IDs are user-defined. Please provide IDs for all vectors of this table."); } // user didn't provided id before, no need to provide user id if ((table_info.flag_ & engine::meta::FLAG_MASK_NO_USERID) != 0 && user_provide_ids) { return Status(SERVER_ILLEGAL_VECTOR_ID, - "Table vector ids are auto generated, no need to provide id for this batch"); + "Table vector IDs are auto-generated. All vectors of this table must use auto-generated IDs."); } rc.RecordSection("check validation"); @@ -470,13 +476,12 @@ InsertTask::OnExecute() { // TODO(yk): change to one dimension array 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 Status(SERVER_INVALID_ROWRECORD_ARRAY, "Row record array data is empty"); + return Status(SERVER_INVALID_ROWRECORD_ARRAY, "The vector dimension must be equal to the table dimension."); } uint64_t vec_dim = insert_param_->row_record_array(i).vector_data().size(); if (vec_dim != table_info.dimension_) { ErrorCode error_code = SERVER_INVALID_VECTOR_DIMENSION; - std::string error_msg = "Invalid row record dimension: " + std::to_string(vec_dim) + - " vs. table dimension:" + std::to_string(table_info.dimension_); + std::string error_msg = "The vector dimension must be equal to the table dimension."; return Status(error_code, error_msg); } memcpy(&vec_f[i * table_info.dimension_], insert_param_->row_record_array(i).vector_data().data(), @@ -569,7 +574,7 @@ SearchTask::OnExecute() { 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"); + return Status(SERVER_TABLE_NOT_EXIST, TableNotExistMsg(table_name_)); } else { return status; } @@ -587,7 +592,7 @@ SearchTask::OnExecute() { } if (search_param_->query_record_array().empty()) { - return Status(SERVER_INVALID_ROWRECORD_ARRAY, "Row record array is empty"); + return Status(SERVER_INVALID_ROWRECORD_ARRAY, "The vector array is empty. Make sure you have entered vector records."); } // step 4: check date range, and convert to db dates @@ -609,13 +614,12 @@ 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 Status(SERVER_INVALID_ROWRECORD_ARRAY, "Row record array data is empty"); + return Status(SERVER_INVALID_ROWRECORD_ARRAY, "The vector dimension must be equal to the table dimension."); } 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_); + std::string error_msg = "The vector dimension must be equal to the table dimension."; return Status(error_code, error_msg); } @@ -707,7 +711,7 @@ CountTableTask::OnExecute() { status = DBWrapper::DB()->GetTableRowCount(table_name_, row_count); if (!status.ok()) { if (status.code(), DB_NOT_FOUND) { - return Status(SERVER_TABLE_NOT_EXIST, "Table " + table_name_ + " not exists"); + return Status(SERVER_TABLE_NOT_EXIST, TableNotExistMsg(table_name_)); } else { return status; } @@ -779,7 +783,7 @@ DeleteByRangeTask::OnExecute() { 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"); + return Status(SERVER_TABLE_NOT_EXIST, TableNotExistMsg(table_name)); } else { return status; } @@ -917,7 +921,7 @@ DropIndexTask::OnExecute() { } if (!has_table) { - return Status(SERVER_TABLE_NOT_EXIST, "Table " + table_name_ + " not exists"); + return Status(SERVER_TABLE_NOT_EXIST, TableNotExistMsg(table_name_)); } // step 2: check table existence diff --git a/core/src/utils/ValidationUtil.cpp b/core/src/utils/ValidationUtil.cpp index e275f533a3..11ff5da68e 100644 --- a/core/src/utils/ValidationUtil.cpp +++ b/core/src/utils/ValidationUtil.cpp @@ -37,14 +37,15 @@ Status ValidationUtil::ValidateTableName(const std::string& table_name) { // Table name shouldn't be empty. if (table_name.empty()) { - std::string msg = "Empty table name"; + std::string msg = "Table name should not be empty."; SERVER_LOG_ERROR << msg; return Status(SERVER_INVALID_TABLE_NAME, msg); } + std::string invalid_msg = "Invalid table name: " + table_name + ". "; // Table name size shouldn't exceed 16384. if (table_name.size() > TABLE_NAME_SIZE_LIMIT) { - std::string msg = "Table name size exceed the limitation"; + std::string msg = invalid_msg + "The length of a table name must be less than 255 characters."; SERVER_LOG_ERROR << msg; return Status(SERVER_INVALID_TABLE_NAME, msg); } @@ -52,7 +53,7 @@ ValidationUtil::ValidateTableName(const std::string& table_name) { // Table name first character should be underscore or character. char first_char = table_name[0]; if (first_char != '_' && std::isalpha(first_char) == 0) { - std::string msg = "Table name first character isn't underscore or character"; + std::string msg = invalid_msg + "The first character of a table name must be an underscore or letter."; SERVER_LOG_ERROR << msg; return Status(SERVER_INVALID_TABLE_NAME, msg); } @@ -61,7 +62,7 @@ ValidationUtil::ValidateTableName(const std::string& table_name) { for (int64_t i = 1; i < table_name_size; ++i) { char name_char = table_name[i]; if (name_char != '_' && std::isalnum(name_char) == 0) { - std::string msg = "Table name character isn't underscore or alphanumber"; + std::string msg = invalid_msg + "Table name can only contain numbers, letters, and underscores."; SERVER_LOG_ERROR << msg; return Status(SERVER_INVALID_TABLE_NAME, msg); } @@ -72,12 +73,9 @@ ValidationUtil::ValidateTableName(const std::string& table_name) { Status ValidationUtil::ValidateTableDimension(int64_t dimension) { - if (dimension <= 0) { - std::string msg = "Dimension value should be greater than 0"; - SERVER_LOG_ERROR << msg; - return Status(SERVER_INVALID_VECTOR_DIMENSION, msg); - } else if (dimension > TABLE_DIMENSION_LIMIT) { - std::string msg = "Table dimension excceed the limitation: " + std::to_string(TABLE_DIMENSION_LIMIT); + if (dimension <= 0 || dimension > TABLE_DIMENSION_LIMIT) { + std::string msg = "Invalid table dimension: " + std::to_string(dimension) + ". " + + "The table dimension must be within the range of 1 ~ 16384."; SERVER_LOG_ERROR << msg; return Status(SERVER_INVALID_VECTOR_DIMENSION, msg); } else { @@ -89,7 +87,8 @@ Status ValidationUtil::ValidateTableIndexType(int32_t index_type) { int engine_type = static_cast(engine::EngineType(index_type)); if (engine_type <= 0 || engine_type > static_cast(engine::EngineType::MAX_VALUE)) { - std::string msg = "Invalid index type: " + std::to_string(index_type); + std::string msg = "Invalid index type: " + std::to_string(index_type) + ". " + + "Make sure the index type is in IndexType list."; SERVER_LOG_ERROR << msg; return Status(SERVER_INVALID_INDEX_TYPE, msg); } @@ -109,7 +108,8 @@ ValidationUtil::ValidateTableIndexType(int32_t index_type) { Status ValidationUtil::ValidateTableIndexNlist(int32_t nlist) { if (nlist <= 0) { - std::string msg = "nlist value should be greater than 0"; + std::string msg = "Invalid index nlist: " + std::to_string(nlist) + ". " + + "The index nlist must be greater than 0."; SERVER_LOG_ERROR << msg; return Status(SERVER_INVALID_INDEX_NLIST, msg); } @@ -120,7 +120,9 @@ ValidationUtil::ValidateTableIndexNlist(int32_t nlist) { Status ValidationUtil::ValidateTableIndexFileSize(int64_t index_file_size) { if (index_file_size <= 0 || index_file_size > INDEX_FILE_SIZE_LIMIT) { - std::string msg = "Invalid index file size: " + std::to_string(index_file_size); + std::string msg = "Invalid index file size: " + std::to_string(index_file_size) + ". " + + "The index file size must be within the range of 1 ~ " + + std::to_string(INDEX_FILE_SIZE_LIMIT) + "."; SERVER_LOG_ERROR << msg; return Status(SERVER_INVALID_INDEX_FILE_SIZE, msg); } @@ -132,7 +134,8 @@ Status ValidationUtil::ValidateTableIndexMetricType(int32_t metric_type) { if (metric_type != static_cast(engine::MetricType::L2) && metric_type != static_cast(engine::MetricType::IP)) { - std::string msg = "Invalid metric type: " + std::to_string(metric_type); + std::string msg = "Invalid index metric type: " + std::to_string(metric_type) + ". " + + "Make sure the metric type is either MetricType.L2 or MetricType.IP."; SERVER_LOG_ERROR << msg; return Status(SERVER_INVALID_INDEX_METRIC_TYPE, msg); } @@ -142,7 +145,8 @@ ValidationUtil::ValidateTableIndexMetricType(int32_t metric_type) { Status ValidationUtil::ValidateSearchTopk(int64_t top_k, const engine::meta::TableSchema& table_schema) { if (top_k <= 0 || top_k > 2048) { - std::string msg = "Invalid top k value: " + std::to_string(top_k) + ", rational range [1, 2048]"; + std::string msg = "Invalid topk: " + std::to_string(top_k) + ". " + + "The topk must be within the range of 1 ~ 2048."; SERVER_LOG_ERROR << msg; return Status(SERVER_INVALID_TOPK, msg); } @@ -153,8 +157,8 @@ ValidationUtil::ValidateSearchTopk(int64_t top_k, const engine::meta::TableSchem Status ValidationUtil::ValidateSearchNprobe(int64_t nprobe, const engine::meta::TableSchema& table_schema) { if (nprobe <= 0 || nprobe > table_schema.nlist_) { - std::string msg = "Invalid nprobe value: " + std::to_string(nprobe) + ", rational range [1, " + - std::to_string(table_schema.nlist_) + "]"; + std::string msg = "Invalid nprobe: " + std::to_string(nprobe) + ". " + + "The nprobe must be within the range of 1 ~ index nlist."; SERVER_LOG_ERROR << msg; return Status(SERVER_INVALID_NPROBE, msg); } From bcc4bbde0365c1cfbee828ea7dedbaf660aa86c5 Mon Sep 17 00:00:00 2001 From: starlord Date: Fri, 18 Oct 2019 14:41:32 +0800 Subject: [PATCH 2/2] format code Former-commit-id: 993b88487da976f809fdb29ffb07cdb0188a16f7 --- core/src/server/grpc_impl/GrpcRequestTask.cpp | 28 ++++++++++------- core/src/utils/ValidationUtil.cpp | 30 +++++++++---------- 2 files changed, 33 insertions(+), 25 deletions(-) diff --git a/core/src/server/grpc_impl/GrpcRequestTask.cpp b/core/src/server/grpc_impl/GrpcRequestTask.cpp index 9e14ecece8..86ff23b3d0 100644 --- a/core/src/server/grpc_impl/GrpcRequestTask.cpp +++ b/core/src/server/grpc_impl/GrpcRequestTask.cpp @@ -114,9 +114,11 @@ ConvertTimeRangeToDBDates(const std::vector<::milvus::grpc::Range>& range_array, return Status::OK(); } -std::string TableNotExistMsg(const std::string& table_name) { - return "Table " + table_name - + " not exist. Use milvus.has_table to verify whether the table exists. You also can check if the table name exists."; +std::string +TableNotExistMsg(const std::string& table_name) { + return "Table " + table_name + + " not exist. Use milvus.has_table to verify whether the table exists. You also can check if the table name " + "exists."; } } // namespace @@ -426,12 +428,14 @@ InsertTask::OnExecute() { return status; } if (insert_param_->row_record_array().empty()) { - return Status(SERVER_INVALID_ROWRECORD_ARRAY, "The vector array is empty. Make sure you have entered vector records."); + return Status(SERVER_INVALID_ROWRECORD_ARRAY, + "The vector array is empty. Make sure you have entered vector records."); } if (!insert_param_->row_id_array().empty()) { if (insert_param_->row_id_array().size() != insert_param_->row_record_array_size()) { - return Status(SERVER_ILLEGAL_VECTOR_ID, "The size of vector ID array must be equal to the size of the vector."); + return Status(SERVER_ILLEGAL_VECTOR_ID, + "The size of vector ID array must be equal to the size of the vector."); } } @@ -458,8 +462,9 @@ InsertTask::OnExecute() { // user didn't provided id before, no need to provide user id if ((table_info.flag_ & engine::meta::FLAG_MASK_NO_USERID) != 0 && user_provide_ids) { - return Status(SERVER_ILLEGAL_VECTOR_ID, - "Table vector IDs are auto-generated. All vectors of this table must use auto-generated IDs."); + return Status( + SERVER_ILLEGAL_VECTOR_ID, + "Table vector IDs are auto-generated. All vectors of this table must use auto-generated IDs."); } rc.RecordSection("check validation"); @@ -476,7 +481,8 @@ InsertTask::OnExecute() { // TODO(yk): change to one dimension array 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 Status(SERVER_INVALID_ROWRECORD_ARRAY, "The vector dimension must be equal to the table dimension."); + return Status(SERVER_INVALID_ROWRECORD_ARRAY, + "The vector dimension must be equal to the table dimension."); } uint64_t vec_dim = insert_param_->row_record_array(i).vector_data().size(); if (vec_dim != table_info.dimension_) { @@ -592,7 +598,8 @@ SearchTask::OnExecute() { } if (search_param_->query_record_array().empty()) { - return Status(SERVER_INVALID_ROWRECORD_ARRAY, "The vector array is empty. Make sure you have entered vector records."); + return Status(SERVER_INVALID_ROWRECORD_ARRAY, + "The vector array is empty. Make sure you have entered vector records."); } // step 4: check date range, and convert to db dates @@ -614,7 +621,8 @@ 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 Status(SERVER_INVALID_ROWRECORD_ARRAY, "The vector dimension must be equal to the table dimension."); + return Status(SERVER_INVALID_ROWRECORD_ARRAY, + "The vector dimension must be equal to the table dimension."); } uint64_t query_vec_dim = search_param_->query_record_array(i).vector_data().size(); if (query_vec_dim != table_info.dimension_) { diff --git a/core/src/utils/ValidationUtil.cpp b/core/src/utils/ValidationUtil.cpp index 11ff5da68e..68088d6c93 100644 --- a/core/src/utils/ValidationUtil.cpp +++ b/core/src/utils/ValidationUtil.cpp @@ -74,8 +74,8 @@ ValidationUtil::ValidateTableName(const std::string& table_name) { Status ValidationUtil::ValidateTableDimension(int64_t dimension) { if (dimension <= 0 || dimension > TABLE_DIMENSION_LIMIT) { - std::string msg = "Invalid table dimension: " + std::to_string(dimension) + ". " - + "The table dimension must be within the range of 1 ~ 16384."; + std::string msg = "Invalid table dimension: " + std::to_string(dimension) + ". " + + "The table dimension must be within the range of 1 ~ 16384."; SERVER_LOG_ERROR << msg; return Status(SERVER_INVALID_VECTOR_DIMENSION, msg); } else { @@ -87,8 +87,8 @@ Status ValidationUtil::ValidateTableIndexType(int32_t index_type) { int engine_type = static_cast(engine::EngineType(index_type)); if (engine_type <= 0 || engine_type > static_cast(engine::EngineType::MAX_VALUE)) { - std::string msg = "Invalid index type: " + std::to_string(index_type) + ". " - + "Make sure the index type is in IndexType list."; + std::string msg = "Invalid index type: " + std::to_string(index_type) + ". " + + "Make sure the index type is in IndexType list."; SERVER_LOG_ERROR << msg; return Status(SERVER_INVALID_INDEX_TYPE, msg); } @@ -108,8 +108,8 @@ ValidationUtil::ValidateTableIndexType(int32_t index_type) { Status ValidationUtil::ValidateTableIndexNlist(int32_t nlist) { if (nlist <= 0) { - std::string msg = "Invalid index nlist: " + std::to_string(nlist) + ". " - + "The index nlist must be greater than 0."; + std::string msg = + "Invalid index nlist: " + std::to_string(nlist) + ". " + "The index nlist must be greater than 0."; SERVER_LOG_ERROR << msg; return Status(SERVER_INVALID_INDEX_NLIST, msg); } @@ -120,9 +120,9 @@ ValidationUtil::ValidateTableIndexNlist(int32_t nlist) { Status ValidationUtil::ValidateTableIndexFileSize(int64_t index_file_size) { if (index_file_size <= 0 || index_file_size > INDEX_FILE_SIZE_LIMIT) { - std::string msg = "Invalid index file size: " + std::to_string(index_file_size) + ". " - + "The index file size must be within the range of 1 ~ " - + std::to_string(INDEX_FILE_SIZE_LIMIT) + "."; + std::string msg = "Invalid index file size: " + std::to_string(index_file_size) + ". " + + "The index file size must be within the range of 1 ~ " + + std::to_string(INDEX_FILE_SIZE_LIMIT) + "."; SERVER_LOG_ERROR << msg; return Status(SERVER_INVALID_INDEX_FILE_SIZE, msg); } @@ -134,8 +134,8 @@ Status ValidationUtil::ValidateTableIndexMetricType(int32_t metric_type) { if (metric_type != static_cast(engine::MetricType::L2) && metric_type != static_cast(engine::MetricType::IP)) { - std::string msg = "Invalid index metric type: " + std::to_string(metric_type) + ". " - + "Make sure the metric type is either MetricType.L2 or MetricType.IP."; + std::string msg = "Invalid index metric type: " + std::to_string(metric_type) + ". " + + "Make sure the metric type is either MetricType.L2 or MetricType.IP."; SERVER_LOG_ERROR << msg; return Status(SERVER_INVALID_INDEX_METRIC_TYPE, msg); } @@ -145,8 +145,8 @@ ValidationUtil::ValidateTableIndexMetricType(int32_t metric_type) { Status ValidationUtil::ValidateSearchTopk(int64_t top_k, const engine::meta::TableSchema& table_schema) { if (top_k <= 0 || top_k > 2048) { - std::string msg = "Invalid topk: " + std::to_string(top_k) + ". " - + "The topk must be within the range of 1 ~ 2048."; + std::string msg = + "Invalid topk: " + std::to_string(top_k) + ". " + "The topk must be within the range of 1 ~ 2048."; SERVER_LOG_ERROR << msg; return Status(SERVER_INVALID_TOPK, msg); } @@ -157,8 +157,8 @@ ValidationUtil::ValidateSearchTopk(int64_t top_k, const engine::meta::TableSchem Status ValidationUtil::ValidateSearchNprobe(int64_t nprobe, const engine::meta::TableSchema& table_schema) { if (nprobe <= 0 || nprobe > table_schema.nlist_) { - std::string msg = "Invalid nprobe: " + std::to_string(nprobe) + ". " - + "The nprobe must be within the range of 1 ~ index nlist."; + std::string msg = "Invalid nprobe: " + std::to_string(nprobe) + ". " + + "The nprobe must be within the range of 1 ~ index nlist."; SERVER_LOG_ERROR << msg; return Status(SERVER_INVALID_NPROBE, msg); }