From d7ebf6dcf61a5cf6578cb1927d1f6e56da42aee6 Mon Sep 17 00:00:00 2001 From: Livne Kestin Date: Fri, 2 Dec 2016 13:02:15 +0200 Subject: [PATCH 1/2] added cleaner_one action, style changes, performace improment --- lib/resque_cleaner.rb | 46 +++++++--- lib/resque_cleaner/server.rb | 47 ++++++++--- lib/resque_cleaner/server/public/cleaner.css | 10 +++ lib/resque_cleaner/server/views/_paginate.erb | 28 +++---- lib/resque_cleaner/server/views/_stats.erb | 5 +- .../server/views/cleaner_exec.erb | 2 +- .../server/views/cleaner_list.erb | 7 +- .../server/views/cleaner_one.erb | 83 +++++++++++++++++++ 8 files changed, 184 insertions(+), 44 deletions(-) create mode 100644 lib/resque_cleaner/server/views/cleaner_one.erb diff --git a/lib/resque_cleaner.rb b/lib/resque_cleaner.rb index 4a8148b..87cd6ff 100644 --- a/lib/resque_cleaner.rb +++ b/lib/resque_cleaner.rb @@ -74,6 +74,26 @@ def stats_by_exception(&block) stats end + # Stats by exception & class & date. + def get_all_stats(&block) + jobs, stats = select(&block), { exception: {}, klass: {}, date: {} } + jobs.each do |job| + date = job["failed_at"][0,10] + stats[:date][date] ||= 0 + stats[:date][date] += 1 + + exception = job["exception"] + stats[:exception][exception] ||= 0 + stats[:exception][exception] += 1 + + klass = job["payload"] && job["payload"]["class"] ? job["payload"]["class"] : "UNKNOWN" + stats[:klass][klass] ||= 0 + stats[:klass][klass] += 1 + end + + stats + end + # Print stats def print_stats(stats) log too_many_message if @limiter.on? @@ -85,8 +105,7 @@ def print_stats(stats) # Returns every jobs for which block evaluates to true. def select(&block) - jobs = @limiter.jobs - block_given? ? @limiter.jobs.select(&block) : jobs + block_given? ? @limiter.jobs.select(&block) : @limiter.jobs end alias :failure_jobs :select @@ -104,7 +123,7 @@ def clear(&block) if !block_given? || block.call(job) index = @limiter.start_index + i - cleared # fetches again since you can't ensure that it is always true: - # a == endode(decode(a)) + # a == encode(decode(a)) value = redis.lindex(:failed, index) redis.lrem(:failed, 1, value) cleared += 1 @@ -153,7 +172,7 @@ def clear_stale c end - # Exntends job(Hash instance) with some helper methods. + # Extends job(Hash instance) with some helper methods. module FailedJobEx # Returns true if the job has been already retried. Otherwise returns # false. @@ -213,7 +232,7 @@ def default_maximum=(v) end end - attr_accessor :maximum + attr_accessor :maximum, :start_offset, :count_failed def initialize(cleaner) @cleaner = cleaner @maximum = @@default_maximum @@ -240,14 +259,16 @@ def count def jobs if @locked @jobs + elsif start_offset && count_failed + all(start_offset, count_failed) else - all( - count, count) + all(-count, count) end end # Wraps Resque's all and returns always array. def all(index=0,count=1) - jobs = @cleaner.failure.all( index, count) + jobs = @cleaner.failure.all(index, count) jobs = [] unless jobs jobs = [jobs] unless jobs.is_a?(Array) jobs.each{|j| j.extend FailedJobEx} @@ -259,7 +280,7 @@ def start_index if @locked @start_index else - on? ? @cleaner.failure.count-@maximum : 0 + on? ? @cleaner.failure.count - @maximum : 0 end end @@ -270,12 +291,12 @@ def lock unless @locked total_count = @cleaner.failure.count - if total_count>@maximum - @start_index = total_count-@maximum - @jobs = all( @start_index, @maximum) + if total_count > @maximum + @start_index = total_count - @maximum + @jobs = all(@start_index, @maximum) else @start_index = 0 - @jobs = all( 0, total_count) + @jobs = all(0, total_count) end end @@ -304,4 +325,3 @@ def too_many_message end require 'resque_cleaner/server' - diff --git a/lib/resque_cleaner/server.rb b/lib/resque_cleaner/server.rb index 4db4e81..7b44e41 100644 --- a/lib/resque_cleaner/server.rb +++ b/lib/resque_cleaner/server.rb @@ -19,7 +19,7 @@ def initialize(jobs, url, page=1, page_size=20) @jobs = jobs @url = url @page = (!page || page < 1) ? 1 : page - @page_size = 20 + @page_size = page_size end def first_index @@ -28,11 +28,11 @@ def first_index def last_index last = first_index + @page_size - 1 - last > @jobs.size-1 ? @jobs.size-1 : last + last > @jobs.size - 1 ? @jobs.size - 1 : last end def paginated_jobs - @jobs[first_index,@page_size] + @jobs[first_index, @page_size] end def first_page? @@ -58,7 +58,7 @@ def total_size end def max_page - ((total_size-1) / @page_size) + 1 + ((total_size - 1) / @page_size) + 1 end end @@ -151,17 +151,36 @@ def text_filter(id, name, value) block = filter_block - @failed = cleaner.select(&block).reverse + selected_jobs = cleaner.select(&block) + @failed = selected_jobs.reverse + @count = selected_jobs.size @paginate = Paginate.new(@failed, @list_url, params[:p].to_i) - @klasses = cleaner.stats_by_class.keys - @exceptions = cleaner.stats_by_exception.keys - @count = cleaner.select(&block).size + all_stats = cleaner.get_all_stats + @klasses = all_stats[:klass].keys + @exceptions = all_stats[:exception].keys erb File.read(ResqueCleaner::Server.erb_path('cleaner_list.erb')) end + get "/cleaner_one" do + load_library + load_cleaner_filter + set_failed_list_indexes + + @total = cleaner.failure.count + + block = filter_block + + selected_jobs = cleaner.select(&block) + @failed = selected_jobs.reverse + + @count = selected_jobs.size + + erb File.read(ResqueCleaner::Server.erb_path('cleaner_one.erb')) + end + post "/cleaner_exec" do load_library load_cleaner_filter @@ -244,7 +263,7 @@ def build_urls end def filter_block - block = lambda{|j| + lambda{|j| (!@from || j.after?(hours_ago(@from))) && (!@to || j.before?(hours_ago(@to))) && (!@klass || j.klass?(@klass)) && @@ -254,6 +273,15 @@ def filter_block } end + def set_failed_list_indexes + failed_start_index = params[:fsi].present? ? params[:fsi].to_i : nil + count_failed = params[:cf].present? ? params[:cf].to_i : nil + return unless failed_start_index && count_failed + + cleaner.limiter.start_offset = failed_start_index + cleaner.limiter.count_failed = count_failed + end + def hours_ago(h) Time.now - h.to_i*60*60 end @@ -264,4 +292,3 @@ def hours_ago(h) Resque::Server.class_eval do include ResqueCleaner::Server end - diff --git a/lib/resque_cleaner/server/public/cleaner.css b/lib/resque_cleaner/server/public/cleaner.css index 8358930..e32dac5 100644 --- a/lib/resque_cleaner/server/public/cleaner.css +++ b/lib/resque_cleaner/server/public/cleaner.css @@ -60,5 +60,15 @@ #main .cleaner div.dump{ float: right } +#main .cleaner .start-indexes { + float: right; + padding: 7px; + background-color: #eee; + border-radius: 5px; + margin: 5px; + border: 1px solid #ccc; + text-decoration: none; +} + diff --git a/lib/resque_cleaner/server/views/_paginate.erb b/lib/resque_cleaner/server/views/_paginate.erb index 83f82fc..6589b30 100644 --- a/lib/resque_cleaner/server/views/_paginate.erb +++ b/lib/resque_cleaner/server/views/_paginate.erb @@ -2,53 +2,53 @@
- Dump + Dump
diff --git a/lib/resque_cleaner/server/views/_stats.erb b/lib/resque_cleaner/server/views/_stats.erb index 05bafb7..b317ee5 100644 --- a/lib/resque_cleaner/server/views/_stats.erb +++ b/lib/resque_cleaner/server/views/_stats.erb @@ -8,10 +8,11 @@ In last 3 days In last 7 days - <% @stats[type.to_sym].each do |field,count| %> + <% @stats[type.to_sym].each do |field, count| %> <% filter = "#{q}=#{URI.encode(field)}" %> - <%= field %> + <% label = type == 'klass' ? "#{field}" : field %> + <%= label %> <%= count[:total] %> diff --git a/lib/resque_cleaner/server/views/cleaner_exec.erb b/lib/resque_cleaner/server/views/cleaner_exec.erb index 3695c68..124777c 100644 --- a/lib/resque_cleaner/server/views/cleaner_exec.erb +++ b/lib/resque_cleaner/server/views/cleaner_exec.erb @@ -3,6 +3,6 @@

