From b2f0ca495df93d751d3949072706e6d8b8694e44 Mon Sep 17 00:00:00 2001 From: Kishore Nallan Date: Thu, 21 Sep 2017 07:10:18 +0530 Subject: [PATCH] Refactor a couple of methods in collection manager. --- TODO.md | 14 ++++++------ include/collection_manager.h | 2 +- include/replicator.h | 13 +++++------ src/collection_manager.cpp | 42 ++++++++++++++++-------------------- 4 files changed, 34 insertions(+), 37 deletions(-) diff --git a/TODO.md b/TODO.md index 6fcb2190..e23bbf8f 100644 --- a/TODO.md +++ b/TODO.md @@ -58,20 +58,23 @@ - ~~Prefix-search strings should not be null terminated~~ - ~~sort results by float field~~ - ~~json::parse must be wrapped in try catch~~ -- Collection Manager collections map should store plain collection name -- init_collection of Collection manager should probably take seq_id as param +- ~~Collection Manager collections map should store plain collection name~~ +- ~~init_collection of Collection manager should probably take seq_id as param~~ +- node score should be int32, no longer uint16 like in document struct +- Proper logging - https support - Validate before string to int conversion in the http api layer - When field of "id" but not string, what happens? - Typo in prefix search -- node score should be int32, no longer uint16 like in document struct - test for num_documents - test for string filter comparison: title < "foo" - test for token ranking on float field - test for float int field deletion during doc deletion +- Test for sorted_array::indexOf when length is 0 +- Test for snippets +- Test for pagination - > INT32_MAX validation for float field - art bool support -- Proper logging - Add docs/explanation around ranking calc - Use rocksdb batch put for atomic insertion - Query token ids should match query token ordering @@ -79,9 +82,6 @@ - Group results by field - Handle store-get() not finding a key - Delete using range: https://github.com/facebook/rocksdb/wiki/Delete-A-Range-Of-Keys -- Test for sorted_array::indexOf when length is 0 -- Test for snippets -- Test for pagination - Test for string utils - Prevent string copy during indexing - clean special chars before indexing diff --git a/include/collection_manager.h b/include/collection_manager.h index ea1a6b8e..479c1a0f 100644 --- a/include/collection_manager.h +++ b/include/collection_manager.h @@ -47,7 +47,7 @@ public: // frees in-memory data structures when server is shutdown - helps us run a memory leak detecter properly void dispose(); - Option init_collection(const std::string & collection_meta_json); + Collection* init_collection(const nlohmann::json & collection_meta, const uint32_t collection_next_seq_id); void add_to_collections(Collection* collection); diff --git a/include/replicator.h b/include/replicator.h index 514368fb..a44c4c1b 100644 --- a/include/replicator.h +++ b/include/replicator.h @@ -130,17 +130,18 @@ public: } if(replication_event->type == "ADD_COLLECTION_META") { - CollectionManager & collection_manager = CollectionManager::get_instance(); - Option collection_op = collection_manager.init_collection(replication_event->value); - - if(!collection_op.ok()) { - std::cerr << "Failed to initialize collection. Error: " << collection_op.error() << std::endl; + nlohmann::json collection_meta; + try { + collection_meta = nlohmann::json::parse(replication_event->value); + } catch(...) { + std::cerr << "Failed to parse collection meta JSON." << std::endl; std::cerr << "Replication event value: " << replication_event->value << std::endl; delete replication_event; exit(1); } - Collection* collection = collection_op.get(); + CollectionManager & collection_manager = CollectionManager::get_instance(); + Collection* collection = collection_manager.init_collection(replication_event->value, 0); collection_manager.add_to_collections(collection); } diff --git a/src/collection_manager.cpp b/src/collection_manager.cpp index ddfa19a0..a1655486 100644 --- a/src/collection_manager.cpp +++ b/src/collection_manager.cpp @@ -8,15 +8,8 @@ CollectionManager::CollectionManager() { } -Option CollectionManager::init_collection(const std::string & collection_meta_json) { - nlohmann::json collection_meta; - - try { - collection_meta = nlohmann::json::parse(collection_meta_json); - } catch(...) { - return Option(500, "Error while parsing collection meta."); - } - +Collection* CollectionManager::init_collection(const nlohmann::json & collection_meta, + const uint32_t collection_next_seq_id) { std::string this_collection_name = collection_meta[COLLECTION_NAME_KEY].get(); std::vector search_fields; @@ -33,11 +26,6 @@ Option CollectionManager::init_collection(const std::string & colle facet_fields.push_back({it.value()[fields::name], it.value()[fields::type]}); } - std::string collection_next_seq_id_str; - store->get(Collection::get_next_seq_id_key(this_collection_name), collection_next_seq_id_str); - uint32_t collection_next_seq_id = collection_next_seq_id_str.size() == 0 ? 0 : - (const uint32_t) std::stoi(collection_next_seq_id_str); - std::vector collection_sort_fields; nlohmann::json sort_fields_map = collection_meta[COLLECTION_SORT_FIELDS_KEY]; @@ -56,11 +44,11 @@ Option CollectionManager::init_collection(const std::string & colle collection_sort_fields, token_ranking_field); - return Option(collection); + return collection; } void CollectionManager::add_to_collections(Collection* collection) { - collections.emplace(Collection::get_meta_key(collection->get_name()), collection); + collections.emplace(collection->get_name(), collection); collection_id_names.emplace(collection->get_collection_id(), collection->get_name()); } @@ -80,13 +68,21 @@ Option CollectionManager::init(Store *store, const std::string & auth_key) store->scan_fill(Collection::COLLECTION_META_PREFIX, collection_meta_jsons); for(auto & collection_meta_json: collection_meta_jsons) { - Option collection_op = init_collection(collection_meta_json); + nlohmann::json collection_meta; - if(!collection_op.ok()) { - return Option(collection_op.code(), collection_op.error()); + try { + collection_meta = nlohmann::json::parse(collection_meta_json); + } catch(...) { + return Option(500, "Error while parsing collection meta."); } - Collection* collection = collection_op.get(); + const std::string & this_collection_name = collection_meta[COLLECTION_NAME_KEY].get(); + std::string collection_next_seq_id_str; + store->get(Collection::get_next_seq_id_key(this_collection_name), collection_next_seq_id_str); + uint32_t collection_next_seq_id = collection_next_seq_id_str.size() == 0 ? 0 : + (const uint32_t) std::stoi(collection_next_seq_id_str); + + Collection* collection = init_collection(collection_meta, collection_next_seq_id); // Fetch records from the store and re-create memory index std::vector documents; @@ -187,8 +183,8 @@ Option CollectionManager::create_collection(std::string name, const } Collection* CollectionManager::get_collection(const std::string & collection_name) { - if(collections.count(Collection::get_meta_key(collection_name)) != 0) { - return collections.at(Collection::get_meta_key(collection_name)); + if(collections.count(collection_name) != 0) { + return collections.at(collection_name); } return nullptr; @@ -238,7 +234,7 @@ Option CollectionManager::drop_collection(std::string collection_name, con store->remove(Collection::get_meta_key(collection_name)); } - collections.erase(Collection::get_meta_key(collection_name)); + collections.erase(collection_name); collection_id_names.erase(collection->get_collection_id()); delete collection;