O Rails 6.0.0 foi lançado há 25 dias, então acho que é hora de atualizar o Discourse Houve algumas etapas que precisei realizar para fazer isso funcionar.
Corrigir testes quebrados
Adicionar método vazio trigger_transactional_callbacks? em lib/mini_sql_multisite_connection.rb
UrlHelper está carregando por padrão ActionView::Helpers::UrlHelper em vez de lib/UrlHelper. Resolvi isso adicionando :: na frente, no entanto, o que vocês acham sobre mudar o nome dessa classe?
No Rails 5.2.3, o MigrationContext aceitava um argumento; no 6.0.0, um esquema adicional é necessário
Corrigir aviso de constante já inicializada (TRADITIONAL_ESCAPED_CHAR, RFC_5987_ESCAPED_CHAR). Antes de removê-los, confirmei que os valores em ActionPack são os mesmos
Usar o autoloader clássico como primeira etapa antes do Zeitwerk
Corrigir migrações no Rails 6.0.0 - o Rails mais recente não permite erro de migração antiga (você não pode definir uma coluna já definida como ‘integer’)
Realizei testes de fumaça para confirmar que o Discourse funciona conforme o esperado. Além disso, executei testes de desempenho para garantir que não haja regressão (usei as 500 iterações padrão).
Teste
Rails 5.2.3
Rails 6.0.0
Porcentagem
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%
Média
89,31%
Vou criar um pull request com todas as alterações mencionadas acima. Por favor, me avise se quiser que eu ajuste algo ou realize testes adicionais para garantir que tudo funcione conforme o esperado.
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.
Houve algumas etapas que precisei realizar para fazer isso funcionar.
