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.
動作させるためにいくつかの手順が必要でした。
