From 413e510f38c9d466ca168e29557e8d979a973459 Mon Sep 17 00:00:00 2001 From: kthui <18255193+kthui@users.noreply.github.com> Date: Fri, 16 Aug 2024 18:38:02 -0700 Subject: [PATCH] Revert "Re-organize ModelTimestamp() and throw exception" This reverts commit 41e57e55cf55c3f0496fd95735e06e4764cf5659. --- .../model_repository_manager.cc | 140 ++++++++---------- .../model_repository_manager.h | 18 +-- 2 files changed, 64 insertions(+), 94 deletions(-) diff --git a/src/model_repository_manager/model_repository_manager.cc b/src/model_repository_manager/model_repository_manager.cc index 610826fae..a7ccc6093 100644 --- a/src/model_repository_manager/model_repository_manager.cc +++ b/src/model_repository_manager/model_repository_manager.cc @@ -276,9 +276,9 @@ GetPathModifiedTime(const std::string& path) bool path_is_dir; Status status = IsDirectory(path, &path_is_dir); if (!status.IsOk()) { - throw std::runtime_error( - std::string("Failed to determine modification time for '") + path + - "': " + status.AsString()); + LOG_ERROR << "Failed to determine modification time for '" << path + << "': " << status.AsString(); + return 0; } // If 'path' is a file return its mtime. Otherwise, using the modification @@ -286,9 +286,9 @@ GetPathModifiedTime(const std::string& path) int64_t mtime = 0; status = FileModificationTime(path, &mtime); if (!status.IsOk()) { - throw std::runtime_error( - std::string("Failed to determine modification time for '") + path + - "': " + status.AsString()); + LOG_ERROR << "Failed to determine modification time for '" << path + << "': " << status.AsString(); + return 0; } if (!path_is_dir) { return mtime; @@ -299,9 +299,9 @@ GetPathModifiedTime(const std::string& path) std::set contents; status = GetDirectoryContents(path, &contents); if (!status.IsOk()) { - throw std::runtime_error( - std::string("Failed to determine modification time for '") + path + - "': " + status.AsString()); + LOG_ERROR << "Failed to determine modification time for '" << path + << "': " << status.AsString(); + return 0; } for (const auto& child : contents) { @@ -317,60 +317,49 @@ GetPathModifiedTime(const std::string& path) ModelTimestamp::ModelTimestamp( const std::string& model_dir_path, const std::string& model_config_path) { - AssertPathIsDirectory(model_dir_path); - PopulateModelDirectoryTime(model_dir_path); - PopulateModelDirectoryContentsTime(model_dir_path, model_config_path); -} - -void -ModelTimestamp::AssertPathIsDirectory(const std::string& path) const -{ + // Check if 'model_dir_path' is a directory. bool is_dir; - Status status = IsDirectory(path, &is_dir); + Status status = IsDirectory(model_dir_path, &is_dir); if (!status.IsOk()) { - throw std::runtime_error( - std::string("Failed to determine modification time for '") + path + - "': " + status.AsString()); + LOG_ERROR << "Failed to determine modification time for '" << model_dir_path + << "': " << status.AsString(); + return; } if (!is_dir) { - throw std::runtime_error( - std::string("Failed to determine modification time for '") + path + - "': Path is not a directory"); + LOG_ERROR << "Failed to determine modification time for '" << model_dir_path + << "': Model directory path is not a directory"; + return; } -} -void -ModelTimestamp::PopulateModelDirectoryTime(const std::string& dir_path) -{ - int64_t time_ns = 0; - Status status = FileModificationTime(dir_path, &time_ns); + // Populate time for 'model_dir_path'. + int64_t model_dir_time = 0; + status = FileModificationTime(model_dir_path, &model_dir_time); if (!status.IsOk()) { - throw std::runtime_error( - std::string("Failed to determine modification time for '") + dir_path + - "': " + status.AsString()); + LOG_ERROR << "Failed to determine modification time for '" << model_dir_path + << "': " << status.AsString(); + return; } - model_timestamps_.emplace("", time_ns); -} + model_timestamps_.emplace("", model_dir_time); -void -ModelTimestamp::PopulateModelDirectoryContentsTime( - const std::string& dir_path, const std::string& config_path) -{ + // Populate time for all immediate files/folders in 'model_dir_path'. std::set dir_contents; - Status status = GetDirectoryContents(dir_path, &dir_contents); + status = GetDirectoryContents(model_dir_path, &dir_contents); if (!status.IsOk()) { - throw std::runtime_error( - std::string("Failed to determine modification time for '") + dir_path + - "': " + status.AsString()); + LOG_ERROR << "Failed to determine modification time for '" << model_dir_path + << "': " << status.AsString(); + model_timestamps_.clear(); + return; } for (const auto& content_name : dir_contents) { - const auto content_path = JoinPath({dir_path, content_name}); - bool is_model_config = config_path.rfind(content_path, 0) == 0; + 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()) { - throw std::runtime_error( - std::string("Failed to determine modification time for '") + - dir_path + "': Duplicate model config is detected"); + LOG_ERROR << "Failed to determine modification time for '" + << model_dir_path << "': Duplicate model config is detected"; + model_timestamps_.clear(); + model_config_content_name_.clear(); + return; } model_config_content_name_ = content_name; } @@ -379,7 +368,7 @@ ModelTimestamp::PopulateModelDirectoryContentsTime( } bool -ModelTimestamp::IsModified(const ModelTimestamp& new_timestamp) const noexcept +ModelTimestamp::IsModified(const ModelTimestamp& new_timestamp) const { int64_t old_modified_time = GetModifiedTime(); int64_t new_modified_time = new_timestamp.GetModifiedTime(); @@ -388,7 +377,7 @@ ModelTimestamp::IsModified(const ModelTimestamp& new_timestamp) const noexcept bool ModelTimestamp::IsModelVersionModified( - const ModelTimestamp& new_timestamp, const int64_t version) const noexcept + const ModelTimestamp& new_timestamp, const int64_t version) const { int64_t old_modified_time = std::max( GetModelVersionModifiedTime(version), @@ -400,7 +389,7 @@ ModelTimestamp::IsModelVersionModified( } int64_t -ModelTimestamp::GetModifiedTime() const noexcept +ModelTimestamp::GetModifiedTime() const { int64_t modified_time = 0; for (const auto& pair : model_timestamps_) { @@ -411,8 +400,7 @@ ModelTimestamp::GetModifiedTime() const noexcept } int64_t -ModelTimestamp::GetModelVersionModifiedTime( - const int64_t version) const noexcept +ModelTimestamp::GetModelVersionModifiedTime(const int64_t version) const { int64_t modified_time = 0; auto itr = model_timestamps_.find(std::to_string(version)); @@ -423,7 +411,7 @@ ModelTimestamp::GetModelVersionModifiedTime( } int64_t -ModelTimestamp::GetNonModelConfigNorVersionNorDirModifiedTime() const noexcept +ModelTimestamp::GetNonModelConfigNorVersionNorDirModifiedTime() const { // Get modified time excluding time from model config, model version // directory(s) and model directory. @@ -454,9 +442,9 @@ void ModelTimestamp::SetModelConfigModifiedTime(const int64_t time_ns) { if (model_config_content_name_.empty()) { - throw std::runtime_error( - "Failed to set model configuration modification time: " - "model_config_content_name_ is empty"); + LOG_ERROR << "Failed to set config modification time: " + "model_config_content_name_ is empty"; + return; } model_timestamps_[model_config_content_name_] = time_ns; } @@ -1520,22 +1508,17 @@ ModelRepositoryManager::InitializeModelInfo( linfo->model_config_path_ = GetModelConfigFullPath(linfo->model_path_, model_config_name_); // Model is not loaded. - try { // ModelTimestamp(...) may throw - if (iitr == infos_.end()) { - 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_; - 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; - } - } - catch (const std::runtime_error& e) { - return Status(Status::Code::INTERNAL, e.what()); + if (iitr == infos_.end()) { + 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_; + 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; } } @@ -1550,16 +1533,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(); - int64_t time_since_epoch_ns = + linfo->mtime_nsec_.SetModelConfigModifiedTime( std::chrono::duration_cast(time_since_epoch) - .count(); - try { - linfo->mtime_nsec_.SetModelConfigModifiedTime(time_since_epoch_ns); - } - catch (const std::runtime_error& e) { - return Status(Status::Code::INTERNAL, e.what()); - } - + .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 f03773b96..0cb78ed84 100644 --- a/src/model_repository_manager/model_repository_manager.h +++ b/src/model_repository_manager/model_repository_manager.h @@ -52,26 +52,20 @@ enum ActionType { NO_ACTION, LOAD, UNLOAD }; // Information about timestamps in model directory. class ModelTimestamp { public: - ModelTimestamp() noexcept {} + ModelTimestamp() {} ModelTimestamp( const std::string& model_dir_path, const std::string& model_config_path); - bool IsModified(const ModelTimestamp& new_timestamp) const noexcept; + bool IsModified(const ModelTimestamp& new_timestamp) const; bool IsModelVersionModified( - const ModelTimestamp& new_timestamp, - const int64_t version) const noexcept; + const ModelTimestamp& new_timestamp, const int64_t version) const; void SetModelConfigModifiedTime(const int64_t time_ns); private: - void AssertPathIsDirectory(const std::string& path) const; - void PopulateModelDirectoryTime(const std::string& dir_path); - void PopulateModelDirectoryContentsTime( - const std::string& dir_path, const std::string& config_path); - - int64_t GetModifiedTime() const noexcept; - int64_t GetModelVersionModifiedTime(const int64_t version) const noexcept; - int64_t GetNonModelConfigNorVersionNorDirModifiedTime() const noexcept; + 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)