Geo: ee/lib/gitlab/geo.rb does not use memoization

I was looking at the incident today: gitlab-com/gl-infra/production#936 (comment 187998572) and @andrewn noticed there was awfully a lot of load redis load coming from Geo:

65066 get cache:gitlab:geo:current_node:12.0.0-pre:5.1.7
65024 get cache:gitlab:geo:node_enabled:12.0.0-pre:5.1.7
 711 get cache:gitlab:geo:secondary_nodes:12.0.0-pre:5.1.7
 147 setex cache:gitlab:geo:current_node:12.0.0-pre:5.1.7
 109 setex cache:gitlab:geo:node_enabled:12.0.0-pre:5.1.7
  52 del cache:gitlab:geo:current_node:12.0.0-pre:5.1.7
  38 del cache:gitlab:geo:node_enabled:12.0.0-pre:5.1.7
   1 setex cache:gitlab:geo:secondary_nodes:12.0.0-pre:5.1.7

The cache:gitlab:geo:current_node and cache:gitlab:geo:node_enabled are located in ee/lib/gitlab/geo.rb:

module Gitlab
  module Geo
    #... 

    def self.current_node
      self.cache_value(:current_node, as: GeoNode) { GeoNode.current_node }
    end

    def self.primary_node
      self.cache_value(:primary_node, as: GeoNode) { GeoNode.primary_node }
    end

    def self.secondary_nodes
      self.cache_value(:secondary_nodes, as: GeoNode) { GeoNode.secondary_nodes }
    end

    def self.connected?
      Gitlab::Database.postgresql? && GeoNode.connected? && GeoNode.table_exists?
    end

    def self.enabled?
      self.cache_value(:node_enabled) { GeoNode.exists? }
    end

    def self.primary?
      self.enabled? && self.current_node&.primary?
    end

    def self.secondary?
      self.enabled? && self.current_node&.secondary?
    end

Looking at that code, I feel we're missing simple memoization here. I expect calls to Gitlab::Geo.primary? and Gitlab::Geo.secondary? trigger this redis load, but I'm not sure on the exact source yet.

Edited Jul 03, 2019 by Toon Claes
Assignee Loading
Time tracking Loading