From 32b1f34910e430907e4385f20027ac3e895d4ab5 Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Wed, 31 Oct 2018 12:38:57 +1100
Subject: [PATCH] PERF: avoid DNS lookups when getting IP info

Also cleans up interface in DiscourseIpInfo
grew cache to 2000 entries
---
 app/controllers/admin/users_controller.rb     |  2 +-
 .../concerns/user_auth_tokens_mixin.rb        |  2 +-
 lib/discourse_ip_info.rb                      | 25 +++++++++++--------
 3 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index 776568e4c51..d24231fbffa 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@@ -436,7 +436,7 @@ class Admin::UsersController < Admin::AdminController
   def ip_info
     params.require(:ip)
 
-    render json: DiscourseIpInfo.get(params[:ip])
+    render json: DiscourseIpInfo.get(params[:ip], resolve_hostname: true)
   end
 
   def sync_sso
diff --git a/app/serializers/concerns/user_auth_tokens_mixin.rb b/app/serializers/concerns/user_auth_tokens_mixin.rb
index 95315c4ed7a..c0aa2fcb52b 100644
--- a/app/serializers/concerns/user_auth_tokens_mixin.rb
+++ b/app/serializers/concerns/user_auth_tokens_mixin.rb
@@ -20,7 +20,7 @@ module UserAuthTokensMixin
   end
 
   def location
-    ipinfo = DiscourseIpInfo.get(client_ip, I18n.locale)
+    ipinfo = DiscourseIpInfo.get(client_ip, locale: I18n.locale)
 
     location = [ipinfo[:city], ipinfo[:region], ipinfo[:country]].reject { |x| x.blank? }.join(", ")
     return I18n.t('staff_action_logs.unknown') if location.blank?
diff --git a/lib/discourse_ip_info.rb b/lib/discourse_ip_info.rb
index e01c4457ec4..facc0b115d6 100644
--- a/lib/discourse_ip_info.rb
+++ b/lib/discourse_ip_info.rb
@@ -11,7 +11,7 @@ class DiscourseIpInfo
   def open_db(path)
     @loc_mmdb = mmdb_load(File.join(path, 'GeoLite2-City.mmdb'))
     @asn_mmdb = mmdb_load(File.join(path, 'GeoLite2-ASN.mmdb'))
-    @cache = LruRedux::ThreadSafeCache.new(1000)
+    @cache = LruRedux::ThreadSafeCache.new(2000)
   end
 
   def mmdb_load(filepath)
@@ -24,7 +24,7 @@ class DiscourseIpInfo
     end
   end
 
-  def lookup(ip, locale = :en)
+  def lookup(ip, locale: :en, resolve_hostname: false)
     ret = {}
 
     if @loc_mmdb
@@ -57,27 +57,32 @@ class DiscourseIpInfo
       end
     end
 
-    begin
-      result = Resolv::DNS.new.getname(ip)
-      ret[:hostname] = result&.to_s
-    rescue Resolv::ResolvError
+    # this can block for quite a while
+    # only use it explicitly when needed
+    if resolve_hostname
+      begin
+        result = Resolv::DNS.new.getname(ip)
+        ret[:hostname] = result&.to_s
+      rescue Resolv::ResolvError
+      end
     end
 
     ret
   end
 
-  def get(ip, locale = :en)
+  def get(ip, locale: :en, resolve_hostname: false)
     ip = ip.to_s
     locale = locale.to_s.sub('_', '-')
 
-    @cache["#{ip}-#{locale}"] ||= lookup(ip, locale)
+    @cache["#{ip}-#{locale}-#{resolve_hostname}"] ||=
+      lookup(ip, locale: locale, resolve_hostname: resolve_hostname)
   end
 
   def self.open_db(path)
     instance.open_db(path)
   end
 
-  def self.get(ip, locale = :en)
-    instance.get(ip, locale)
+  def self.get(ip, locale: :en, resolve_hostname: false)
+    instance.get(ip, locale: locale, resolve_hostname: resolve_hostname)
   end
 end