Defect #36998

Revert lazy loading of i18n files introduced in Redmine 5.0

Added by Mischa The Evil about 1 month ago. Updated 24 days ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Marius BALTEANU% Done:


Target version:5.0.1
Resolution:Fixed Affected version:5.0.0


Starting with 5.0.0, Redmine subclasses the ::I18n::Backend::LazyLoadable backend (#36728). I haven't looked deeply into the inner workings of this particular backend, but I did notice the following notes/recommendations in the upstream PR:

Most notably, a local test environment.

This backend should only be enabled in test environments!

It's designed for the local test environment [...]

Now I wonder: has the above been considered at all before it was committed? If so: what were the reasons this was nevertheless deemed fit for production use in the Redmine project?

Related issues

Related to Redmine - Feature #36728: Reintroduce lazy loading of i18n files Closed

Associated revisions

Revision 21557
Added by Marius BALTEANU 24 days ago

Merge r21556 to 5.0-stable (#36998).


#1 Updated by Pavel Rosický about 1 month ago

Hi Mischa!
thank you for catching this. Yes, there're potential thread-safety issues. Especially in production environments, everything should be eager-loaded.

by reviewing the PR again, I've found that the feature actually has to be enabled explicitly by lazy_load otherwise, it behaves the same way as SimpleBackend

I propose this patch to fix the issue, what do you think?

diff --git a/config/initializers/30-redmine.rb b/config/initializers/30-redmine.rb
index fc36977f6..887fcc8ee 100644
--- a/config/initializers/30-redmine.rb
+++ b/config/initializers/30-redmine.rb
@@ -4,7 +4,7 @@ require 'redmine/configuration'
 require 'redmine/plugin_loader'

 Rails.application.config.to_prepare do
-  I18n.backend =
+  I18n.backend = Rails.env.development?)
   # Forces I18n to load available locales from the backend
   I18n.config.available_locales = nil

note that the previous change was safe, simply because the feature was disabled in all environments and this change enables it for dev environments only.

#2 Updated by Pavel Rosický about 1 month ago

diff --git a/lib/redmine/i18n.rb b/lib/redmine/i18n.rb
index 13b84512f..c3578f34c 100644
--- a/lib/redmine/i18n.rb
+++ b/lib/redmine/i18n.rb
@@ -158,11 +158,9 @@ module Redmine
     # Custom backend based on I18n::Backend::Simple with the following changes:
     # * available_locales are determined by looking at translation file names
     class Backend < ::I18n::Backend::LazyLoadable
-      module Implementation
-        # Get available locales from the translations filenames
-        def available_locales
-          @available_locales ||= {|path| File.basename(path, '.*')}
-        end
+      # Get available locales from the translations filenames
+      def available_locales
+        @available_locales ||= {|path| File.basename(path, '.*')}

       # Adds custom pluralization rules

#3 Updated by Go MAEDA about 1 month ago

  • Related to Feature #36728: Reintroduce lazy loading of i18n files added

#4 Updated by Holger Just 30 days ago

Reading a bit through the PR and the previous issues here on, I'd rather tend to switch to the ::I18n::Backend::Simple backend again.

Within Redmine, (skipping the) load of the translations during startup doesn't seem to meaningfully affect the overall startup time. On my system, regardless of which backend or RAILS_ENV is used, full startup takes between 4 and 6 seconds with a plain Redmine.

As such, given the possible issues with the LazyLoadable backend, I think it's just not worth it and we should just use the simpler and more deterministic ::I18n::Backend::Simple.

#5 Updated by Pavel Rosický 30 days ago

@holger even with the simple backend I18n does load translations after the first call of I18n.t, so you have to measure the first request, not just the startup time.

with these two patches (and lazy loading enabled), all tests passed, but I understand your concerns about complexity. Thanks for considering it!

#6 Updated by Marius BALTEANU 30 days ago

  • Target version set to 5.0.1

Is it enough to revert r21448 or we should allow lazy loading on dev environment?

#7 Updated by Holger Just 29 days ago

I'd rather just revert r21448 unless someone can show a real tangible upside of this diversion between environments (which I don't see right now).

#8 Updated by Pavel Rosický 29 days ago

these diversions between environments are common - auto_load in development vs eager_load in production

but if you think the risk is too high, please revert r21448 thank you.

#9 Updated by Marius BALTEANU 25 days ago

  • Resolution set to Fixed

Reverted r21448 in r21556.

#10 Updated by Marius BALTEANU 25 days ago

  • Status changed from New to Resolved
  • Assignee set to Marius BALTEANU

#11 Updated by Marius BALTEANU 24 days ago

  • Subject changed from ::I18n::Backend::LazyLoadable is not deemed fit for production use to Revert lazy loading of i18n files introduced in Redmine 5.0
  • Status changed from Resolved to Closed

Reverted in r21556 and merged to 5.0-stable branch. Thank you all for your reviews!

Pavel, I agree with Holger, we can discuss this again for non production environments if we can obtain a meaningfully improvement.

#12 Updated by Pavel Rosický 24 days ago

note that Redmine used to have its own backend based on the same idea for years (originally introduced by JPLang in 2012 :) ), so this isn't something new

but I agree it's better to keep it as simple as possible. Thank you all for your comments!

Also available in: Atom PDF