From aabf03b39964f3b36990a0144d11d5ed1b91b0ce Mon Sep 17 00:00:00 2001 From: Igor Peshansky Date: Thu, 22 Mar 2018 20:00:21 -0400 Subject: [PATCH 1/2] Invert the test in resource lookup. --- src/api_server.cc | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/api_server.cc b/src/api_server.cc index f3f2f2d6..1a74e54a 100644 --- a/src/api_server.cc +++ b/src/api_server.cc @@ -44,7 +44,14 @@ void MetadataApiServer::Handler::operator()(const HttpServer::request& request, std::string id = request.destination.substr(kPrefix.size()); std::lock_guard lock(store_.resource_mu_); const auto result = store_.resource_map_.find(id); - if (result == store_.resource_map_.end()) { + if (result != store_.resource_map_.end()) { + const MonitoredResource& resource = result->second; + if (config_.VerboseLogging()) { + LOG(INFO) << "Found resource for " << id << ": " << resource; + } + conn->set_status(HttpServer::connection::ok); + conn->write(resource.ToJSON()->ToString()); + } else { // TODO: This could be considered log spam. // As we add more resource mappings, these will become less and less // frequent, and could be promoted to ERROR. @@ -52,13 +59,6 @@ void MetadataApiServer::Handler::operator()(const HttpServer::request& request, LOG(WARNING) << "No matching resource for " << id; } conn->set_status(HttpServer::connection::not_found); - } else { - const MonitoredResource& resource = result->second; - if (config_.VerboseLogging()) { - LOG(INFO) << "Found resource for " << id << ": " << resource; - } - conn->set_status(HttpServer::connection::ok); - conn->write(resource.ToJSON()->ToString()); } } } From 068d921927052506336ba24fa1eab5c33e366bb2 Mon Sep 17 00:00:00 2001 From: Igor Peshansky Date: Thu, 22 Mar 2018 20:02:06 -0400 Subject: [PATCH 2/2] Add MetadataStore::LookupResource. --- src/api_server.cc | 10 ++++------ src/store.cc | 6 ++++++ src/store.h | 7 ++++++- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/api_server.cc b/src/api_server.cc index 1a74e54a..c673854e 100644 --- a/src/api_server.cc +++ b/src/api_server.cc @@ -41,17 +41,15 @@ void MetadataApiServer::Handler::operator()(const HttpServer::request& request, << " body: " << request.body; } if (request.method == "GET" && request.destination.find(kPrefix) == 0) { - std::string id = request.destination.substr(kPrefix.size()); - std::lock_guard lock(store_.resource_mu_); - const auto result = store_.resource_map_.find(id); - if (result != store_.resource_map_.end()) { - const MonitoredResource& resource = result->second; + const std::string id = request.destination.substr(kPrefix.size()); + try { + const MonitoredResource& resource = store_.LookupResource(id); if (config_.VerboseLogging()) { LOG(INFO) << "Found resource for " << id << ": " << resource; } conn->set_status(HttpServer::connection::ok); conn->write(resource.ToJSON()->ToString()); - } else { + } catch (const std::out_of_range& e) { // TODO: This could be considered log spam. // As we add more resource mappings, these will become less and less // frequent, and could be promoted to ERROR. diff --git a/src/store.cc b/src/store.cc index 32a36dd7..ca412e62 100644 --- a/src/store.cc +++ b/src/store.cc @@ -29,6 +29,12 @@ MetadataStore::Metadata MetadataStore::Metadata::IGNORED() { MetadataStore::MetadataStore(const MetadataAgentConfiguration& config) : config_(config) {} +const MonitoredResource& MetadataStore::LookupResource( + const std::string& resource_id) const throw(std::out_of_range) { + std::lock_guard lock(resource_mu_); + return resource_map_.at(resource_id); +} + void MetadataStore::UpdateResource(const std::vector& resource_ids, const MonitoredResource& resource) { std::lock_guard lock(resource_mu_); diff --git a/src/store.h b/src/store.h index 211adace..a84c7080 100644 --- a/src/store.h +++ b/src/store.h @@ -20,6 +20,7 @@ #include #include +#include #include #include "json.h" @@ -75,6 +76,11 @@ class MetadataStore { MetadataStore(const MetadataAgentConfiguration& config); + // Looks up the local resource map entry for a given resource id. + // Throws an exception if the resource is not found. + const MonitoredResource& LookupResource(const std::string& resource_id) const + throw(std::out_of_range); + // Updates the local resource map entry for a given resource. // Each local id in `resource_ids` is effectively an alias for `resource`. // Adds a resource mapping from each of the `resource_ids` to the `resource`. @@ -87,7 +93,6 @@ class MetadataStore { Metadata&& entry); private: - friend class MetadataApiServer; friend class MetadataReporter; std::map GetMetadataMap() const;