Commit db3fc55258dfaf5b13982b2395865d599573a473
1 parent
7dd78504
Exists in
master
and in
1 other branch
Problem notices cache is more space effective
Showing
4 changed files
with
83 additions
and
24 deletions
Show diff stats
app/helpers/application_helper.rb
| @@ -47,8 +47,8 @@ module ApplicationHelper | @@ -47,8 +47,8 @@ module ApplicationHelper | ||
| 47 | end | 47 | end |
| 48 | 48 | ||
| 49 | def tally(collection) | 49 | def tally(collection) |
| 50 | - collection.inject({}) do |tallies, value| | ||
| 51 | - tallies[value] = (tallies[value] || 0) + 1 | 50 | + collection.values.inject({}) do |tallies, tally| |
| 51 | + tallies[tally['value']] = tally['count'] | ||
| 52 | tallies | 52 | tallies |
| 53 | end | 53 | end |
| 54 | end | 54 | end |
app/models/problem.rb
| @@ -18,9 +18,9 @@ class Problem | @@ -18,9 +18,9 @@ class Problem | ||
| 18 | field :environment | 18 | field :environment |
| 19 | field :klass | 19 | field :klass |
| 20 | field :where | 20 | field :where |
| 21 | - field :user_agents, :type => Array, :default => [] | ||
| 22 | - field :messages, :type => Array, :default => [] | ||
| 23 | - field :hosts, :type => Array, :default => [] | 21 | + field :user_agents, :type => Hash, :default => {} |
| 22 | + field :messages, :type => Hash, :default => {} | ||
| 23 | + field :hosts, :type => Hash, :default => {} | ||
| 24 | field :comments_count, :type => Integer, :default => 0 | 24 | field :comments_count, :type => Integer, :default => 0 |
| 25 | 25 | ||
| 26 | index :app_id | 26 | index :app_id |
| @@ -128,19 +128,45 @@ class Problem | @@ -128,19 +128,45 @@ class Problem | ||
| 128 | :environment => notice.environment_name, | 128 | :environment => notice.environment_name, |
| 129 | :klass => notice.klass, | 129 | :klass => notice.klass, |
| 130 | :where => notice.where, | 130 | :where => notice.where, |
| 131 | - :messages => messages.push(notice.message), | ||
| 132 | - :hosts => hosts.push(notice.host), | ||
| 133 | - :user_agents => user_agents.push(notice.user_agent_string) | 131 | + :messages => attribute_count_increase(:messages, notice.message), |
| 132 | + :hosts => attribute_count_increase(:hosts, notice.host), | ||
| 133 | + :user_agents => attribute_count_increase(:user_agents, notice.user_agent_string) | ||
| 134 | ) if notice | 134 | ) if notice |
| 135 | update_attributes!(attrs) | 135 | update_attributes!(attrs) |
| 136 | end | 136 | end |
| 137 | 137 | ||
| 138 | def remove_cached_notice_attribures(notice) | 138 | def remove_cached_notice_attribures(notice) |
| 139 | - messages.delete_at(messages.index(notice.message)) | ||
| 140 | - hosts.delete_at(hosts.index(notice.host)) | ||
| 141 | - user_agents.delete_at(user_agents.index(notice.user_agent_string)) | ||
| 142 | - save! | 139 | + update_attributes!( |
| 140 | + :messages => attribute_count_descrease(:messages, notice.message), | ||
| 141 | + :hosts => attribute_count_descrease(:hosts, notice.host), | ||
| 142 | + :user_agents => attribute_count_descrease(:user_agents, notice.user_agent_string) | ||
| 143 | + ) | ||
| 143 | end | 144 | end |
| 144 | 145 | ||
| 146 | + private | ||
| 147 | + def attribute_count_increase(name, value) | ||
| 148 | + counter, index = send(name), attribute_index(value) | ||
| 149 | + if counter[index].nil? | ||
| 150 | + counter[index] = {'value' => value, 'count' => 1} | ||
| 151 | + else | ||
| 152 | + counter[index]['count'] += 1 | ||
| 153 | + end | ||
| 154 | + counter | ||
| 155 | + end | ||
| 156 | + | ||
| 157 | + def attribute_count_descrease(name, value) | ||
| 158 | + counter, index = send(name), attribute_index(value) | ||
| 159 | + if counter[index]['count'] > 1 | ||
| 160 | + counter[index]['count'] -= 1 | ||
| 161 | + else | ||
| 162 | + counter.delete(index) | ||
| 163 | + end | ||
| 164 | + counter | ||
| 165 | + end | ||
| 166 | + | ||
| 167 | + def attribute_index(value) | ||
| 168 | + Digest::MD5.hexdigest(value.to_s) | ||
| 169 | + end | ||
| 170 | + | ||
| 145 | end | 171 | end |
| 146 | 172 |
db/migrate/20111102173347_cache_problem_statistics_fix.rb
0 → 100644
| @@ -0,0 +1,33 @@ | @@ -0,0 +1,33 @@ | ||
| 1 | +class CacheProblemStatisticsFix < Mongoid::Migration | ||
| 2 | + def self.up | ||
| 3 | + Problem.all.each do |problem| | ||
| 4 | + messages = {} | ||
| 5 | + hosts = {} | ||
| 6 | + user_agents = {} | ||
| 7 | + problem.notices.each do |notice| | ||
| 8 | + messages = count_attribute(messages, notice.message) | ||
| 9 | + hosts = count_attribute(hosts, notice.host) | ||
| 10 | + user_agents = count_attribute(user_agents, notice.user_agent_string) | ||
| 11 | + end | ||
| 12 | + problem.update_attributes(:messages => messages, :hosts => hosts, :user_agents => user_agents) | ||
| 13 | + end | ||
| 14 | + end | ||
| 15 | + | ||
| 16 | + def self.down | ||
| 17 | + Problem.all.each do |problem| | ||
| 18 | + problem.update_attributes(:messages => {}, :hosts => {}, :user_agents => {}) | ||
| 19 | + end | ||
| 20 | + end | ||
| 21 | + | ||
| 22 | + private | ||
| 23 | + def self.count_attribute(counter, value) | ||
| 24 | + index = Digest::MD5.hexdigest(value.to_s) | ||
| 25 | + if counter[index].nil? | ||
| 26 | + counter[index] = {'value' => value, 'count' => 1} | ||
| 27 | + else | ||
| 28 | + counter[index]['count'] += 1 | ||
| 29 | + end | ||
| 30 | + counter | ||
| 31 | + end | ||
| 32 | + | ||
| 33 | +end | ||
| 0 | \ No newline at end of file | 34 | \ No newline at end of file |
spec/models/problem_spec.rb
| @@ -205,14 +205,14 @@ describe Problem do | @@ -205,14 +205,14 @@ describe Problem do | ||
| 205 | @err = Factory(:err, :problem => @problem) | 205 | @err = Factory(:err, :problem => @problem) |
| 206 | end | 206 | end |
| 207 | 207 | ||
| 208 | - it "#messages returns [] by default" do | ||
| 209 | - @problem.messages.should == [] | 208 | + it "#messages should be empty by default" do |
| 209 | + @problem.messages.should == {} | ||
| 210 | end | 210 | end |
| 211 | 211 | ||
| 212 | it "adding a notice adds a string to #messages" do | 212 | it "adding a notice adds a string to #messages" do |
| 213 | lambda { | 213 | lambda { |
| 214 | Factory(:notice, :err => @err, :message => 'ERR 1') | 214 | Factory(:notice, :err => @err, :message => 'ERR 1') |
| 215 | - }.should change(@problem, :messages).from([]).to(['ERR 1']) | 215 | + }.should change(@problem, :messages).from({}).to({Digest::MD5.hexdigest('ERR 1') => {'value' => 'ERR 1', 'count' => 1}}) |
| 216 | end | 216 | end |
| 217 | 217 | ||
| 218 | it "removing a notice removes string from #messages" do | 218 | it "removing a notice removes string from #messages" do |
| @@ -220,7 +220,7 @@ describe Problem do | @@ -220,7 +220,7 @@ describe Problem do | ||
| 220 | lambda { | 220 | lambda { |
| 221 | @err.notices.first.destroy | 221 | @err.notices.first.destroy |
| 222 | @problem.reload | 222 | @problem.reload |
| 223 | - }.should change(@problem, :messages).from(['ERR 1']).to([]) | 223 | + }.should change(@problem, :messages).from({Digest::MD5.hexdigest('ERR 1') => {'value' => 'ERR 1', 'count' => 1}}).to({}) |
| 224 | end | 224 | end |
| 225 | end | 225 | end |
| 226 | 226 | ||
| @@ -231,14 +231,14 @@ describe Problem do | @@ -231,14 +231,14 @@ describe Problem do | ||
| 231 | @err = Factory(:err, :problem => @problem) | 231 | @err = Factory(:err, :problem => @problem) |
| 232 | end | 232 | end |
| 233 | 233 | ||
| 234 | - it "#hosts returns [] by default" do | ||
| 235 | - @problem.hosts.should == [] | 234 | + it "#hosts should be empty by default" do |
| 235 | + @problem.hosts.should == {} | ||
| 236 | end | 236 | end |
| 237 | 237 | ||
| 238 | it "adding a notice adds a string to #hosts" do | 238 | it "adding a notice adds a string to #hosts" do |
| 239 | lambda { | 239 | lambda { |
| 240 | Factory(:notice, :err => @err, :request => {'url' => "http://example.com/resource/12"}) | 240 | Factory(:notice, :err => @err, :request => {'url' => "http://example.com/resource/12"}) |
| 241 | - }.should change(@problem, :hosts).from([]).to(['example.com']) | 241 | + }.should change(@problem, :hosts).from({}).to({Digest::MD5.hexdigest('example.com') => {'value' => 'example.com', 'count' => 1}}) |
| 242 | end | 242 | end |
| 243 | 243 | ||
| 244 | it "removing a notice removes string from #hosts" do | 244 | it "removing a notice removes string from #hosts" do |
| @@ -246,7 +246,7 @@ describe Problem do | @@ -246,7 +246,7 @@ describe Problem do | ||
| 246 | lambda { | 246 | lambda { |
| 247 | @err.notices.first.destroy | 247 | @err.notices.first.destroy |
| 248 | @problem.reload | 248 | @problem.reload |
| 249 | - }.should change(@problem, :hosts).from(['example.com']).to([]) | 249 | + }.should change(@problem, :hosts).from({Digest::MD5.hexdigest('example.com') => {'value' => 'example.com', 'count' => 1}}).to({}) |
| 250 | end | 250 | end |
| 251 | end | 251 | end |
| 252 | 252 | ||
| @@ -257,14 +257,14 @@ describe Problem do | @@ -257,14 +257,14 @@ describe Problem do | ||
| 257 | @err = Factory(:err, :problem => @problem) | 257 | @err = Factory(:err, :problem => @problem) |
| 258 | end | 258 | end |
| 259 | 259 | ||
| 260 | - it "#user_agents returns [] by default" do | ||
| 261 | - @problem.user_agents.should == [] | 260 | + it "#user_agents should be empty by default" do |
| 261 | + @problem.user_agents.should == {} | ||
| 262 | end | 262 | end |
| 263 | 263 | ||
| 264 | it "adding a notice adds a string to #user_agents" do | 264 | it "adding a notice adds a string to #user_agents" do |
| 265 | lambda { | 265 | lambda { |
| 266 | Factory(:notice, :err => @err, :request => {'cgi-data' => {'HTTP_USER_AGENT' => 'Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_7; en-US) AppleWebKit/534.16 (KHTML, like Gecko) Chrome/10.0.648.204 Safari/534.16'}}) | 266 | Factory(:notice, :err => @err, :request => {'cgi-data' => {'HTTP_USER_AGENT' => 'Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_7; en-US) AppleWebKit/534.16 (KHTML, like Gecko) Chrome/10.0.648.204 Safari/534.16'}}) |
| 267 | - }.should change(@problem, :user_agents).from([]).to(['Chrome 10.0.648.204']) | 267 | + }.should change(@problem, :user_agents).from({}).to({Digest::MD5.hexdigest('Chrome 10.0.648.204') => {'value' => 'Chrome 10.0.648.204', 'count' => 1}}) |
| 268 | end | 268 | end |
| 269 | 269 | ||
| 270 | it "removing a notice removes string from #user_agents" do | 270 | it "removing a notice removes string from #user_agents" do |
| @@ -272,7 +272,7 @@ describe Problem do | @@ -272,7 +272,7 @@ describe Problem do | ||
| 272 | lambda { | 272 | lambda { |
| 273 | @err.notices.first.destroy | 273 | @err.notices.first.destroy |
| 274 | @problem.reload | 274 | @problem.reload |
| 275 | - }.should change(@problem, :user_agents).from(['Chrome 10.0.648.204']).to([]) | 275 | + }.should change(@problem, :user_agents).from({Digest::MD5.hexdigest('Chrome 10.0.648.204') => {'value' => 'Chrome 10.0.648.204', 'count' => 1}}).to({}) |
| 276 | end | 276 | end |
| 277 | end | 277 | end |
| 278 | 278 |