Skip to content

Commit 8ee430f

Browse files
committed
Remove unnecessary will_change! calls causing excessive UPDATE queries
What went wrong: - will_change! was called even when the value didn't actually change - Example: user.mother_id = mother.id (assigning the same value) - ActiveRecord marked all serialized columns as dirty - Result: massive number of unnecessary UPDATE queries, causing outage Root cause analysis: - ActiveRecord's serialize columns automatically perform dirty tracking when hash values are modified internally - Manual will_change! calls were completely unnecessary
1 parent 800481c commit 8ee430f

File tree

3 files changed

+143
-3
lines changed

3 files changed

+143
-3
lines changed

lib/coaster/serialized_properties.rb

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ def _write_sprop_value(key, value, apply_setter: true)
2727
setting = self.class.serialized_property_setting(key)
2828
return unless setting
2929
col = setting[:column]
30-
send("#{col}_will_change!")
3130
hsh = send(col)
3231
if value.nil?
3332
hsh.delete(key.to_s)
@@ -312,7 +311,6 @@ def _define_serialized_property(serialize_column, key, getter: nil, setter: nil,
312311
if is_active_record
313312
define_method "#{key}_without_callback=".to_sym do |val|
314313
col = serialize_column
315-
send("#{col}_will_change!")
316314
hsh = send(col)
317315
if val.nil?
318316
hsh.delete(key.to_s)

lib/coaster/version.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
module Coaster
2-
VERSION = '1.4.38'
2+
VERSION = '1.4.39'
33
end
Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
require 'test_helper'
2+
require 'minitest/autorun'
3+
4+
module Coaster
5+
class TestSerializedProperty < Minitest::Test
6+
def setup
7+
end
8+
9+
def teardown
10+
end
11+
12+
def test_dirty
13+
user = User.create(name: 'abc')
14+
mother = User.create(name: 'mother')
15+
user.mother = mother
16+
assert_equal mother, user.mother
17+
assert_equal mother.id, user.mother_id
18+
assert_equal({"mother_id"=>mother.id}, user.data)
19+
assert_equal({"data" => [{}, {"mother_id"=>mother.id}]}, user.changes)
20+
assert_equal({"mother_id" => [nil, mother.id]}, user.sprop_changes)
21+
assert_equal(true, user.mother_id_changed?)
22+
assert_equal(nil, user.mother_id_was)
23+
user.save!
24+
user = User.find(user.id)
25+
user.mother_id = mother.id
26+
assert_equal(false, user.mother_id_changed?)
27+
assert_equal({}, user.changes)
28+
step_mother = User.create(name: 'step_mother')
29+
user.mother = step_mother
30+
assert_equal({"data" => [{"mother_id" => mother.id}, {"mother_id" => step_mother.id}]}, user.changes)
31+
assert_equal({"mother_id" => [mother.id, step_mother.id]}, user.sprop_changes)
32+
end
33+
34+
# Tests for dirty tracking optimization (skip dirty marking when value unchanged)
35+
def test_same_value_setter_does_not_mark_dirty
36+
user = User.create(name: 'test_same_value')
37+
user.simple = 'initial_value'
38+
user.save!
39+
40+
# Reload to clear dirty state
41+
user.reload
42+
assert_equal false, user.changed?
43+
44+
# Set same value - should NOT mark dirty
45+
user.simple = 'initial_value'
46+
assert_equal false, user.simple_changed?, 'Setting same value should not mark property as changed'
47+
assert_equal false, user.changed?, 'Setting same value should not mark record as changed'
48+
end
49+
50+
def test_different_value_setter_marks_dirty
51+
user = User.create(name: 'test_different_value')
52+
user.simple = 'initial_value'
53+
user.save!
54+
user.reload
55+
56+
# Set different value - should mark dirty
57+
user.simple = 'new_value'
58+
assert_equal true, user.simple_changed?, 'Setting different value should mark property as changed'
59+
assert_equal true, user.changed?, 'Setting different value should mark record as changed'
60+
assert_equal ['initial_value', 'new_value'], user.simple_change
61+
end
62+
63+
def test_nil_to_value_marks_dirty
64+
user = User.create(name: 'test_nil_to_value')
65+
user.save!
66+
user.reload
67+
68+
assert_nil user.simple
69+
user.simple = 'some_value'
70+
assert_equal true, user.simple_changed?
71+
assert_equal [nil, 'some_value'], user.simple_change
72+
end
73+
74+
def test_value_to_nil_marks_dirty
75+
user = User.create(name: 'test_value_to_nil')
76+
user.simple = 'some_value'
77+
user.save!
78+
user.reload
79+
80+
user.simple = nil
81+
assert_equal true, user.simple_changed?
82+
assert_equal ['some_value', nil], user.simple_change
83+
end
84+
85+
def test_nil_to_nil_does_not_mark_dirty
86+
user = User.create(name: 'test_nil_to_nil')
87+
user.save!
88+
user.reload
89+
90+
assert_nil user.simple
91+
user.simple = nil
92+
assert_equal false, user.simple_changed?, 'Setting nil to nil should not mark as changed'
93+
assert_equal false, user.changed?
94+
end
95+
96+
def test_same_value_via_write_attribute_does_not_mark_dirty
97+
user = User.create(name: 'test_write_attr_same')
98+
user.write_attribute(:simple, 'initial')
99+
user.save!
100+
user.reload
101+
102+
user.write_attribute(:simple, 'initial')
103+
assert_equal false, user.simple_changed?
104+
assert_equal false, user.changed?
105+
end
106+
107+
def test_same_value_via_bracket_accessor_does_not_mark_dirty
108+
user = User.create(name: 'test_bracket_same')
109+
user[:simple] = 'initial'
110+
user.save!
111+
user.reload
112+
113+
user[:simple] = 'initial'
114+
assert_equal false, user.simple_changed?
115+
assert_equal false, user.changed?
116+
end
117+
118+
def test_same_value_with_setter_proc_does_not_mark_dirty
119+
student = Student.create(name: 'test_setter_same')
120+
student.uppercase_name = 'hello' # Stored as 'HELLO'
121+
student.save!
122+
student.reload
123+
124+
# Setting same raw input - setter transforms to same value
125+
student.uppercase_name = 'hello'
126+
assert_equal false, student.uppercase_name_changed?, 'Same value after setter transform should not mark dirty'
127+
assert_equal false, student.changed?
128+
end
129+
130+
def test_different_case_with_setter_proc_marks_dirty
131+
student = Student.create(name: 'test_setter_diff')
132+
student.uppercase_name = 'hello' # Stored as 'HELLO'
133+
student.save!
134+
student.reload
135+
136+
# Setting different value
137+
student.uppercase_name = 'world' # Stored as 'WORLD'
138+
assert_equal true, student.uppercase_name_changed?
139+
assert_equal ['HELLO', 'WORLD'], student.uppercase_name_change
140+
end
141+
end
142+
end

0 commit comments

Comments
 (0)