diff options
Diffstat (limited to 'spec/lib/gitlab/memory')
| -rw-r--r-- | spec/lib/gitlab/memory/jemalloc_spec.rb | 25 | ||||
| -rw-r--r-- | spec/lib/gitlab/memory/reports/jemalloc_stats_spec.rb | 124 | ||||
| -rw-r--r-- | spec/lib/gitlab/memory/reports_daemon_spec.rb | 136 | ||||
| -rw-r--r-- | spec/lib/gitlab/memory/watchdog_spec.rb | 139 |
4 files changed, 343 insertions, 81 deletions
diff --git a/spec/lib/gitlab/memory/jemalloc_spec.rb b/spec/lib/gitlab/memory/jemalloc_spec.rb index 8847516b52c..482ac6e5802 100644 --- a/spec/lib/gitlab/memory/jemalloc_spec.rb +++ b/spec/lib/gitlab/memory/jemalloc_spec.rb @@ -28,11 +28,12 @@ RSpec.describe Gitlab::Memory::Jemalloc do describe '.dump_stats' do it 'writes stats JSON file' do - described_class.dump_stats(path: outdir, format: format) + file_path = described_class.dump_stats(path: outdir, format: format) file = Dir.entries(outdir).find { |e| e.match(/jemalloc_stats\.#{$$}\.\d+\.json$/) } expect(file).not_to be_nil - expect(File.read(File.join(outdir, file))).to eq(output) + expect(file_path).to eq(File.join(outdir, file)) + expect(File.read(file_path)).to eq(output) end end end @@ -52,12 +53,22 @@ RSpec.describe Gitlab::Memory::Jemalloc do end describe '.dump_stats' do - it 'writes stats text file' do - described_class.dump_stats(path: outdir, format: format) + shared_examples 'writes stats text file' do |filename_label, filename_pattern| + it do + described_class.dump_stats(path: outdir, format: format, filename_label: filename_label) + + file = Dir.entries(outdir).find { |e| e.match(filename_pattern) } + expect(file).not_to be_nil + expect(File.read(File.join(outdir, file))).to eq(output) + end + end - file = Dir.entries(outdir).find { |e| e.match(/jemalloc_stats\.#{$$}\.\d+\.txt$/) } - expect(file).not_to be_nil - expect(File.read(File.join(outdir, file))).to eq(output) + context 'when custom filename label is passed' do + include_examples 'writes stats text file', 'puma_0', /jemalloc_stats\.#{$$}\.puma_0\.\d+\.txt$/ + end + + context 'when custom filename label is not passed' do + include_examples 'writes stats text file', nil, /jemalloc_stats\.#{$$}\.\d+\.txt$/ end end end diff --git a/spec/lib/gitlab/memory/reports/jemalloc_stats_spec.rb b/spec/lib/gitlab/memory/reports/jemalloc_stats_spec.rb new file mode 100644 index 00000000000..53fae48776b --- /dev/null +++ b/spec/lib/gitlab/memory/reports/jemalloc_stats_spec.rb @@ -0,0 +1,124 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Memory::Reports::JemallocStats do + let(:reports_dir) { '/empty-dir' } + let(:jemalloc_stats) { described_class.new(reports_path: reports_dir) } + + describe '.run' do + context 'when :report_jemalloc_stats ops FF is enabled' do + let(:worker_id) { 'puma_1' } + let(:report_name) { 'report.json' } + let(:report_path) { File.join(reports_dir, report_name) } + + before do + allow(Prometheus::PidProvider).to receive(:worker_id).and_return(worker_id) + end + + it 'invokes Jemalloc.dump_stats and returns file path' do + expect(Gitlab::Memory::Jemalloc) + .to receive(:dump_stats).with(path: reports_dir, filename_label: worker_id).and_return(report_path) + + expect(jemalloc_stats.run).to eq(report_path) + end + + describe 'reports cleanup' do + let_it_be(:outdir) { Dir.mktmpdir } + + let(:jemalloc_stats) { described_class.new(reports_path: outdir) } + + before do + stub_env('GITLAB_DIAGNOSTIC_REPORTS_JEMALLOC_MAX_REPORTS_STORED', 3) + allow(Gitlab::Memory::Jemalloc).to receive(:dump_stats) + end + + after do + FileUtils.rm_f(outdir) + end + + context 'when number of reports exceeds `max_reports_stored`' do + let_it_be(:reports) do + now = Time.current + + (1..5).map do |i| + Tempfile.new("jemalloc_stats.#{i}.worker_#{i}.#{Time.current.to_i}.json", outdir).tap do |f| + FileUtils.touch(f, mtime: (now + i.second).to_i) + end + end + end + + after do + reports.each do |f| + f.close + f.unlink + rescue Errno::ENOENT + # Some of the files are already unlinked by the code we test; Ignore + end + end + + it 'keeps only `max_reports_stored` total newest files' do + expect { jemalloc_stats.run } + .to change { Dir.entries(outdir).count { |e| e.match(/jemalloc_stats.*/) } } + .from(5).to(3) + + # Keeps only the newest reports + expect(reports.last(3).all? { |r| File.exist?(r) }).to be true + end + end + + context 'when number of reports does not exceed `max_reports_stored`' do + let_it_be(:reports) do + now = Time.current + + (1..3).map do |i| + Tempfile.new("jemalloc_stats.#{i}.worker_#{i}.#{Time.current.to_i}.json", outdir).tap do |f| + FileUtils.touch(f, mtime: (now + i.second).to_i) + end + end + end + + after do + reports.each do |f| + f.close + f.unlink + end + end + + it 'does not remove any reports' do + expect { jemalloc_stats.run } + .not_to change { Dir.entries(outdir).count { |e| e.match(/jemalloc_stats.*/) } } + end + end + end + end + + context 'when :report_jemalloc_stats ops FF is disabled' do + before do + stub_feature_flags(report_jemalloc_stats: false) + end + + it 'does not run the report and returns nil' do + expect(Gitlab::Memory::Jemalloc).not_to receive(:dump_stats) + + expect(jemalloc_stats.run).to be_nil + end + end + end + + describe '.active?' do + subject(:active) { jemalloc_stats.active? } + + context 'when :report_jemalloc_stats ops FF is enabled' do + it { is_expected.to be true } + end + + context 'when :report_jemalloc_stats ops FF is disabled' do + before do + stub_feature_flags(report_jemalloc_stats: false) + end + + it { is_expected.to be false } + end + end +end diff --git a/spec/lib/gitlab/memory/reports_daemon_spec.rb b/spec/lib/gitlab/memory/reports_daemon_spec.rb new file mode 100644 index 00000000000..c9562470971 --- /dev/null +++ b/spec/lib/gitlab/memory/reports_daemon_spec.rb @@ -0,0 +1,136 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Memory::ReportsDaemon do + let(:daemon) { described_class.new } + + describe '#run_thread' do + let(:report_duration_counter) { instance_double(::Prometheus::Client::Counter) } + let(:file_size) { 1_000_000 } + + before do + allow(Gitlab::Metrics).to receive(:counter).and_return(report_duration_counter) + allow(report_duration_counter).to receive(:increment) + + # make sleep no-op + allow(daemon).to receive(:sleep) {} + + # let alive return 3 times: true, true, false + allow(daemon).to receive(:alive).and_return(true, true, false) + + allow(File).to receive(:size).with(/#{daemon.reports_path}.*\.json/).and_return(file_size) + end + + it 'runs reports' do + expect(daemon.send(:reports)).to all(receive(:run).twice.and_call_original) + + daemon.send(:run_thread) + end + + it 'logs report execution' do + expect(::Prometheus::PidProvider).to receive(:worker_id).at_least(:once).and_return('worker_1') + + expect(Gitlab::AppLogger).to receive(:info).with( + hash_including( + :duration_s, + :cpu_s, + perf_report_size_bytes: file_size, + message: 'finished', + pid: Process.pid, + worker_id: 'worker_1', + perf_report: 'jemalloc_stats' + )).twice + + daemon.send(:run_thread) + end + + context 'when the report object returns invalid file path' do + before do + allow(File).to receive(:size).with(/#{daemon.reports_path}.*\.json/).and_raise(Errno::ENOENT) + end + + it 'logs `0` as `perf_report_size_bytes`' do + expect(Gitlab::AppLogger).to receive(:info).with(hash_including(perf_report_size_bytes: 0)).twice + + daemon.send(:run_thread) + end + end + + it 'sets real time duration gauge' do + expect(report_duration_counter).to receive(:increment).with({ report: 'jemalloc_stats' }, an_instance_of(Float)) + + daemon.send(:run_thread) + end + + it 'allows configure and run multiple reports' do + # rubocop: disable RSpec/VerifiedDoubles + # We test how ReportsDaemon could be extended in the future + # We configure it with new reports classes which are not yet defined so we cannot make this an instance_double. + active_report_1 = double("Active Report 1", active?: true) + active_report_2 = double("Active Report 2", active?: true) + inactive_report = double("Inactive Report", active?: false) + # rubocop: enable RSpec/VerifiedDoubles + + allow(daemon).to receive(:reports).and_return([active_report_1, inactive_report, active_report_2]) + + expect(active_report_1).to receive(:run).and_return('/tmp/report_1.json').twice + expect(active_report_2).to receive(:run).and_return('/tmp/report_2.json').twice + expect(inactive_report).not_to receive(:run) + + daemon.send(:run_thread) + end + + context 'sleep timers logic' do + it 'wakes up every (fixed interval + defined delta), sleeps between reports each cycle' do + stub_env('GITLAB_DIAGNOSTIC_REPORTS_SLEEP_MAX_DELTA_S', 1) # rand(1) == 0, so we will have fixed sleep interval + daemon = described_class.new + allow(daemon).to receive(:alive).and_return(true, true, false) + + expect(daemon).to receive(:sleep).with(described_class::DEFAULT_SLEEP_S).ordered + expect(daemon).to receive(:sleep).with(described_class::DEFAULT_SLEEP_BETWEEN_REPORTS_S).ordered + expect(daemon).to receive(:sleep).with(described_class::DEFAULT_SLEEP_S).ordered + expect(daemon).to receive(:sleep).with(described_class::DEFAULT_SLEEP_BETWEEN_REPORTS_S).ordered + + daemon.send(:run_thread) + end + end + end + + describe '#stop_working' do + it 'changes :alive to false' do + expect { daemon.send(:stop_working) }.to change { daemon.send(:alive) }.from(true).to(false) + end + end + + context 'timer intervals settings' do + context 'when no settings are set in the environment' do + it 'uses defaults' do + daemon = described_class.new + + expect(daemon.sleep_s).to eq(described_class::DEFAULT_SLEEP_S) + expect(daemon.sleep_max_delta_s).to eq(described_class::DEFAULT_SLEEP_MAX_DELTA_S) + expect(daemon.sleep_between_reports_s).to eq(described_class::DEFAULT_SLEEP_BETWEEN_REPORTS_S) + expect(daemon.reports_path).to eq(described_class::DEFAULT_REPORTS_PATH) + end + end + + context 'when settings are passed through the environment' do + before do + stub_env('GITLAB_DIAGNOSTIC_REPORTS_SLEEP_S', 100) + stub_env('GITLAB_DIAGNOSTIC_REPORTS_SLEEP_MAX_DELTA_S', 50) + stub_env('GITLAB_DIAGNOSTIC_REPORTS_SLEEP_BETWEEN_REPORTS_S', 2) + stub_env('GITLAB_DIAGNOSTIC_REPORTS_PATH', '/empty-dir') + end + + it 'uses provided values' do + daemon = described_class.new + + expect(daemon.sleep_s).to eq(100) + expect(daemon.sleep_max_delta_s).to eq(50) + expect(daemon.sleep_between_reports_s).to eq(2) + expect(daemon.reports_path).to eq('/empty-dir') + end + end + end +end diff --git a/spec/lib/gitlab/memory/watchdog_spec.rb b/spec/lib/gitlab/memory/watchdog_spec.rb index 8b82078bcb9..010f6884df3 100644 --- a/spec/lib/gitlab/memory/watchdog_spec.rb +++ b/spec/lib/gitlab/memory/watchdog_spec.rb @@ -14,32 +14,57 @@ RSpec.describe Gitlab::Memory::Watchdog, :aggregate_failures, :prometheus do let(:sleep_time) { 0.1 } let(:max_heap_fragmentation) { 0.2 } + # Tests should set this to control the number of loop iterations in `call`. + let(:watchdog_iterations) { 1 } + subject(:watchdog) do described_class.new(handler: handler, logger: logger, sleep_time_seconds: sleep_time, - max_strikes: max_strikes, max_heap_fragmentation: max_heap_fragmentation) + max_strikes: max_strikes, max_heap_fragmentation: max_heap_fragmentation).tap do |instance| + # We need to defuse `sleep` and stop the internal loop after N iterations. + iterations = 0 + expect(instance).to receive(:sleep) do + instance.stop if (iterations += 1) >= watchdog_iterations + end.at_most(watchdog_iterations) + end + end + + def stub_prometheus_metrics + allow(Gitlab::Metrics).to receive(:gauge) + .with(:gitlab_memwd_heap_frag_limit, anything) + .and_return(heap_frag_limit_gauge) + allow(Gitlab::Metrics).to receive(:counter) + .with(:gitlab_memwd_heap_frag_violations_total, anything, anything) + .and_return(heap_frag_violations_counter) + allow(Gitlab::Metrics).to receive(:counter) + .with(:gitlab_memwd_heap_frag_violations_handled_total, anything, anything) + .and_return(heap_frag_violations_handled_counter) + + allow(heap_frag_limit_gauge).to receive(:set) + allow(heap_frag_violations_counter).to receive(:increment) + allow(heap_frag_violations_handled_counter).to receive(:increment) end before do + stub_prometheus_metrics + allow(handler).to receive(:on_high_heap_fragmentation).and_return(true) allow(logger).to receive(:warn) allow(logger).to receive(:info) allow(Gitlab::Metrics::Memory).to receive(:gc_heap_fragmentation).and_return(fragmentation) - end - after do - watchdog.stop + allow(::Prometheus::PidProvider).to receive(:worker_id).and_return('worker_1') end - context 'when starting up' do + context 'when created' do let(:fragmentation) { 0 } let(:max_strikes) { 0 } it 'sets the heap fragmentation limit gauge' do - allow(Gitlab::Metrics).to receive(:gauge).and_return(heap_frag_limit_gauge) - expect(heap_frag_limit_gauge).to receive(:set).with({}, max_heap_fragmentation) + + watchdog end context 'when no settings are set in the environment' do @@ -76,77 +101,54 @@ RSpec.describe Gitlab::Memory::Watchdog, :aggregate_failures, :prometheus do it 'does not signal the handler' do expect(handler).not_to receive(:on_high_heap_fragmentation) - watchdog.start - - sleep sleep_time * 3 + watchdog.call end end context 'when process exceeds heap fragmentation threshold permanently' do let(:fragmentation) { max_heap_fragmentation + 0.1 } - - before do - allow(Gitlab::Metrics).to receive(:counter) - .with(:gitlab_memwd_heap_frag_violations_total, anything, anything) - .and_return(heap_frag_violations_counter) - allow(Gitlab::Metrics).to receive(:counter) - .with(:gitlab_memwd_heap_frag_violations_handled_total, anything, anything) - .and_return(heap_frag_violations_handled_counter) - allow(heap_frag_violations_counter).to receive(:increment) - allow(heap_frag_violations_handled_counter).to receive(:increment) - end + let(:max_strikes) { 3 } context 'when process has not exceeded allowed number of strikes' do - let(:max_strikes) { 10 } + let(:watchdog_iterations) { max_strikes } it 'does not signal the handler' do expect(handler).not_to receive(:on_high_heap_fragmentation) - watchdog.start - - sleep sleep_time * 3 + watchdog.call end it 'does not log any events' do expect(logger).not_to receive(:warn) - watchdog.start - - sleep sleep_time * 3 + watchdog.call end it 'increments the violations counter' do - expect(heap_frag_violations_counter).to receive(:increment) - - watchdog.start + expect(heap_frag_violations_counter).to receive(:increment).exactly(watchdog_iterations) - sleep sleep_time * 3 + watchdog.call end it 'does not increment violations handled counter' do expect(heap_frag_violations_handled_counter).not_to receive(:increment) - watchdog.start - - sleep sleep_time * 3 + watchdog.call end end context 'when process exceeds the allowed number of strikes' do - let(:max_strikes) { 1 } + let(:watchdog_iterations) { max_strikes + 1 } it 'signals the handler and resets strike counter' do expect(handler).to receive(:on_high_heap_fragmentation).and_return(true) - watchdog.start - - sleep sleep_time * 3 + watchdog.call expect(watchdog.strikes).to eq(0) end it 'logs the event' do - expect(::Prometheus::PidProvider).to receive(:worker_id).at_least(:once).and_return('worker_1') expect(Gitlab::Metrics::System).to receive(:memory_usage_rss).at_least(:once).and_return(1024) expect(logger).to receive(:warn).with({ message: 'heap fragmentation limit exceeded', @@ -161,18 +163,14 @@ RSpec.describe Gitlab::Memory::Watchdog, :aggregate_failures, :prometheus do memwd_rss_bytes: 1024 }) - watchdog.start - - sleep sleep_time * 3 + watchdog.call end it 'increments both the violations and violations handled counters' do - expect(heap_frag_violations_counter).to receive(:increment) + expect(heap_frag_violations_counter).to receive(:increment).exactly(watchdog_iterations) expect(heap_frag_violations_handled_counter).to receive(:increment) - watchdog.start - - sleep sleep_time * 3 + watchdog.call end context 'when enforce_memory_watchdog ops toggle is off' do @@ -186,35 +184,31 @@ RSpec.describe Gitlab::Memory::Watchdog, :aggregate_failures, :prometheus do receive(:on_high_heap_fragmentation).with(fragmentation).and_return(true) ) - watchdog.start - - sleep sleep_time * 3 + watchdog.call end end - end - - context 'when handler result is true' do - let(:max_strikes) { 1 } - it 'considers the event handled and stops itself' do - expect(handler).to receive(:on_high_heap_fragmentation).once.and_return(true) + context 'when handler result is true' do + it 'considers the event handled and stops itself' do + expect(handler).to receive(:on_high_heap_fragmentation).once.and_return(true) + expect(logger).to receive(:info).with(hash_including(message: 'stopped')) - watchdog.start - - sleep sleep_time * 3 + watchdog.call + end end - end - - context 'when handler result is false' do - let(:max_strikes) { 1 } - it 'keeps running' do - # Return true the third time to terminate the daemon. - expect(handler).to receive(:on_high_heap_fragmentation).and_return(false, false, true) + context 'when handler result is false' do + let(:max_strikes) { 0 } # to make sure the handler fires each iteration + let(:watchdog_iterations) { 3 } - watchdog.start + it 'keeps running' do + expect(heap_frag_violations_counter).to receive(:increment).exactly(watchdog_iterations) + expect(heap_frag_violations_handled_counter).to receive(:increment).exactly(watchdog_iterations) + # Return true the third time to terminate the daemon. + expect(handler).to receive(:on_high_heap_fragmentation).and_return(false, false, true) - sleep sleep_time * 4 + watchdog.call + end end end end @@ -222,6 +216,7 @@ RSpec.describe Gitlab::Memory::Watchdog, :aggregate_failures, :prometheus do context 'when process exceeds heap fragmentation threshold temporarily' do let(:fragmentation) { max_heap_fragmentation } let(:max_strikes) { 1 } + let(:watchdog_iterations) { 4 } before do allow(Gitlab::Metrics::Memory).to receive(:gc_heap_fragmentation).and_return( @@ -235,9 +230,7 @@ RSpec.describe Gitlab::Memory::Watchdog, :aggregate_failures, :prometheus do it 'does not signal the handler' do expect(handler).not_to receive(:on_high_heap_fragmentation) - watchdog.start - - sleep sleep_time * 4 + watchdog.call end end @@ -252,9 +245,7 @@ RSpec.describe Gitlab::Memory::Watchdog, :aggregate_failures, :prometheus do it 'does not monitor heap fragmentation' do expect(Gitlab::Metrics::Memory).not_to receive(:gc_heap_fragmentation) - watchdog.start - - sleep sleep_time * 3 + watchdog.call end end end |