Skip to content

Commit 1d24f5c

Browse files
authored
hold sync_lock for both full_sync calls (#1780)
### WHAT is this pull request doing? The second `full_sync` was not protected by `@sync_lock`, allowing concurrent syncs. Nests `@lock` inside `@sync_lock` so all full_sync calls are serialized. Related to #1708, Follow-up to #1720, based on review feedback from @spuun.
1 parent 810cbd7 commit 1d24f5c

File tree

1 file changed

+7
-6
lines changed

1 file changed

+7
-6
lines changed

src/lavinmq/clustering/server.cr

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -185,17 +185,18 @@ module LavinMQ
185185
end
186186
@followers << follower # Starts in Syncing state
187187
end
188-
# Only allow one follower to do bulk sync at a time
188+
# Only allow one follower to do full sync at a time
189189
# The bandwidth between nodes should be very high, so
190190
# better with one fully synced than 2 partially synced followers
191191
# Also @files and @checksums are not protected by locks
192+
# @sync_lock is always acquired before @lock to avoid deadlock
192193
@sync_lock.synchronize do
193194
follower.full_sync # sync the bulk
194-
end
195-
@lock.synchronize do
196-
follower.full_sync # sync the last
197-
follower.mark_synced! # Change state to Synced
198-
update_isr
195+
@lock.synchronize do
196+
follower.full_sync # sync the last
197+
follower.mark_synced! # Change state to Synced
198+
update_isr
199+
end
199200
end
200201
begin
201202
follower.action_loop

0 commit comments

Comments
 (0)