diff --git a/CHANGELOG.md b/CHANGELOG.md index 289836cfed..a85d7a2bbf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ Please mark all change in change log and use the issue from GitHub - \#1571 Meta engine type become IDMAP after dropping index for BINARY table - \#1574 Set all existing bitset in cache when applying deletes - \#1577 Row count incorrect if delete vectors then create index +- \#1580 Old segment folder not removed after merge/compact if create_index is called before adding data - \#1590 Server down caused by failure to write file during concurrent mixed operations ## Feature @@ -79,7 +80,7 @@ Please mark all change in change log and use the issue from GitHub - \#738 Use Openblas / lapack from apt install - \#758 Enhance config description - \#791 Remove Arrow -- \#834 add cpu mode for built-in Faiss +- \#834 Add cpu mode for built-in Faiss - \#848 Add ready-to-use config files to the Milvus repo for enhanced user experince - \#860 Remove redundant checks in CacheMgr's constructor - \#908 Move "primary_path" and "secondary_path" to storage config diff --git a/core/src/db/meta/MySQLMetaImpl.cpp b/core/src/db/meta/MySQLMetaImpl.cpp index bd0d4de96d..ca59d82793 100644 --- a/core/src/db/meta/MySQLMetaImpl.cpp +++ b/core/src/db/meta/MySQLMetaImpl.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include "MetaConsts.h" #include "db/IDGenerator.h" @@ -2026,6 +2027,7 @@ Status MySQLMetaImpl::CleanUpFilesWithTTL(uint64_t seconds /*, CleanUpFilter* filter*/) { auto now = utils::GetMicroSecTimeStamp(); std::set table_ids; + std::map segment_ids; // remove to_delete files try { @@ -2053,7 +2055,7 @@ MySQLMetaImpl::CleanUpFilesWithTTL(uint64_t seconds /*, CleanUpFilter* filter*/) mysqlpp::StoreQueryResult res = query.store(); TableFileSchema table_file; - std::vector idsToDelete; + std::vector delete_ids; int64_t clean_files = 0; for (auto& resRow : res) { @@ -2078,30 +2080,22 @@ MySQLMetaImpl::CleanUpFilesWithTTL(uint64_t seconds /*, CleanUpFilter* filter*/) server::CommonUtil::EraseFromCache(table_file.location_); if (table_file.file_type_ == (int)TableFileSchema::TO_DELETE) { - // If we are deleting a raw table file, it means it's okay to delete the entire segment directory. - // Else, we can only delete the single file - // TODO(zhiru): We determine whether a table file is raw by its engine type. This is a bit hacky - if (utils::IsRawIndexType(table_file.engine_type_)) { - utils::DeleteSegment(options_, table_file); - std::string segment_dir; - utils::GetParentPath(table_file.location_, segment_dir); - ENGINE_LOG_DEBUG << "Remove segment directory: " << segment_dir; - } else { - utils::DeleteTableFilePath(options_, table_file); - ENGINE_LOG_DEBUG << "Remove table file: " << table_file.location_; - } + // delete file from disk storage + utils::DeleteTableFilePath(options_, table_file); + ENGINE_LOG_DEBUG << "Remove file id:" << table_file.id_ << " location:" << table_file.location_; - idsToDelete.emplace_back(std::to_string(table_file.id_)); + delete_ids.emplace_back(std::to_string(table_file.id_)); table_ids.insert(table_file.table_id_); + segment_ids.insert(std::make_pair(table_file.segment_id_, table_file)); - ++clean_files; + clean_files++; } } // delete file from meta - if (!idsToDelete.empty()) { + if (!delete_ids.empty()) { std::stringstream idsToDeleteSS; - for (auto& id : idsToDelete) { + for (auto& id : delete_ids) { idsToDeleteSS << "id = " << id << " OR "; } @@ -2217,6 +2211,51 @@ MySQLMetaImpl::CleanUpFilesWithTTL(uint64_t seconds /*, CleanUpFilter* filter*/) return HandleException("GENERAL ERROR WHEN CLEANING UP TABLES WITH TTL", e.what()); } + // remove deleted segment folder + // don't remove segment folder until all its tablefiles has been deleted + try { + server::MetricCollector metric; + + { + mysqlpp::ScopedConnection connectionPtr(*mysql_connection_pool_, safe_grab_); + + bool is_null_connection = (connectionPtr == nullptr); + fiu_do_on("MySQLMetaImpl.CleanUpFilesWithTTL.RemoveDeletedSegmentFolder_NUllConnection", + is_null_connection = true); + fiu_do_on("MySQLMetaImpl.CleanUpFilesWithTTL.RemoveDeletedSegmentFolder_ThrowException", + throw std::exception();); + if (is_null_connection) { + return Status(DB_ERROR, "Failed to connect to meta server(mysql)"); + } + + int64_t remove_segments = 0; + for (auto& segment_id : segment_ids) { + mysqlpp::Query query = connectionPtr->query(); + query << "SELECT id" + << " FROM " << META_TABLEFILES << " WHERE segment_id = " << mysqlpp::quote << segment_id.first + << ";"; + + ENGINE_LOG_DEBUG << "MySQLMetaImpl::CleanUpFilesWithTTL: " << query.str(); + + mysqlpp::StoreQueryResult res = query.store(); + + if (res.empty()) { + utils::DeleteSegment(options_, segment_id.second); + std::string segment_dir; + utils::GetParentPath(segment_id.second.location_, segment_dir); + ENGINE_LOG_DEBUG << "Remove segment directory: " << segment_dir; + ++remove_segments; + } + } + + if (remove_segments > 0) { + ENGINE_LOG_DEBUG << "Remove " << remove_segments << " segments folder"; + } + } + } catch (std::exception& e) { + return HandleException("GENERAL ERROR WHEN CLEANING UP TABLES WITH TTL", e.what()); + } + return Status::OK(); } diff --git a/core/src/db/meta/SqliteMetaImpl.cpp b/core/src/db/meta/SqliteMetaImpl.cpp index ce5ff99742..622d36175e 100644 --- a/core/src/db/meta/SqliteMetaImpl.cpp +++ b/core/src/db/meta/SqliteMetaImpl.cpp @@ -1231,11 +1231,13 @@ SqliteMetaImpl::FilesByType(const std::string& table_id, const std::vector& break; case (int)TableFileSchema::NEW:msg = msg + " new files:" + std::to_string(new_count); break; - case (int)TableFileSchema::NEW_MERGE:msg = msg + " new_merge files:" - + std::to_string(new_merge_count); + case (int)TableFileSchema::NEW_MERGE: + msg = msg + " new_merge files:" + + std::to_string(new_merge_count); break; - case (int)TableFileSchema::NEW_INDEX:msg = msg + " new_index files:" - + std::to_string(new_index_count); + case (int)TableFileSchema::NEW_INDEX: + msg = msg + " new_index files:" + + std::to_string(new_index_count); break; case (int)TableFileSchema::TO_INDEX:msg = msg + " to_index files:" + std::to_string(to_index_count); break; @@ -1360,6 +1362,7 @@ Status SqliteMetaImpl::CleanUpFilesWithTTL(uint64_t seconds /*, CleanUpFilter* filter*/) { auto now = utils::GetMicroSecTimeStamp(); std::set table_ids; + std::map segment_ids; // remove to_delete files try { @@ -1409,23 +1412,16 @@ SqliteMetaImpl::CleanUpFilesWithTTL(uint64_t seconds /*, CleanUpFilter* filter*/ server::CommonUtil::EraseFromCache(table_file.location_); if (table_file.file_type_ == (int)TableFileSchema::TO_DELETE) { - // If we are deleting a raw table file, it means it's okay to delete the entire segment directory. - // Else, we can only delete the single file - // TODO(zhiru): We determine whether a table file is raw by its engine type. This is a bit hacky - if (utils::IsRawIndexType(table_file.engine_type_)) { - utils::DeleteSegment(options_, table_file); - std::string segment_dir; - utils::GetParentPath(table_file.location_, segment_dir); - ENGINE_LOG_DEBUG << "Remove segment directory: " << segment_dir; - } else { - utils::DeleteTableFilePath(options_, table_file); - ENGINE_LOG_DEBUG << "Remove table file: " << table_file.location_; - } - // delete file from meta ConnectorPtr->remove(table_file.id_); + // delete file from disk storage + utils::DeleteTableFilePath(options_, table_file); + + ENGINE_LOG_DEBUG << "Remove file id:" << table_file.file_id_ << " location:" + << table_file.location_; table_ids.insert(table_file.table_id_); + segment_ids.insert(std::make_pair(table_file.segment_id_, table_file)); ++clean_files; } @@ -1500,6 +1496,32 @@ SqliteMetaImpl::CleanUpFilesWithTTL(uint64_t seconds /*, CleanUpFilter* filter*/ return HandleException("Encounter exception when delete table folder", e.what()); } + // remove deleted segment folder + // don't remove segment folder until all its tablefiles has been deleted + try { + fiu_do_on("SqliteMetaImpl.CleanUpFilesWithTTL.RemoveSegmentFolder_ThrowException", throw std::exception()); + server::MetricCollector metric; + + int64_t remove_segments = 0; + for (auto& segment_id : segment_ids) { + auto selected = ConnectorPtr->select(columns(&TableFileSchema::id_), + where(c(&TableFileSchema::segment_id_) == segment_id.first)); + if (selected.size() == 0) { + utils::DeleteSegment(options_, segment_id.second); + std::string segment_dir; + utils::GetParentPath(segment_id.second.location_, segment_dir); + ENGINE_LOG_DEBUG << "Remove segment directory: " << segment_dir; + ++remove_segments; + } + } + + if (remove_segments > 0) { + ENGINE_LOG_DEBUG << "Remove " << remove_segments << " segments folder"; + } + } catch (std::exception& e) { + return HandleException("Encounter exception when delete table folder", e.what()); + } + return Status::OK(); }