From 34f25eaaff900846895b1a30e9246f9bef658282 Mon Sep 17 00:00:00 2001 From: Zhang Lei Date: Tue, 6 Aug 2024 19:35:14 +0800 Subject: [PATCH] refactor(interactive): Replace `CHECK` with std::runtime_error in `schema.cc` (#4107) We previously used `CHECK` to verify schema operations, but its lack of robustness could lead to server downtime. We have replaced glog CHECK with exception throwing. --- flex/engines/hqps_db/core/base_engine.h | 14 +- flex/engines/hqps_db/core/operator/get_v.h | 10 +- flex/engines/hqps_db/core/operator/group_by.h | 1 - flex/storages/rt_mutable_graph/schema.cc | 194 ++++++++++-------- 4 files changed, 127 insertions(+), 92 deletions(-) diff --git a/flex/engines/hqps_db/core/base_engine.h b/flex/engines/hqps_db/core/base_engine.h index fd4307c90b10..fd56c4410b8d 100644 --- a/flex/engines/hqps_db/core/base_engine.h +++ b/flex/engines/hqps_db/core/base_engine.h @@ -828,9 +828,11 @@ class BaseEngine { auto& head_y = ctx_y.GetMutableHead(); auto left_repeat_array = ctx_x.ObtainOffsetFromTag(real_alias_x - 1); auto right_repeat_array = ctx_y.ObtainOffsetFromTag(real_alias_y - 1); - CHECK(left_repeat_array.size() == right_repeat_array.size()) - << "left size " << left_repeat_array.size() << " right size " - << right_repeat_array.size(); + if (left_repeat_array.size() != right_repeat_array.size()) { + throw std::runtime_error("The two context has different repeat size.: " + + std::to_string(left_repeat_array.size()) + ", " + + std::to_string(right_repeat_array.size())); + } std::vector active_indices, new_offsets; std::tie(active_indices, new_offsets) = @@ -867,7 +869,11 @@ class BaseEngine { } grape::Bitset bitset; bitset.init(max_vid + 1); - CHECK(left_repeat_array.size() == right_repeat_array.size()); + if (left_repeat_array.size() != right_repeat_array.size()) { + throw std::runtime_error("The two context has different repeat size.: " + + std::to_string(left_repeat_array.size()) + ", " + + std::to_string(right_repeat_array.size())); + } for (size_t i = 0; i + 1 < left_repeat_array.size(); ++i) { auto x_start = left_repeat_array[i]; auto x_end = left_repeat_array[i + 1]; diff --git a/flex/engines/hqps_db/core/operator/get_v.h b/flex/engines/hqps_db/core/operator/get_v.h index e396a20e608b..887d66ee33c5 100644 --- a/flex/engines/hqps_db/core/operator/get_v.h +++ b/flex/engines/hqps_db/core/operator/get_v.h @@ -109,8 +109,10 @@ class GetVertex { const GRAPH_INTERFACE& graph, const SET_T& set, const GetVOpt& get_v_opt) { auto v_opt = get_v_opt.v_opt_; - CHECK(v_opt == VOpt::Itself) - << "Can only get v from vertex set with v_opt == vopt::Itself"; + if (v_opt != VOpt::Itself) { + throw std::runtime_error( + "Can only get v from vertex set with v_opt == vopt::Itself"); + } auto v_labels = get_v_opt.v_labels_; auto props = get_v_opt.props_; auto expr = get_v_opt.expr_; @@ -236,7 +238,9 @@ class GetVertex { std::sort(labels.begin(), labels.end()); labels.erase(std::unique(labels.begin(), labels.end()), labels.end()); // Can only be one label. - CHECK(labels.size() == 1); + if (labels.size() != 1) { + throw std::runtime_error("Path set should have only one label"); + } // if req_labels is empty, then use the label from path set. if (req_label_vec.empty()) { req_label_vec.push_back(labels[0]); diff --git a/flex/engines/hqps_db/core/operator/group_by.h b/flex/engines/hqps_db/core/operator/group_by.h index c2ee6e0df8e6..ff261a3f163e 100644 --- a/flex/engines/hqps_db/core/operator/group_by.h +++ b/flex/engines/hqps_db/core/operator/group_by.h @@ -489,7 +489,6 @@ class GroupByOp { } auto data_ele = gs::get_from_tuple(data_tuple); int32_t ind = insert_to_keyed_set(keyed_set_builder, key_ele, data_ele); - // CHECK(ind != -1); if (ind != -1) { insert_to_value_set_builder(value_set_builder_tuple, ele_tuple, data_tuple, ind); diff --git a/flex/storages/rt_mutable_graph/schema.cc b/flex/storages/rt_mutable_graph/schema.cc index 4e760b0e2ac5..4326858a4836 100644 --- a/flex/storages/rt_mutable_graph/schema.cc +++ b/flex/storages/rt_mutable_graph/schema.cc @@ -19,6 +19,11 @@ namespace gs { +#define THROW_EXCEPTION_IF(cond, msg) \ + if (cond) { \ + throw std::runtime_error(msg); \ + } + Schema::Schema() = default; Schema::~Schema() = default; @@ -98,11 +103,9 @@ bool Schema::contains_vertex_label(const std::string& label) const { label_t Schema::get_vertex_label_id(const std::string& label) const { label_t ret; - if (vlabel_indexer_.get_index(label, ret)) { - return ret; - } else { - throw std::runtime_error("Fail to get vertex label: " + label); - } + THROW_EXCEPTION_IF(!vlabel_indexer_.get_index(label, ret), + "Fail to get vertex label: " + label); + return ret; } void Schema::set_vertex_properties( @@ -115,8 +118,7 @@ void Schema::set_vertex_properties( const std::vector& Schema::get_vertex_properties( const std::string& label) const { - label_t index; - CHECK(vlabel_indexer_.get_index(label, index)); + label_t index = get_vertex_label_id(label); return vproperties_[index]; } @@ -127,53 +129,56 @@ const std::vector& Schema::get_vertex_properties( const std::vector& Schema::get_vertex_property_names( const std::string& label) const { - label_t index; - CHECK(vlabel_indexer_.get_index(label, index)); - CHECK(index < vprop_names_.size()); - return vprop_names_[index]; + label_t index = get_vertex_label_id(label); + return get_vertex_property_names(index); } const std::vector& Schema::get_vertex_property_names( label_t label) const { - if (label < vprop_names_.size()) { - return vprop_names_[label]; - } else { - throw std::runtime_error("Fail to get vertex property names: " + - std::to_string(label)); - } + THROW_EXCEPTION_IF( + label >= vprop_names_.size(), + "Fail to get vertex property names: " + std::to_string(label) + + ", out of range of vprop_names_ " + + std::to_string(vprop_names_.size())); + return vprop_names_[label]; } const std::string& Schema::get_vertex_description( const std::string& label) const { - label_t index; - CHECK(vlabel_indexer_.get_index(label, index)); + label_t index = get_vertex_label_id(label); return get_vertex_description(index); } const std::string& Schema::get_vertex_description(label_t label) const { - CHECK(label < v_descriptions_.size()); + THROW_EXCEPTION_IF( + label >= v_descriptions_.size(), + "Fail to get vertex description: " + std::to_string(label) + + ", out of range of v_descriptions_ " + + std::to_string(v_descriptions_.size())); return v_descriptions_[label]; } const std::vector& Schema::get_vertex_storage_strategies( const std::string& label) const { - label_t index; - CHECK(vlabel_indexer_.get_index(label, index)); + label_t index = get_vertex_label_id(label); + THROW_EXCEPTION_IF( + index >= vprop_storage_.size(), + "Fail to get vertex storage strategies: " + std::to_string(index) + + ", out of range of vprop_storage_ " + + std::to_string(vprop_storage_.size())); return vprop_storage_[index]; } size_t Schema::get_max_vnum(const std::string& label) const { - label_t index; - CHECK(vlabel_indexer_.get_index(label, index)); + label_t index = get_vertex_label_id(label); return max_vnum_[index]; } bool Schema::exist(const std::string& src_label, const std::string& dst_label, const std::string& edge_label) const { - label_t src, dst, edge; - CHECK(vlabel_indexer_.get_index(src_label, src)); - CHECK(vlabel_indexer_.get_index(dst_label, dst)); - CHECK(elabel_indexer_.get_index(edge_label, edge)); + label_t src = get_vertex_label_id(src_label); + label_t dst = get_vertex_label_id(dst_label); + label_t edge = get_edge_label_id(edge_label); uint32_t index = generate_edge_label(src, dst, edge); return eproperties_.find(index) != eproperties_.end(); } @@ -187,19 +192,23 @@ bool Schema::exist(label_t src_label, label_t dst_label, const std::vector& Schema::get_edge_properties( const std::string& src_label, const std::string& dst_label, const std::string& label) const { - label_t src, dst, edge; - CHECK(vlabel_indexer_.get_index(src_label, src)); - CHECK(vlabel_indexer_.get_index(dst_label, dst)); - CHECK(elabel_indexer_.get_index(label, edge)); + label_t src = get_vertex_label_id(src_label); + label_t dst = get_vertex_label_id(dst_label); + label_t edge = get_edge_label_id(label); uint32_t index = generate_edge_label(src, dst, edge); return eproperties_.at(index); } const std::vector& Schema::get_edge_properties( label_t src_label, label_t dst_label, label_t label) const { - CHECK(src_label < vlabel_indexer_.size()); - CHECK(dst_label < vlabel_indexer_.size()); - CHECK(label < elabel_indexer_.size()); + THROW_EXCEPTION_IF( + src_label >= vlabel_indexer_.size(), + "vertex label " + std::to_string(src_label) + " not found"); + THROW_EXCEPTION_IF( + dst_label >= vlabel_indexer_.size(), + "vertex label " + std::to_string(dst_label) + " not found"); + THROW_EXCEPTION_IF(label >= elabel_indexer_.size(), + "edge label " + std::to_string(label) + " not found"); uint32_t index = generate_edge_label(src_label, dst_label, label); return eproperties_.at(index); } @@ -207,17 +216,27 @@ const std::vector& Schema::get_edge_properties( std::string Schema::get_edge_description(const std::string& src_label, const std::string& dst_label, const std::string& label) const { - label_t src, dst, edge; - CHECK(vlabel_indexer_.get_index(src_label, src)); - CHECK(vlabel_indexer_.get_index(dst_label, dst)); - CHECK(elabel_indexer_.get_index(label, edge)); + label_t src = get_vertex_label_id(src_label); + label_t dst = get_vertex_label_id(dst_label); + label_t edge = get_edge_label_id(label); return get_edge_description(src, dst, edge); } std::string Schema::get_edge_description(label_t src_label, label_t dst_label, label_t label) const { + THROW_EXCEPTION_IF( + src_label >= vlabel_indexer_.size(), + "vertex label " + std::to_string(src_label) + " not found"); + THROW_EXCEPTION_IF( + dst_label >= vlabel_indexer_.size(), + "vertex label " + std::to_string(dst_label) + " not found"); + THROW_EXCEPTION_IF(label >= elabel_indexer_.size(), + "edge label " + std::to_string(label) + " not found"); uint32_t index = generate_edge_label(src_label, dst_label, label); - CHECK(index < e_descriptions_.size()); + THROW_EXCEPTION_IF(index >= e_descriptions_.size(), + "Fail to get edge description: " + std::to_string(index) + + ", out of range of e_descriptions_ " + + std::to_string(e_descriptions_.size())); return e_descriptions_.at(index); } @@ -230,20 +249,23 @@ PropertyType Schema::get_edge_property(label_t src, label_t dst, const std::vector& Schema::get_edge_property_names( const std::string& src_label, const std::string& dst_label, const std::string& label) const { - label_t src, dst, edge; - CHECK(vlabel_indexer_.get_index(src_label, src)); - CHECK(vlabel_indexer_.get_index(dst_label, dst)); - CHECK(elabel_indexer_.get_index(label, edge)); - uint32_t index = generate_edge_label(src, dst, edge); - return eprop_names_.at(index); + label_t src = get_vertex_label_id(src_label); + label_t dst = get_vertex_label_id(dst_label); + label_t edge = get_edge_label_id(label); + return get_edge_property_names(src, dst, edge); } const std::vector& Schema::get_edge_property_names( const label_t& src_label, const label_t& dst_label, const label_t& label) const { - CHECK(src_label < vlabel_indexer_.size()); - CHECK(dst_label < vlabel_indexer_.size()); - CHECK(label < elabel_indexer_.size()); + THROW_EXCEPTION_IF( + src_label >= vlabel_indexer_.size(), + "vertex label " + std::to_string(src_label) + " not found"); + THROW_EXCEPTION_IF( + dst_label >= vlabel_indexer_.size(), + "vertex label " + std::to_string(dst_label) + " not found"); + THROW_EXCEPTION_IF(label >= elabel_indexer_.size(), + "edge label " + std::to_string(label) + " not found"); uint32_t index = generate_edge_label(src_label, dst_label, label); return eprop_names_.at(index); } @@ -251,10 +273,9 @@ const std::vector& Schema::get_edge_property_names( bool Schema::valid_edge_property(const std::string& src_label, const std::string& dst_label, const std::string& label) const { - label_t src, dst, edge; - CHECK(vlabel_indexer_.get_index(src_label, src)); - CHECK(vlabel_indexer_.get_index(dst_label, dst)); - CHECK(elabel_indexer_.get_index(label, edge)); + label_t src = get_vertex_label_id(src_label); + label_t dst = get_vertex_label_id(dst_label); + label_t edge = get_edge_label_id(label); uint32_t index = generate_edge_label(src, dst, edge); return eproperties_.find(index) != eproperties_.end(); } @@ -262,10 +283,9 @@ bool Schema::valid_edge_property(const std::string& src_label, EdgeStrategy Schema::get_outgoing_edge_strategy( const std::string& src_label, const std::string& dst_label, const std::string& label) const { - label_t src, dst, edge; - CHECK(vlabel_indexer_.get_index(src_label, src)); - CHECK(vlabel_indexer_.get_index(dst_label, dst)); - CHECK(elabel_indexer_.get_index(label, edge)); + label_t src = get_vertex_label_id(src_label); + label_t dst = get_vertex_label_id(dst_label); + label_t edge = get_edge_label_id(label); uint32_t index = generate_edge_label(src, dst, edge); return oe_strategy_.at(index); } @@ -273,10 +293,9 @@ EdgeStrategy Schema::get_outgoing_edge_strategy( EdgeStrategy Schema::get_incoming_edge_strategy( const std::string& src_label, const std::string& dst_label, const std::string& label) const { - label_t src, dst, edge; - CHECK(vlabel_indexer_.get_index(src_label, src)); - CHECK(vlabel_indexer_.get_index(dst_label, dst)); - CHECK(elabel_indexer_.get_index(label, edge)); + label_t src = get_vertex_label_id(src_label); + label_t dst = get_vertex_label_id(dst_label); + label_t edge = get_edge_label_id(label); uint32_t index = generate_edge_label(src, dst, edge); return ie_strategy_.at(index); } @@ -284,10 +303,9 @@ EdgeStrategy Schema::get_incoming_edge_strategy( bool Schema::outgoing_edge_mutable(const std::string& src_label, const std::string& dst_label, const std::string& label) const { - label_t src, dst, edge; - CHECK(vlabel_indexer_.get_index(src_label, src)); - CHECK(vlabel_indexer_.get_index(dst_label, dst)); - CHECK(elabel_indexer_.get_index(label, edge)); + label_t src = get_vertex_label_id(src_label); + label_t dst = get_vertex_label_id(dst_label); + label_t edge = get_edge_label_id(label); uint32_t index = generate_edge_label(src, dst, edge); return oe_mutability_.at(index); } @@ -295,10 +313,9 @@ bool Schema::outgoing_edge_mutable(const std::string& src_label, bool Schema::incoming_edge_mutable(const std::string& src_label, const std::string& dst_label, const std::string& label) const { - label_t src, dst, edge; - CHECK(vlabel_indexer_.get_index(src_label, src)); - CHECK(vlabel_indexer_.get_index(dst_label, dst)); - CHECK(elabel_indexer_.get_index(label, edge)); + label_t src = get_vertex_label_id(src_label); + label_t dst = get_vertex_label_id(dst_label); + label_t edge = get_edge_label_id(label); uint32_t index = generate_edge_label(src, dst, edge); return ie_mutability_.at(index); } @@ -306,18 +323,22 @@ bool Schema::incoming_edge_mutable(const std::string& src_label, bool Schema::get_sort_on_compaction(const std::string& src_label, const std::string& dst_label, const std::string& label) const { - label_t src, dst, edge; - CHECK(vlabel_indexer_.get_index(src_label, src)); - CHECK(vlabel_indexer_.get_index(dst_label, dst)); - CHECK(elabel_indexer_.get_index(label, edge)); + label_t src = get_vertex_label_id(src_label); + label_t dst = get_vertex_label_id(dst_label); + label_t edge = get_edge_label_id(label); uint32_t index = generate_edge_label(src, dst, edge); - CHECK(sort_on_compactions_.find(index) != sort_on_compactions_.end()); + THROW_EXCEPTION_IF( + sort_on_compactions_.find(index) == sort_on_compactions_.end(), + "Fail to get sort on compaction: " + std::to_string(index) + + ", out of range of sort_on_compactions_ " + + std::to_string(sort_on_compactions_.size())); return sort_on_compactions_.at(index); } label_t Schema::get_edge_label_id(const std::string& label) const { label_t ret; - CHECK(elabel_indexer_.get_index(label, ret)); + THROW_EXCEPTION_IF(!elabel_indexer_.get_index(label, ret), + "Edge label " + label + " not found"); return ret; } @@ -328,23 +349,26 @@ bool Schema::contains_edge_label(const std::string& label) const { std::string Schema::get_vertex_label_name(label_t index) const { std::string ret; - vlabel_indexer_.get_key(index, ret); + THROW_EXCEPTION_IF( + !vlabel_indexer_.get_key(index, ret), + "No vertex label found for label id: " + std::to_string(index)); return ret; } std::string Schema::get_edge_label_name(label_t index) const { std::string ret; - elabel_indexer_.get_key(index, ret); + THROW_EXCEPTION_IF( + !elabel_indexer_.get_key(index, ret), + "No edge label found for label id: " + std::to_string(index)); return ret; } const std::vector>& Schema::get_vertex_primary_key(label_t index) const { - if (v_primary_keys_.size() > index) { - return v_primary_keys_.at(index); - } - throw std::runtime_error("Fail to get vertex primary key: " + - std::to_string(index)); + THROW_EXCEPTION_IF(index >= v_primary_keys_.size(), + "Fail to get vertex primary key: " + + std::to_string(index) + ", out of range"); + return v_primary_keys_[index]; } // Note that plugin_dir_ and plugin_name_to_path_and_id_ are not serialized. @@ -1296,7 +1320,8 @@ std::string Schema::GetVersion() const { return version_; } bool Schema::vertex_has_property(const std::string& label, const std::string& prop) const { auto v_label_id = get_vertex_label_id(label); - CHECK(v_label_id < vprop_names_.size()); + THROW_EXCEPTION_IF(v_label_id >= vprop_names_.size(), + "vertex label id out of range of vprop_names_"); auto& v_prop_names = vprop_names_[v_label_id]; return std::find(v_prop_names.begin(), v_prop_names.end(), prop) != v_prop_names.end() || @@ -1306,7 +1331,8 @@ bool Schema::vertex_has_property(const std::string& label, bool Schema::vertex_has_primary_key(const std::string& label, const std::string& prop) const { auto v_label_id = get_vertex_label_id(label); - CHECK(v_label_id < vprop_names_.size()); + THROW_EXCEPTION_IF(v_label_id >= v_primary_keys_.size(), + "vertex label id out of range of v_primary_keys_"); auto& keys = v_primary_keys_[v_label_id]; for (size_t i = 0; i < keys.size(); ++i) { if (std::get<1>(keys[i]) == prop) {