Skip to content

Address possible memoization issue in Gitlab::Database class

What does this MR do and why?

This MR fixes a potential memoization issue in Gitlab::Database class. The method Gitlab::Database.database_base_models_using_load_balancing was using the same variable as Gitlab::Database.database_base_models_with_gitlab_shared for memoization.

This is not an issue right now because the implementation of the two methods is exactly the same. But it could become an issue in the future.

Reading this comment, it seems like this behaviour is not intentional.

I did not write specs for this because it is hard to test, since the methods are currently having the same implementation.

How to set up and validate locally

  • Change the implementation of Gitlab::Database.database_base_models_using_load_balancing so it returns something different than Gitlab::Database.database_base_models_with_gitlab_shared
  • Open rails console and check that the output of Gitlab::Database.database_base_models_using_load_balancing is different now

Since the implementation of both methods is exactly the same, we need to 'break' the method in order to verify it is using its own memoization variable. For Example by removing ci from the returned hash:

    def self.database_base_models_using_load_balancing
      @database_base_models_with_gitlab_shared ||= {
        main: ::ActiveRecord::Base,
        # ci: ::Ci::ApplicationRecord.connection_class? ? ::Ci::ApplicationRecord : nil
      }.compact.with_indifferent_access.freeze
    end

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #387042 (closed)

Edited by Rutger Wessels

Merge request reports

Loading