Skip to content

Commit cfe6042

Browse files
committed
refactor: inline the instance lookup queries
Put each lookup at its only call site, and drop the two helpers that forwarded a single query. Assert that every worker in the create race test finishes, and kill any worker that outlives its timeout, so a worker cannot hold a pooled connection while the assertions run.
1 parent 8ceb243 commit cfe6042

3 files changed

Lines changed: 8 additions & 21 deletions

File tree

‎lib/solid_objects/mailbox.rb‎

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -107,19 +107,14 @@ def with_instance_retry
107107

108108
# @rbs (Reference, Class) -> Instance
109109
def find_or_create_instance(reference, actor_class)
110-
identifier = instance_identifier(reference)
110+
identifier = Instance
111+
.where(actor_type: reference.actor_type, actor_id: reference.actor_id)
112+
.pick(:id)
111113
return lock_instance!(identifier) if identifier
112114

113115
create_locked_instance(reference, actor_class)
114116
end
115117

116-
# @rbs (Reference) -> Integer?
117-
def instance_identifier(reference)
118-
Instance
119-
.where(actor_type: reference.actor_type, actor_id: reference.actor_id)
120-
.pick(:id)
121-
end
122-
123118
# @rbs (Reference, Class) -> Instance
124119
def create_locked_instance(reference, actor_class)
125120
Instance.transaction(requires_new: true) do
@@ -131,14 +126,10 @@ def create_locked_instance(reference, actor_class)
131126
)
132127
end
133128
rescue ActiveRecord::RecordNotUnique
134-
lock_instance!(committed_instance_identifier(reference))
135-
end
136-
137-
# @rbs (Reference) -> Integer?
138-
def committed_instance_identifier(reference)
139-
database_adapter.share_locked(
129+
identifier = database_adapter.share_locked(
140130
Instance.where(actor_type: reference.actor_type, actor_id: reference.actor_id)
141131
).pick(:id)
132+
lock_instance!(identifier)
142133
end
143134

144135
# @rbs (Integer?) -> Instance

‎sig/generated/lib/solid_objects/mailbox.rbs‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,9 @@ module SolidObjects
2828
# @rbs (Reference, Class) -> Instance
2929
def find_or_create_instance: (Reference, Class) -> Instance
3030

31-
# @rbs (Reference) -> Integer?
32-
def instance_identifier: (Reference) -> Integer?
33-
3431
# @rbs (Reference, Class) -> Instance
3532
def create_locked_instance: (Reference, Class) -> Instance
3633

37-
# @rbs (Reference) -> Integer?
38-
def committed_instance_identifier: (Reference) -> Integer?
39-
4034
# @rbs (Integer?) -> Instance
4135
def lock_instance!: (Integer?) -> Instance
4236

‎test/integration/enqueue_test.rb‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,10 @@ def add(product_id:)
116116
end
117117

118118
threads.length.times { start << true }
119-
threads.each { |thread| thread.join(30) }
119+
unfinished = threads.reject { |thread| thread.join(30) }
120+
unfinished.each(&:kill).each(&:join)
120121

122+
assert_empty unfinished, "an enqueue was still running after its timeout"
121123
assert_empty errors.size.times.map { errors.pop }
122124
assert_equal (1..8).to_a, sequences.size.times.map { sequences.pop }.sort
123125
assert_equal 1, SolidObjects::Instance.where(actor_type: "enqueue-carts", actor_id: "alice").count

0 commit comments

Comments
 (0)