From ae99d04bac4eabb9fbcc5ef4cb2f3957a4788f62 Mon Sep 17 00:00:00 2001 From: Jacky <18255193+kthui@users.noreply.github.com> Date: Tue, 20 Aug 2024 12:22:37 -0700 Subject: [PATCH] feat: Load new model version should not reload loaded existing model version(s) (#388) * Do not reload unmodified loaded model version * Track model directory timestamps more granularly to detect updates to model version files * Rename model config util config change require reload function * Re-organize ModelTimestamp() and throw exception * Revert "Re-organize ModelTimestamp() and throw exception" This reverts commit 41e57e55cf55c3f0496fd95735e06e4764cf5659. * Break constructor into multiple functions * Comment on should raise an exception when failed to create timestamp --- src/model_config_utils.cc | 6 +- src/model_config_utils.h | 10 +- .../model_lifecycle.cc | 7 +- .../model_lifecycle.h | 6 +- .../model_repository_manager.cc | 207 +++++++++++++----- .../model_repository_manager.h | 48 +++- 6 files changed, 204 insertions(+), 80 deletions(-) diff --git a/src/model_config_utils.cc b/src/model_config_utils.cc index cc31f666c..4fd1e1be0 100644 --- a/src/model_config_utils.cc +++ b/src/model_config_utils.cc @@ -2432,14 +2432,16 @@ TritonToDataType(const TRITONSERVER_DataType dtype) } bool -EquivalentInNonInstanceGroupConfig( +ConfigChangeRequiresReload( const inference::ModelConfig& old_config, const inference::ModelConfig& new_config) { ::google::protobuf::util::MessageDifferencer pb_diff; pb_diff.IgnoreField( old_config.descriptor()->FindFieldByLowercaseName("instance_group")); - return pb_diff.Compare(old_config, new_config); + pb_diff.IgnoreField( + old_config.descriptor()->FindFieldByLowercaseName("version_policy")); + return !pb_diff.Compare(old_config, new_config); } bool diff --git a/src/model_config_utils.h b/src/model_config_utils.h index 8bd9af600..ba4d75636 100644 --- a/src/model_config_utils.h +++ b/src/model_config_utils.h @@ -288,12 +288,14 @@ TRITONSERVER_DataType DataTypeToTriton(const inference::DataType dtype); /// \return The data type. inference::DataType TritonToDataType(const TRITONSERVER_DataType dtype); -/// Check if non instance group settings on the model configs are equivalent. +/// Check if non instance group and non version policy settings on the model +/// configs are equivalent. /// \param old_config The old model config. /// \param new_config The new model config. -/// \return True if the model configs are equivalent in all non instance group -/// settings. False if they differ in non instance group settings. -bool EquivalentInNonInstanceGroupConfig( +/// \return False if the model configs are equivalent in all non instance group +/// and non version policy settings. True if they differ in non instance group +/// and non version policy settings. +bool ConfigChangeRequiresReload( const inference::ModelConfig& old_config, const inference::ModelConfig& new_config); diff --git a/src/model_repository_manager/model_lifecycle.cc b/src/model_repository_manager/model_lifecycle.cc index 437829a4c..a04d99a97 100644 --- a/src/model_repository_manager/model_lifecycle.cc +++ b/src/model_repository_manager/model_lifecycle.cc @@ -434,7 +434,7 @@ Status ModelLifeCycle::AsyncLoad( const ModelIdentifier& model_id, const std::string& model_path, const inference::ModelConfig& model_config, const bool is_config_provided, - const bool is_model_file_updated, + const ModelTimestamp& prev_timestamp, const ModelTimestamp& curr_timestamp, const std::shared_ptr& agent_model_list, std::function&& OnComplete) { @@ -488,8 +488,9 @@ ModelLifeCycle::AsyncLoad( if (serving_model->state_ == ModelReadyState::READY) { // The model is currently being served. Check if the model load could // be completed with a simple config update. - if (!is_model_file_updated && !serving_model->is_ensemble_ && - EquivalentInNonInstanceGroupConfig( + if (!serving_model->is_ensemble_ && + !prev_timestamp.IsModelVersionModified(curr_timestamp, version) && + !ConfigChangeRequiresReload( serving_model->model_config_, model_config)) { // Update the model model_info = serving_model.get(); diff --git a/src/model_repository_manager/model_lifecycle.h b/src/model_repository_manager/model_lifecycle.h index 7016ef29a..13d3aa54d 100644 --- a/src/model_repository_manager/model_lifecycle.h +++ b/src/model_repository_manager/model_lifecycle.h @@ -1,4 +1,4 @@ -// Copyright 2022-2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// Copyright 2022-2024, NVIDIA CORPORATION & AFFILIATES. All rights reserved. // // Redistribution and use in source and binary forms, with or without // modification, are permitted provided that the following conditions @@ -162,6 +162,7 @@ class TritonRepoAgentModelList { }; class InferenceServer; +class ModelTimestamp; class ModelLifeCycle { public: @@ -183,7 +184,8 @@ class ModelLifeCycle { Status AsyncLoad( const ModelIdentifier& model_id, const std::string& model_path, const inference::ModelConfig& model_config, const bool is_config_provided, - const bool is_model_file_updated, + const ModelTimestamp& prev_timestamp, + const ModelTimestamp& curr_timestamp, const std::shared_ptr& agent_model_list, std::function&& OnComplete); diff --git a/src/model_repository_manager/model_repository_manager.cc b/src/model_repository_manager/model_repository_manager.cc index c5609feb3..96df7b637 100644 --- a/src/model_repository_manager/model_repository_manager.cc +++ b/src/model_repository_manager/model_repository_manager.cc @@ -267,7 +267,7 @@ CreateAgentModelListWithLoadAction( // Return the latest modification time in ns for all files/folders starting // from the path. The modification time will be 0 if there is an error. int64_t -GetModifiedTime(const std::string& path) +GetPathModifiedTime(const std::string& path) { // If there is an error in any step the fall-back default // modification time is 0. This means that in error cases 'path' @@ -306,84 +306,173 @@ GetModifiedTime(const std::string& path) for (const auto& child : contents) { const auto full_path = JoinPath({path, child}); - mtime = std::max(mtime, GetModifiedTime(full_path)); + mtime = std::max(mtime, GetPathModifiedTime(full_path)); } return mtime; } -// Return the latest modification time in ns for '' -// in a model directory path. The time for "config.pbtxt" will be 0 if not -// found at "[model_dir_path]/config.pbtxt" or "[model_dir_path]/configs/ -// .pbtxt" if "--model-config-name" is set. The time for -// "model files" includes the time for 'model_dir_path'. -std::pair -GetDetailedModifiedTime( + +} // namespace + +ModelTimestamp::ModelTimestamp( const std::string& model_dir_path, const std::string& model_config_path) { - // Check if 'model_dir_path' is a directory. + // DLIS-7221: Raise an exception when failed to create timestamp + bool init_success = + ModelDirectoryPathIsValid(model_dir_path) && + ReadModelDirectoryTimestamp(model_dir_path, model_config_path) && + ReadModelDirectoryContentTimestamps(model_dir_path, model_config_path); + if (!init_success) { + // Same as calling the default constructor. All timestamps are presumed to + // be 0 when comparing. + model_timestamps_.clear(); + model_config_content_name_.clear(); + } +} + +bool +ModelTimestamp::ModelDirectoryPathIsValid(const std::string& path) const +{ bool is_dir; - Status status = IsDirectory(model_dir_path, &is_dir); + Status status = IsDirectory(path, &is_dir); if (!status.IsOk()) { - LOG_ERROR << "Failed to determine modification time for '" << model_dir_path + LOG_ERROR << "Failed to determine modification time for '" << path << "': " << status.AsString(); - return std::make_pair(0, 0); + return false; } if (!is_dir) { - LOG_ERROR << "Failed to determine modification time for '" << model_dir_path + LOG_ERROR << "Failed to determine modification time for '" << path << "': Model directory path is not a directory"; - return std::make_pair(0, 0); + return false; } + return true; +} - std::pair mtime(0, 0); // - - // Populate 'model files' time to the model directory time - status = FileModificationTime(model_dir_path, &mtime.second); +bool +ModelTimestamp::ReadModelDirectoryTimestamp( + const std::string& model_dir_path, const std::string& model_config_path) +{ + int64_t model_dir_time = 0; + Status status = FileModificationTime(model_dir_path, &model_dir_time); if (!status.IsOk()) { LOG_ERROR << "Failed to determine modification time for '" << model_dir_path << "': " << status.AsString(); - return std::make_pair(0, 0); + return false; } + model_timestamps_.emplace("", model_dir_time); - // Get files/folders in model_dir_path. - std::set contents; - status = GetDirectoryContents(model_dir_path, &contents); + return true; +} + +bool +ModelTimestamp::ReadModelDirectoryContentTimestamps( + const std::string& model_dir_path, const std::string& model_config_path) +{ + std::set dir_contents; + Status status = GetDirectoryContents(model_dir_path, &dir_contents); if (!status.IsOk()) { LOG_ERROR << "Failed to determine modification time for '" << model_dir_path << "': " << status.AsString(); - return std::make_pair(0, 0); - } - // Get latest modification time for each files/folders, and place it at the - // correct category. - for (const auto& child : contents) { - const auto full_path = JoinPath({model_dir_path, child}); - if (full_path == model_config_path) { - // config.pbtxt or customized config file in configs folder - mtime.first = GetModifiedTime(full_path); - } else { - // model files - mtime.second = std::max(mtime.second, GetModifiedTime(full_path)); + return false; + } + for (const auto& content_name : dir_contents) { + const auto content_path = JoinPath({model_dir_path, content_name}); + bool is_model_config = model_config_path.rfind(content_path, 0) == 0; + if (is_model_config) { + if (!model_config_content_name_.empty()) { + LOG_ERROR << "Failed to determine modification time for '" + << model_dir_path << "': Duplicate model config is detected"; + return false; + } + model_config_content_name_ = content_name; } + model_timestamps_.emplace(content_name, GetPathModifiedTime(content_path)); } - return mtime; + return true; } -// Return true if any file in the 'model_dir_path' has been modified more -// recently than 'last_ns', which represents last modified time for -// ''. Update the 'last_ns' to the most-recent -// modified time. + bool -IsModified( - const std::string& model_dir_path, const std::string& model_config_path, - std::pair* last_ns) +ModelTimestamp::IsModified(const ModelTimestamp& new_timestamp) const { - auto new_ns = GetDetailedModifiedTime(model_dir_path, model_config_path); - bool modified = std::max(new_ns.first, new_ns.second) > - std::max(last_ns->first, last_ns->second); - last_ns->swap(new_ns); - return modified; + int64_t old_modified_time = GetModifiedTime(); + int64_t new_modified_time = new_timestamp.GetModifiedTime(); + return new_modified_time > old_modified_time; } -} // namespace +bool +ModelTimestamp::IsModelVersionModified( + const ModelTimestamp& new_timestamp, const int64_t version) const +{ + int64_t old_modified_time = std::max( + GetModelVersionModifiedTime(version), + GetNonModelConfigNorVersionNorDirModifiedTime()); + int64_t new_modified_time = std::max( + new_timestamp.GetModelVersionModifiedTime(version), + new_timestamp.GetNonModelConfigNorVersionNorDirModifiedTime()); + return new_modified_time > old_modified_time; +} + +int64_t +ModelTimestamp::GetModifiedTime() const +{ + int64_t modified_time = 0; + for (const auto& pair : model_timestamps_) { + const int64_t time = pair.second; + modified_time = std::max(modified_time, time); + } + return modified_time; +} + +int64_t +ModelTimestamp::GetModelVersionModifiedTime(const int64_t version) const +{ + int64_t modified_time = 0; + auto itr = model_timestamps_.find(std::to_string(version)); + if (itr != model_timestamps_.end()) { + modified_time = itr->second; + } + return modified_time; +} + +int64_t +ModelTimestamp::GetNonModelConfigNorVersionNorDirModifiedTime() const +{ + // Get modified time excluding time from model config, model version + // directory(s) and model directory. + int64_t modified_time = 0; + for (const auto& pair : model_timestamps_) { + const std::string& content_name = pair.first; + bool found_non_digit_in_content_name = false; + for (const char& c : content_name) { + if (std::isdigit(c) == 0) { + found_non_digit_in_content_name = true; + break; + } + } + // All model version directory(s) will not 'found_non_digit_in_content_name' + // as they are all digit(s). + // Model directory name will not 'found_non_digit_in_content_name' as it is + // empty. + if (found_non_digit_in_content_name && + content_name != model_config_content_name_) { + const int64_t time = pair.second; + modified_time = std::max(modified_time, time); + } + } + return modified_time; +} + +void +ModelTimestamp::SetModelConfigModifiedTime(const int64_t time_ns) +{ + if (model_config_content_name_.empty()) { + LOG_ERROR << "Failed to set config modification time: " + "model_config_content_name_ is empty"; + return; + } + model_timestamps_[model_config_content_name_] = time_ns; +} ModelRepositoryManager::ModelRepositoryManager( const std::set& repository_paths, const bool autofill, @@ -624,7 +713,7 @@ ModelRepositoryManager::LoadModelByDependency( auto status = model_life_cycle_->AsyncLoad( valid_model->model_id_, itr->second->model_path_, valid_model->model_config_, itr->second->is_config_provided_, - itr->second->mtime_nsec_.second > itr->second->prev_mtime_ns_.second, + itr->second->prev_mtime_ns_, itr->second->mtime_nsec_, itr->second->agent_model_list_, [model_state](Status load_status) { model_state->status_ = load_status; model_state->ready_.set_value(); @@ -1406,7 +1495,7 @@ ModelRepositoryManager::InitializeModelInfo( if (iitr != infos_.end()) { linfo->prev_mtime_ns_ = iitr->second->mtime_nsec_; } else { - linfo->prev_mtime_ns_ = std::make_pair(0, 0); + linfo->prev_mtime_ns_ = ModelTimestamp(); } // Set 'mtime_nsec_' and override 'model_path_' if current path is empty @@ -1435,7 +1524,7 @@ ModelRepositoryManager::InitializeModelInfo( // For file override, set 'mtime_nsec_' to minimum value so that // the next load without override will trigger re-load to undo // the override while the local files may still be unchanged. - linfo->mtime_nsec_ = std::make_pair(0, 0); + linfo->mtime_nsec_ = ModelTimestamp(); linfo->model_path_ = location; linfo->model_config_path_ = JoinPath({location, kModelConfigPbTxt}); linfo->agent_model_list_.reset(new TritonRepoAgentModelList()); @@ -1445,14 +1534,16 @@ ModelRepositoryManager::InitializeModelInfo( GetModelConfigFullPath(linfo->model_path_, model_config_name_); // Model is not loaded. if (iitr == infos_.end()) { - linfo->mtime_nsec_ = GetDetailedModifiedTime( - linfo->model_path_, linfo->model_config_path_); + linfo->mtime_nsec_ = + ModelTimestamp(linfo->model_path_, linfo->model_config_path_); } else { // Check the current timestamps to determine if model actually has been // modified linfo->mtime_nsec_ = linfo->prev_mtime_ns_; - unmodified = !IsModified( - linfo->model_path_, linfo->model_config_path_, &linfo->mtime_nsec_); + ModelTimestamp new_mtime_ns = + ModelTimestamp(linfo->model_path_, linfo->model_config_path_); + unmodified = !linfo->mtime_nsec_.IsModified(new_mtime_ns); + linfo->mtime_nsec_ = new_mtime_ns; } } @@ -1467,9 +1558,9 @@ ModelRepositoryManager::InitializeModelInfo( // the override while the local files may still be unchanged. auto time_since_epoch = std::chrono::system_clock::now().time_since_epoch(); - linfo->mtime_nsec_.first = + linfo->mtime_nsec_.SetModelConfigModifiedTime( std::chrono::duration_cast(time_since_epoch) - .count(); + .count()); unmodified = false; const std::string& override_config = override_parameter->ValueString(); diff --git a/src/model_repository_manager/model_repository_manager.h b/src/model_repository_manager/model_repository_manager.h index ffc0d4c1b..2c382b2dc 100644 --- a/src/model_repository_manager/model_repository_manager.h +++ b/src/model_repository_manager/model_repository_manager.h @@ -49,6 +49,39 @@ enum ActionType { NO_ACTION, LOAD, UNLOAD }; /// Predefined reason strings #define MODEL_READY_REASON_DUPLICATE "model appears in two or more repositories" +// Information about timestamps in model directory. +class ModelTimestamp { + public: + ModelTimestamp() {} + ModelTimestamp( + const std::string& model_dir_path, const std::string& model_config_path); + + bool IsModified(const ModelTimestamp& new_timestamp) const; + bool IsModelVersionModified( + const ModelTimestamp& new_timestamp, const int64_t version) const; + + void SetModelConfigModifiedTime(const int64_t time_ns); + + private: + bool ModelDirectoryPathIsValid(const std::string& path) const; + bool ReadModelDirectoryTimestamp( + const std::string& model_dir_path, const std::string& model_config_path); + bool ReadModelDirectoryContentTimestamps( + const std::string& model_dir_path, const std::string& model_config_path); + + int64_t GetModifiedTime() const; + int64_t GetModelVersionModifiedTime(const int64_t version) const; + int64_t GetNonModelConfigNorVersionNorDirModifiedTime() const; + + // Timestamps of all contents in the model directory, i.e. + // - "": ns timestamp of model config (directory) + // - "1": ns timestamp of model version 1 + // - "": ns timestamp of the model directory + std::unordered_map model_timestamps_; + // If empty, model config is not detected on the model directory. + std::string model_config_content_name_; +}; + /// An object to manage the model repository active in the server. class ModelRepositoryManager { public: @@ -77,23 +110,16 @@ class ModelRepositoryManager { // Information about the model. struct ModelInfo { ModelInfo( - const std::pair& mtime_nsec, - const std::pair& prev_mtime_ns, + const ModelTimestamp& mtime_nsec, const ModelTimestamp& prev_mtime_ns, const std::string& model_path, const std::string& model_config_path) : mtime_nsec_(mtime_nsec), prev_mtime_ns_(prev_mtime_ns), explicitly_load_(true), model_path_(model_path), model_config_path_(model_config_path), is_config_provided_(false) { } - ModelInfo() - : mtime_nsec_(0, 0), prev_mtime_ns_(0, 0), explicitly_load_(true), - is_config_provided_(false) - { - } - // Current last modified time in ns, for '' - std::pair mtime_nsec_; - // Previous last modified time in ns, for '' - std::pair prev_mtime_ns_; + ModelInfo() : explicitly_load_(true), is_config_provided_(false) {} + ModelTimestamp mtime_nsec_; + ModelTimestamp prev_mtime_ns_; bool explicitly_load_; inference::ModelConfig model_config_; std::string model_path_;