Rails 6.0.0 a été publié il y a 25 jours, je pense donc qu’il est temps de mettre à jour Discourse J’ai dû effectuer quelques étapes pour que cela fonctionne.
Corriger les spécifications cassées
Ajouter une méthode vide trigger_transactional_callbacks? dans lib/mini_sql_multisite_connection.rb
UrlHelper charge par défaut ActionView::Helpers::UrlHelper au lieu de lib/UrlHelper. Je l’ai résolu en ajoutant :: devant, mais que pensez-vous de changer le nom de cette classe ?
Dans Rails 5.2.3, MigrationContext acceptait un argument, tandis que dans 6.0.0, un schéma supplémentaire est requis.
Corriger l’avertissement concernant la constante déjà initialisée (TRADITIONAL_ESCAPED_CHAR, RFC_5987_ESCAPED_CHAR). Avant de les supprimer, j’ai confirmé que les valeurs dans ActionPack étaient identiques.
Utiliser le chargeur automatique classique comme première étape avant Zeitwerk.
Corriger les migrations sur Rails 6.0.0 - la version la plus récente de Rails ne permet plus l’erreur présente dans les anciennes migrations (vous ne pouvez pas définir une colonne déjà définie ‘integer’).
J’ai effectué des tests de fumée pour vérifier que Discourse fonctionne comme prévu. De plus, j’ai exécuté des tests de performance pour m’assurer qu’il n’y a pas de régression (j’ai utilisé 500 itérations par défaut).
Test
Rails 5.2.3
Rails 6.0.0
Pourcentage
categories-50
27
24
88,89%
categories-75
31
26
83,87%
categories-90
36
37
102,78%
categories-99
52
50
96,15%
home-50
27
26
96,30%
home-75
30
28
93,33%
home-90
39
38
97,44%
home-99
53
55
103,77%
topic-50
35
27
77,14%
topic-75
36
29
80,56%
topic-90
37
39
105,41%
topic-99
56
50
89,29%
categories_admin-50
47
47
100,00%
categories_admin-75
54
59
109,26%
categories_admin-90
64
66
103,13%
categories_admin-99
132
116
87,88%
home_admin-50
47
46
97,87%
home_admin-75
51
56
109,80%
home_admin-90
63
64
101,59%
home_admin-99
110
97
88,18%
topic_admin-50
50
49
98,00%
topic_admin-75
58
59
101,72%
topic_admin-90
65
67
103,08%
topic_admin-99
113
86
76,11%
load_rails
2593
2618
100,96%
rss_kb
318800
287332
90,13%
pss_kb
306913
275378
89,73%
Moyenne
89,31%
Je vais créer une pull request avec tous les changements mentionnés ci-dessus. Veuillez me faire savoir si vous souhaitez que j’ajuste quelque chose ou que j’effectue des tests supplémentaires pour garantir que tout fonctionne comme prévu.
Can you put this in a markdown table with percent change? Cursory look is showing not much has changes which is awesome.
Plugin wise, we have a rake task that installs all official plugins, can you run that and ensure the plugin specs pass on Rails 6? (rake plugin:spec should do the trick)
I updated the original post to display table. Thank you for pointing me plugins specs. I see that 2 specs failed on Travis, I will take a look and fix them.
I run benchmark 3 times and this time, results are less spectacular. My flow is to type ruby script/bench.rb in correct branch master or rails6, hit enter and don’t touch keyboard to not affect results
topic-50
RSS
5.2.3
50
322852
5.2.3
50
309684
5.2.3
50
346376
Average
50
326304
6.0.0
49
328844
6.0.0
49
321824
6.0.0
49
283584
Average
49
311417
I also connected my development server to performance database to ensure that topic page looks correct. Screenshot below looks fine to me
I would like to get your opinion about one fix.
I downloaded all plugins, however one new spec is failing comparing to master (./plugins/discourse-data-explorer/spec/controllers/queries_controller_spec.rb:32)
1) DataExplorer::QueryController when disabled denies every request
Failure/Error: render 'default/empty'
ActionView::Template::Error:
wrong number of arguments (given 2, expected 1)
I can monkey patch rspec-rails with a new file in lib/freedom_patched/rspec-rails.rb but wanted to ensure that this is the best approach.
I think that this is the last change which is blocking Rails 6 from being merged.
In addition, I noticed that this spec is broken, however, it is broken on master as well, I can try to fix it (./plugins/discourse-calendar/spec/jobs/update_holiday_usernames_spec.rb:14)
Finally, I updated rspec-rails to version 4.0.0.beta2 and it works fine on my local machine. Travis got some problems, however I see same problems in other pull requests so I don’t think this is correlated to upgrade of rspec-rails.
My only concern (and something I would like to get to the bottom of) it appears we get “runaway” threads for a few seconds regularly on the web workers.
# frozen_string_literal: true
class Thread
attr_accessor :origin
end
class ThreadDetective
def self.test_thread
Thread.new { sleep 1 }
end
def self.start(max_threads)
@thread ||= Thread.new do
self.new.monitor(max_threads)
end
@trace = TracePoint.new(:thread_begin) do |tp|
Thread.current.origin = Thread.current.inspect
end
@trace.enable
end
def self.stop
@thread&.kill
@thread = nil
@trace&.disable
@trace.stop
end
def monitor(max_threads)
STDERR.puts "Monitoring threads in #{Process.pid}"
while true
threads = Thread.list
if threads.length > max_threads
str = +("-" * 60)
str << "#{threads.length} found in Process #{Process.pid}!\n"
threads.each do |thread|
str << "\n"
if thread.origin
str << thread.origin
else
str << thread.inspect
end
str << "\n"
end
str << ("-" * 60)
STDERR.puts str
end
sleep 1
end
end
end
I then wired in unicorn after_fork a require of this class and ran ThreadDetective.start(14)
The class diligently watched every time a thread was created using a TracePoint and placed a tiny frame on the thread called origin to help me track where it came from. Once a large number of threads were observed it dumped stuff to STDERR. This can be tracked in /var/www/discourse/logs/unicorn.stderr.log
Once I knew that 100 threads were all coming from a single spot, it was very easy to isolate the root cause.
thanks for your efforts on bringing Rails 6 to Discourse! May I humbly ask when this is expected to land in Discourse? Or is it already in 2.4.0.beta? I am just asking about the possibility whether this might break any plugin people have installed on their instances.
J’ai dû effectuer quelques étapes pour que cela fonctionne.