Processed <%= @count %> jobs.

- Back to List + Back to List

diff --git a/lib/resque_cleaner/server/views/cleaner_list.erb b/lib/resque_cleaner/server/views/cleaner_list.erb index 8067a62..51caf2f 100644 --- a/lib/resque_cleaner/server/views/cleaner_list.erb +++ b/lib/resque_cleaner/server/views/cleaner_list.erb @@ -59,7 +59,6 @@ <% end %> - <% start = 0 %> <% failed = @paginate.paginated_jobs%> <% index = 0 %> <% date_format = "%Y/%m/%d %T %z" %> @@ -67,7 +66,7 @@ <% if @paginate.max_page > 0 %> <%= erb File.read(ResqueCleaner::Server.erb_path("_paginate.erb")) %> <%= erb File.read(ResqueCleaner::Server.erb_path("_paginate.erb")) %> <% else %> @@ -174,6 +173,6 @@ $('#exec a').removeClass('disabled'); $('.failed input').removeAttr('disabled'); } - }; + } diff --git a/lib/resque_cleaner/server/views/cleaner_one.erb b/lib/resque_cleaner/server/views/cleaner_one.erb new file mode 100644 index 0000000..3b57f3c --- /dev/null +++ b/lib/resque_cleaner/server/views/cleaner_one.erb @@ -0,0 +1,83 @@ + + +
+
+

