add max size to resolved host cache

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<SteadyClock>()),
       resolver(std::make_shared<SimpleHostResolver>()),
+      max_cache_size(10000),
       max_result_age(60.0),
       max_resolve_time(1.0),
       num_threads(4)
@@ -58,11 +59,23 @@ AsyncResolver::LoggingHostResolver::ip_address(const vespalib::string &host_name
 //-----------------------------------------------------------------------------
 
 bool
+AsyncResolver::CachingHostResolver::should_evict_oldest_entry(const std::lock_guard<std::mutex> &, 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<std::mutex> 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<LoggingHostResolver>(params.clock, std::move(params.resolver), params.max_resolve_time);
-    auto cacher = std::make_shared<CachingHostResolver>(std::move(params.clock), std::move(logger), params.max_result_age);
+    auto cacher = std::make_shared<CachingHostResolver>(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<Itr>  _queue;
 
+        bool should_evict_oldest_entry(const std::lock_guard<std::mutex> &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;
     };