Skip to content

Commit

Permalink
Revert "Re-organize ModelTimestamp() and throw exception"
Browse files Browse the repository at this point in the history 
This reverts commit 41e57e5.
  • Loading branch information
@kthui
kthui committed Aug 17, 2024
1 parent 41e57e5 commit 413e510
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 94 deletions.
140 changes: 58 additions & 82 deletions src/model_repository_manager/model_repository_manager.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -276,19 +276,19 @@ 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
// time of the directory as baseline in case of file deletion
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;
Expand All @@ -299,9 +299,9 @@ GetPathModifiedTime(const std::string& path)
std::set<std::string> 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) {
Expand All @@ -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<std::string> 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;
}
Expand All @@ -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();
Expand All @@ -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),
Expand All @@ -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_) {
Expand All @@ -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));
Expand All @@ -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.
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
}

Expand All @@ -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<std::chrono::nanoseconds>(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();
Expand Down
18 changes: 6 additions & 12 deletions src/model_repository_manager/model_repository_manager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
// - "<model_config_content_name_>": ns timestamp of model config (directory)
Expand Down

0 comments on commit 413e510

Please sign in to comment.