Skip to content

Let CurrentSettings read from ApplicationSetting or OrganizationSetting

Rutger Wessels requested to merge 434507-read-organization-settings into master

What does this MR do and why?

As part of Cells project, we are introducing the concept of Organization as a new, high level entity. This entity will contain namespaces, groups and projects.

This also means that a (to be determined) number of Settings will be migrated from ApplicationSetting to OrganizationSetting.

Many settings are now accessed using Gitlab::CurrentSettings class. This class is now configured to only read from ApplicationSetting. This MR will change that: it will read from ApplicationSetting but if a setting does not exists, it will try OrganizationSetting.

If a setting can't be found, it will raise a NoMethodError, just like current behaviour of Gitlab::CurrentSettings

Related Issue #434507 (closed)

Performance

No performance impact (degradation) is expected.

Currently, there are no settings that are read from Organization. We did add restricted_visiblity_levels setting to Organization level but since this setting is also on ApplicationSetting, the setting will still be read from ApplicationSetting.

I did try a simple benchmark: reading 5 settings 100 000 times.

➜  gitlab git:(434507-read-organization-settings) gdk rails runner ~/tmp/bm.rb
       user     system      total        real
  42.615016   1.823093  44.438109 ( 53.759137)
➜  gitlab git:(434507-read-organization-settings) gdk rails runner ~/tmp/bm.rb
       user     system      total        real
  42.253397   1.824903  44.078300 ( 53.197330)
➜  gitlab git:(434507-read-organization-settings) gcm
Switched to branch 'master'
Your branch is up to date with 'origin/master'.
➜  gitlab git:(master) gdk rails runner ~/tmp/bm.rb
       user     system      total        real
  46.772270   2.576193  49.348463 ( 59.670021)
➜  gitlab git:(master) gdk rails runner ~/tmp/bm.rb
       user     system      total        real
  42.303233   2.006176  44.309409 ( 53.431309)

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Rutger Wessels

Merge request reports