From fefa2390eef8c358413bfdb802467098fe7b20d5 Mon Sep 17 00:00:00 2001 From: Supriya Garg Date: Wed, 4 Apr 2018 12:17:28 -0400 Subject: [PATCH 1/3] Add a flag for the limit of the request size by count. --- src/configuration.cc | 6 ++++++ src/configuration.h | 5 +++++ test/configuration_unittest.cc | 1 + 3 files changed, 12 insertions(+) diff --git a/src/configuration.cc b/src/configuration.cc index 041cbbf5..2d41cdd1 100644 --- a/src/configuration.cc +++ b/src/configuration.cc @@ -51,6 +51,7 @@ constexpr const char kMetadataIngestionDefaultEndpointFormat[] = "/resourceMetadata:batchUpdate"; constexpr const int kMetadataIngestionDefaultRequestSizeLimitBytes = 8*1024*1024; +constexpr const int kMetadataIngestionDefaultRequestSizeLimitCount = 1000; constexpr const char kMetadataIngestionDefaultRawContentVersion[] = "0.1"; constexpr const int kInstanceUpdaterDefaultIntervalSeconds = 60*60; constexpr const char kDefaultInstanceResourceType[] = @@ -95,6 +96,8 @@ Configuration::Configuration() kMetadataIngestionDefaultEndpointFormat), metadata_ingestion_request_size_limit_bytes_( kMetadataIngestionDefaultRequestSizeLimitBytes), + metadata_ingestion_request_size_limit_count_( + kMetadataIngestionDefaultRequestSizeLimitCount), metadata_ingestion_raw_content_version_( kMetadataIngestionDefaultRawContentVersion), instance_updater_interval_seconds_( @@ -233,6 +236,9 @@ void Configuration::ParseConfiguration(std::istream& input) { metadata_ingestion_request_size_limit_bytes_ = config["MetadataIngestionRequestSizeLimitBytes"].as( metadata_ingestion_request_size_limit_bytes_); + metadata_ingestion_request_size_limit_count_ = + config["MetadataIngestionRequestSizeLimitCount"].as( + metadata_ingestion_request_size_limit_count_); metadata_ingestion_raw_content_version_ = config["MetadataIngestionRawContentVersion"].as( metadata_ingestion_raw_content_version_); diff --git a/src/configuration.h b/src/configuration.h index 4b4cb98c..b8b97087 100644 --- a/src/configuration.h +++ b/src/configuration.h @@ -77,6 +77,10 @@ class Configuration { std::lock_guard lock(mutex_); return metadata_ingestion_request_size_limit_bytes_; } + int MetadataIngestionRequestSizeLimitCount() const { + std::lock_guard lock(mutex_); + return metadata_ingestion_request_size_limit_count_; + } const std::string& MetadataIngestionRawContentVersion() const { std::lock_guard lock(mutex_); return metadata_ingestion_raw_content_version_; @@ -183,6 +187,7 @@ class Configuration { std::string metadata_reporter_user_agent_; std::string metadata_ingestion_endpoint_format_; int metadata_ingestion_request_size_limit_bytes_; + int metadata_ingestion_request_size_limit_count_; std::string metadata_ingestion_raw_content_version_; int instance_updater_interval_seconds_; std::string instance_resource_type_; diff --git a/test/configuration_unittest.cc b/test/configuration_unittest.cc index 4e0cf574..bc707277 100644 --- a/test/configuration_unittest.cc +++ b/test/configuration_unittest.cc @@ -18,6 +18,7 @@ void VerifyDefaultConfig(const Configuration& config) { "v1beta2/projects/{{project_id}}/resourceMetadata:batchUpdate", config.MetadataIngestionEndpointFormat()); EXPECT_EQ(8*1024*1024, config.MetadataIngestionRequestSizeLimitBytes()); + EXPECT_EQ(1000, config.MetadataIngestionRequestSizeLimitCount()); EXPECT_EQ("0.1", config.MetadataIngestionRawContentVersion()); EXPECT_EQ(60*60, config.InstanceUpdaterIntervalSeconds()); EXPECT_EQ("", config.InstanceResourceType()); From 4f8335093d8868e984cb8bef89e28dd9e033e97b Mon Sep 17 00:00:00 2001 From: Supriya Garg Date: Wed, 4 Apr 2018 12:26:55 -0400 Subject: [PATCH 2/3] Update SendMetadata to split the request on hitting the count limit --- src/reporter.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/reporter.cc b/src/reporter.cc index 767a39b6..b1a744c6 100644 --- a/src/reporter.cc +++ b/src/reporter.cc @@ -139,6 +139,7 @@ void MetadataReporter::SendMetadata( const int empty_size = empty_request->ToString().size(); const int limit_bytes = config_.MetadataIngestionRequestSizeLimitBytes(); + const int limit_count = config_.MetadataIngestionRequestSizeLimitCount(); int total_size = empty_size; std::vector entries; @@ -165,7 +166,7 @@ void MetadataReporter::SendMetadata( << "B, dropping; entry " << *metadata_entry; continue; } - if (total_size + size > limit_bytes) { + if (entries.size() > limit_count || total_size + size > limit_bytes) { SendMetadataRequest(std::move(entries), endpoint, auth_header, user_agent, config_.VerboseLogging()); entries.clear(); From 8d66b62b859f31e4f140d32fd3ec3daf2708ae7e Mon Sep 17 00:00:00 2001 From: Supriya Garg Date: Wed, 4 Apr 2018 14:41:40 -0400 Subject: [PATCH 3/3] Address comments --- src/reporter.cc | 2 +- test/configuration_unittest.cc | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/reporter.cc b/src/reporter.cc index b1a744c6..a0072d69 100644 --- a/src/reporter.cc +++ b/src/reporter.cc @@ -166,7 +166,7 @@ void MetadataReporter::SendMetadata( << "B, dropping; entry " << *metadata_entry; continue; } - if (entries.size() > limit_count || total_size + size > limit_bytes) { + if (entries.size() == limit_count || total_size + size > limit_bytes) { SendMetadataRequest(std::move(entries), endpoint, auth_header, user_agent, config_.VerboseLogging()); entries.clear(); diff --git a/test/configuration_unittest.cc b/test/configuration_unittest.cc index bc707277..f2730964 100644 --- a/test/configuration_unittest.cc +++ b/test/configuration_unittest.cc @@ -57,12 +57,14 @@ TEST(ConfigurationTest, PopulatedConfig) { "MetadataReporterPurgeDeleted: true\n" "MetadataReporterUserAgent: \"foobar/foobaz\"\n" "HealthCheckFile: /a/b/c\n" + "MetadataIngestionRequestSizeLimitCount: 500\n" )); EXPECT_EQ("TestProjectId", config.ProjectId()); EXPECT_EQ(13, config.MetadataApiNumThreads()); EXPECT_EQ(true, config.MetadataReporterPurgeDeleted()); EXPECT_EQ("foobar/foobaz", config.MetadataReporterUserAgent()); EXPECT_EQ("/a/b/c", config.HealthCheckFile()); + EXPECT_EQ(500, config.MetadataIngestionRequestSizeLimitCount()); } TEST(ConfigurationTest, CommentSkipped) {