From 28292d275994145c8e295470dee6627bfd84c936 Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Tue, 18 Feb 2020 15:11:30 +1100 Subject: [PATCH] PERF: avoid shelling to get hostname aggressively Previously we had many places in the app that called `hostname` to get hostname of a server. This commit replaces the pattern in 2 ways 1. We cache the result in `Discourse.os_hostname` so it is only ever called once 2. We prefer to use Socket.gethostname which avoids making a shell command This improves performance as we are not spawning hostname processes throughout the app lifetime --- app/jobs/base.rb | 2 +- app/models/optimized_image.rb | 2 +- config/initializers/100-sidekiq.rb | 2 +- lib/demon/sidekiq.rb | 2 +- lib/discourse.rb | 18 ++++++++++++++++++ lib/discourse_logstash_logger.rb | 2 +- .../schema_migration_details.rb | 2 +- lib/message_bus_diags.rb | 2 +- script/mwrap_sidekiq | 2 +- 9 files changed, 26 insertions(+), 8 deletions(-) diff --git a/app/jobs/base.rb b/app/jobs/base.rb index b43bcb5729c..b4e1f2e77b0 100644 --- a/app/jobs/base.rb +++ b/app/jobs/base.rb @@ -40,7 +40,7 @@ module Jobs self.class.mutex.synchronize do @data = {} - @data["hostname"] = `hostname`.strip # Hostname + @data["hostname"] = Discourse.os_hostname @data["pid"] = Process.pid # Pid @data["database"] = db # DB name - multisite db name it ran on @data["job_id"] = jid # Job unique ID diff --git a/app/models/optimized_image.rb b/app/models/optimized_image.rb index 1f839483b51..3f5614a0840 100644 --- a/app/models/optimized_image.rb +++ b/app/models/optimized_image.rb @@ -9,7 +9,7 @@ class OptimizedImage < ActiveRecord::Base URL_REGEX ||= /(\/optimized\/\dX[\/\.\w]*\/([a-zA-Z0-9]+)[\.\w]*)/ def self.lock(upload_id, width, height) - @hostname ||= `hostname`.strip rescue "unknown" + @hostname ||= Discourse.os_hostname # note, the extra lock here ensures we only optimize one image per machine on webs # this can very easily lead to runaway CPU so slowing it down is beneficial and it is hijacked # diff --git a/config/initializers/100-sidekiq.rb b/config/initializers/100-sidekiq.rb index f3241e35625..892380708dd 100644 --- a/config/initializers/100-sidekiq.rb +++ b/config/initializers/100-sidekiq.rb @@ -69,7 +69,7 @@ if Sidekiq.server? Rails.application.config.after_initialize do scheduler_hostname = ENV["UNICORN_SCHEDULER_HOSTNAME"] - if !scheduler_hostname || scheduler_hostname.split(',').include?(`hostname`.strip) + if !scheduler_hostname || scheduler_hostname.split(',').include?(Discourse.os_hostname) MiniScheduler.start(workers: GlobalSetting.mini_scheduler_workers) end end diff --git a/lib/demon/sidekiq.rb b/lib/demon/sidekiq.rb index 6a8dc48d522..e5ff82f702c 100644 --- a/lib/demon/sidekiq.rb +++ b/lib/demon/sidekiq.rb @@ -39,7 +39,7 @@ class Demon::Sidekiq < ::Demon::Base [['critical', 8], ['default', 4], ['low', 2], ['ultra_low', 1]].each do |queue_name, weight| custom_queue_hostname = ENV["UNICORN_SIDEKIQ_#{queue_name.upcase}_QUEUE_HOSTNAME"] - if !custom_queue_hostname || custom_queue_hostname.split(',').include?(`hostname`.strip) + if !custom_queue_hostname || custom_queue_hostname.split(',').include?(Discourse.os_hostname) options << "-q" options << "#{queue_name},#{weight}" end diff --git a/lib/discourse.rb b/lib/discourse.rb index 62ba6133164..57e8778e90b 100644 --- a/lib/discourse.rb +++ b/lib/discourse.rb @@ -312,6 +312,24 @@ module Discourse end end + # hostname of the server, operating system level + # called os_hostname so we do no confuse it with current_hostname + def self.os_hostname + @os_hostname ||= + begin + require 'socket' + Socket.gethostname + rescue => e + warn_exception(e, message: 'Socket.gethostname is not working') + begin + `hostname`.strip + rescue => e + warn_exception(e, message: 'hostname command is not working') + 'unknown_host' + end + end + end + # Get the current base URL for the current site def self.current_hostname SiteSetting.force_hostname.presence || RailsMultisite::ConnectionManagement.current_hostname diff --git a/lib/discourse_logstash_logger.rb b/lib/discourse_logstash_logger.rb index 301354e991f..ba02b31935c 100644 --- a/lib/discourse_logstash_logger.rb +++ b/lib/discourse_logstash_logger.rb @@ -8,7 +8,7 @@ class DiscourseLogstashLogger uri: uri, sync: true, customize_event: ->(event) { - event['hostname'] = `hostname`.chomp + event['hostname'] = Discourse.os_hostname event['severity_name'] = event['severity'] event['severity'] = Object.const_get("Logger::Severity::#{event['severity']}") event['type'] = type diff --git a/lib/freedom_patches/schema_migration_details.rb b/lib/freedom_patches/schema_migration_details.rb index 0f60581a698..5b06a81c26a 100644 --- a/lib/freedom_patches/schema_migration_details.rb +++ b/lib/freedom_patches/schema_migration_details.rb @@ -31,7 +31,7 @@ module FreedomPatches ) SQL - hostname = `hostname` rescue "" + hostname = Discourse.os_hostname sql = ActiveRecord::Base.public_send(:sanitize_sql_array, [sql, { version: version || "", duration: (time.real * 1000).to_i, diff --git a/lib/message_bus_diags.rb b/lib/message_bus_diags.rb index d56badfe620..16bde3a9a14 100644 --- a/lib/message_bus_diags.rb +++ b/lib/message_bus_diags.rb @@ -5,7 +5,7 @@ class MessageBusDiags @host_info = {} def self.my_id - @my_id ||= "#{`hostname`}-#{Process.pid}" + @my_id ||= "#{Discourse.os_hostname}-#{Process.pid}" end def self.seen_host(name) diff --git a/script/mwrap_sidekiq b/script/mwrap_sidekiq index b3a73a59743..8c2a73a5b7b 100755 --- a/script/mwrap_sidekiq +++ b/script/mwrap_sidekiq @@ -128,7 +128,7 @@ begin [['critical', 8], ['default', 4], ['low', 2], ['ultra_low', 1]].each do |queue_name, weight| custom_queue_hostname = ENV["UNICORN_SIDEKIQ_#{queue_name.upcase}_QUEUE_HOSTNAME"] - if !custom_queue_hostname || custom_queue_hostname.split(',').include?(`hostname`.strip) + if !custom_queue_hostname || custom_queue_hostname.split(',').include?(Discourse.os_hostname) options << "-q" options << "#{queue_name},#{weight}" end