Failed Jobs

+
+ +
+ + <% (0..@total).each do |index| %> + <% next unless index % 10 == 0 %> + <%= index %> + <% end %> + +
+ + <% index = 0 %> + <% date_format = "%Y/%m/%d %T %z" %> + + <% if @failed.size > 0 %> + + <% else %> + No match found + <% end %> +
+ From e75edb4e4d5e8a1764c326bf3ffffbe68babc280 Mon Sep 17 00:00:00 2001 From: Livne Kestin Date: Mon, 5 Dec 2016 10:55:04 +0200 Subject: [PATCH 2/2] add some tests --- lib/resque_cleaner/server.rb | 4 ++-- test/resque_cleaner_test.rb | 15 ++++++++++++++- test/resque_web_test.rb | 21 +++++++++++++++++++++ 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/lib/resque_cleaner/server.rb b/lib/resque_cleaner/server.rb index 7b44e41..406f9c4 100644 --- a/lib/resque_cleaner/server.rb +++ b/lib/resque_cleaner/server.rb @@ -274,8 +274,8 @@ def filter_block end def set_failed_list_indexes - failed_start_index = params[:fsi].present? ? params[:fsi].to_i : nil - count_failed = params[:cf].present? ? params[:cf].to_i : nil + failed_start_index = params[:fsi].nil? ? nil : params[:fsi].to_i + count_failed = params[:cf].nil? ? nil : params[:cf].to_i return unless failed_start_index && count_failed cleaner.limiter.start_offset = failed_start_index diff --git a/test/resque_cleaner_test.rb b/test/resque_cleaner_test.rb index 605f7be..6fd6a2a 100644 --- a/test/resque_cleaner_test.rb +++ b/test/resque_cleaner_test.rb @@ -155,7 +155,7 @@ # with block ret = @cleaner.stats_by_date{|j| j['payload']['args']==['Jason']} assert_equal 2, ret['2009/03/13'] - assert_equal nil, ret['2009/11/13'] + assert_nil ret['2009/11/13'] assert_equal 11, ret['2010/08/13'] end @@ -165,6 +165,19 @@ assert_equal 7, ret['BadJobWithSyntaxError'] end + it "#get all stats returns stats grouped by class, exeption and date" do + ret = @cleaner.get_all_stats + + assert_equal 35, ret[:klass]['BadJob'] + assert_equal 7, ret[:klass]['BadJobWithSyntaxError'] + + assert_equal 6, ret[:date]['2009/03/13'] + assert_equal 14, ret[:date]['2009/11/13'] + + assert_equal 35, ret[:exception]['RuntimeError'] + assert_equal 7, ret[:exception]['SyntaxError'] + end + it "#stats_by_class works with broken log" do add_empty_payload_failure ret = @cleaner.stats_by_class diff --git a/test/resque_web_test.rb b/test/resque_web_test.rb index f98d280..041f5c8 100644 --- a/test/resque_web_test.rb +++ b/test/resque_web_test.rb @@ -53,11 +53,32 @@ def setup_some_failed_jobs assert last_response.body.include?('"BadJob"') end + it '#cleaner_one shows the failed jobs' do + get "/cleaner_one" + assert_includes last_response.body, 'BadJob' + end + + it '#cleaner_one shows the failed jobs for a class' do + get "/cleaner_one", :c => "BadJobWithSyntaxError" + assert_includes last_response.body, 'BadJobWithSyntaxError' + end + + it '#cleaner_one shows only the first failed job for index 0' do + get "/cleaner_one", :fsi => "0", :cf => "1" + assert_includes last_response.body, 'BadJobWithSyntaxError' + end + + it '#cleaner_one shows only the first failed job for index 1' do + get "/cleaner_one", :fsi => "1", :cf => "1" + assert_includes last_response.body, 'BadJob' + assert_equal false, last_response.body.include?('BadJobWithSyntaxError') + end it '#cleaner_exec clears job' do post "/cleaner_exec", :action => "clear", :sha1 => Digest::SHA1.hexdigest(@cleaner.select[0].to_json) assert_equal 10, @cleaner.select.size end + it "#cleaner_dump should respond with success" do get "/cleaner_dump" assert last_response.ok?, last_response.errors