From 2fd870335a91c4bb71c8e3016d79e1820043cb7a Mon Sep 17 00:00:00 2001 From: Haavard Date: Tue, 11 Jul 2017 10:09:02 +0000 Subject: add max size to resolved host cache --- .../tests/net/async_resolver/async_resolver_test.cpp | 20 +++++++++++++++++++- vespalib/src/vespa/vespalib/net/async_resolver.cpp | 20 +++++++++++++++++--- vespalib/src/vespa/vespalib/net/async_resolver.h | 5 ++++- 3 files changed, 40 insertions(+), 5 deletions(-) diff --git a/vespalib/src/tests/net/async_resolver/async_resolver_test.cpp b/vespalib/src/tests/net/async_resolver/async_resolver_test.cpp index 806a7f34cbe..434cb8d6a69 100644 --- a/vespalib/src/tests/net/async_resolver/async_resolver_test.cpp +++ b/vespalib/src/tests/net/async_resolver/async_resolver_test.cpp @@ -80,10 +80,13 @@ struct ResolveFixture { size_t get_cnt(const vespalib::string &host) { return host_resolver->get_cnt(host); } size_t get_total_cnt() { return host_resolver->get_total_cnt(); } void set_now(double s) { clock->set_now(MyClock::seconds(s)); } - ResolveFixture() : clock(new MyClock()), host_resolver(new MyHostResolver()), async_resolver() { + ResolveFixture(size_t max_cache_size = 10000) + : clock(new MyClock()), host_resolver(new MyHostResolver()), async_resolver() + { AsyncResolver::Params params; params.clock = clock; params.resolver = host_resolver; + params.max_cache_size = max_cache_size; params.max_result_age = AsyncResolver::seconds(60.0); params.max_resolve_time = AsyncResolver::seconds(1.0); params.num_threads = 4; @@ -115,6 +118,7 @@ TEST("require that async resolver internal duration type is appropriate") { TEST("require that default async resolver is tuned as expected") { AsyncResolver::Params params; + EXPECT_EQUAL(params.max_cache_size, 10000u); EXPECT_EQUAL(params.max_result_age.count(), 60.0); EXPECT_EQUAL(params.max_resolve_time.count(), 1.0); EXPECT_EQUAL(params.num_threads, 4u); @@ -202,6 +206,20 @@ TEST_F("require that cached results expire at the right time", ResolveFixture()) EXPECT_EQUAL(f1.get_total_cnt(), 2u); } +TEST_F("require that max cache size is honored", ResolveFixture(3)) { + EXPECT_EQUAL(f1.resolve("tcp/a:123"), "tcp/127.0.1.1:123"); + EXPECT_EQUAL(f1.resolve("tcp/b:123"), "tcp/127.0.2.1:123"); + EXPECT_EQUAL(f1.resolve("tcp/c:123"), "tcp/127.0.3.1:123"); + EXPECT_EQUAL(f1.resolve("tcp/d:123"), "tcp/127.0.4.1:123"); + EXPECT_EQUAL(f1.get_total_cnt(), 4u); + EXPECT_EQUAL(f1.resolve("tcp/b:123"), "tcp/127.0.2.1:123"); + EXPECT_EQUAL(f1.get_total_cnt(), 4u); + EXPECT_EQUAL(f1.resolve("tcp/a:123"), "tcp/127.0.1.1:123"); + EXPECT_EQUAL(f1.get_total_cnt(), 5u); + EXPECT_EQUAL(f1.resolve("tcp/b:123"), "tcp/127.0.2.1:123"); + EXPECT_EQUAL(f1.get_total_cnt(), 6u); +} + TEST_F("require that missing ip address gives invalid address", ResolveFixture()) { f1.set_ip_addr("localhost", ""); EXPECT_EQUAL(f1.resolve("tcp/localhost:123"), "invalid"); diff --git a/vespalib/src/vespa/vespalib/net/async_resolver.cpp b/vespalib/src/vespa/vespalib/net/async_resolver.cpp index babfcd3764d..bc918434077 100644 --- a/vespalib/src/vespa/vespalib/net/async_resolver.cpp +++ b/vespalib/src/vespa/vespalib/net/async_resolver.cpp @@ -31,6 +31,7 @@ AsyncResolver::SimpleHostResolver::ip_address(const vespalib::string &host_name) AsyncResolver::Params::Params() : clock(std::make_shared()), resolver(std::make_shared()), + max_cache_size(10000), max_result_age(60.0), max_resolve_time(1.0), num_threads(4) @@ -57,12 +58,24 @@ AsyncResolver::LoggingHostResolver::ip_address(const vespalib::string &host_name //----------------------------------------------------------------------------- +bool +AsyncResolver::CachingHostResolver::should_evict_oldest_entry(const std::lock_guard &, time_point now) +{ + if (_queue.empty()) { + return false; + } + if (_queue.size() > _max_cache_size) { + return true; + } + return (_queue.front()->second.end_time <= now); +} + bool AsyncResolver::CachingHostResolver::lookup(const vespalib::string &host_name, vespalib::string &ip_address) { auto now = _clock->now(); std::lock_guard guard(_lock); - while (!_queue.empty() && (_queue.front()->second.end_time <= now)) { + while (should_evict_oldest_entry(guard, now)) { _map.erase(_queue.front()); _queue.pop(); } @@ -87,9 +100,10 @@ AsyncResolver::CachingHostResolver::store(const vespalib::string &host_name, con assert(_map.size() == _queue.size()); } -AsyncResolver::CachingHostResolver::CachingHostResolver(Clock::SP clock, HostResolver::SP resolver, seconds max_result_age) +AsyncResolver::CachingHostResolver::CachingHostResolver(Clock::SP clock, HostResolver::SP resolver, size_t max_cache_size, seconds max_result_age) : _clock(std::move(clock)), _resolver(std::move(resolver)), + _max_cache_size(max_cache_size), _max_result_age(max_result_age), _lock(), _map(), @@ -151,7 +165,7 @@ AsyncResolver::SP AsyncResolver::create(Params params) { auto logger = std::make_shared(params.clock, std::move(params.resolver), params.max_resolve_time); - auto cacher = std::make_shared(std::move(params.clock), std::move(logger), params.max_result_age); + auto cacher = std::make_shared(std::move(params.clock), std::move(logger), params.max_cache_size, params.max_result_age); return SP(new AsyncResolver(std::move(cacher), params.num_threads)); } diff --git a/vespalib/src/vespa/vespalib/net/async_resolver.h b/vespalib/src/vespa/vespalib/net/async_resolver.h index 8323881ee7e..c80fb7d938d 100644 --- a/vespalib/src/vespa/vespalib/net/async_resolver.h +++ b/vespalib/src/vespa/vespalib/net/async_resolver.h @@ -60,6 +60,7 @@ public: struct Params { Clock::SP clock; HostResolver::SP resolver; + size_t max_cache_size; seconds max_result_age; seconds max_resolve_time; size_t num_threads; @@ -91,17 +92,19 @@ private: using Itr = Map::iterator; Clock::SP _clock; HostResolver::SP _resolver; + size_t _max_cache_size; seconds _max_result_age; std::mutex _lock; Map _map; ArrayQueue _queue; + bool should_evict_oldest_entry(const std::lock_guard &guard, time_point now); bool lookup(const vespalib::string &host_name, vespalib::string &ip_address); void resolve(const vespalib::string &host_name, vespalib::string &ip_address); void store(const vespalib::string &host_name, const vespalib::string &ip_address); public: - CachingHostResolver(Clock::SP clock, HostResolver::SP resolver, seconds max_result_age); + CachingHostResolver(Clock::SP clock, HostResolver::SP resolver, size_t max_cache_size, seconds max_result_age); vespalib::string ip_address(const vespalib::string &host_name) override; }; -- cgit v1.2.3