Skip to content

Commit c1d2618

Browse files
Only allow one follower to do bulk sync at a time (#1720)
### WHAT is this pull request doing? Serializes follower `full_sync` so only one follower syncs at a time. Two followers doing `full_sync` simultaneously crashes the leader with `SIGSEGV` because both iterate `@files` and write to `@checksums` from separate threads without synchronization. Fixes #1708 ### HOW can this pull request be tested? TBA --------- Co-authored-by: Carl Hörberg <[email protected]>
1 parent e35be0b commit c1d2618

1 file changed

Lines changed: 8 additions & 1 deletion

File tree

src/lavinmq/clustering/server.cr

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ module LavinMQ
2828
Log = LavinMQ::Log.for "clustering.server"
2929

3030
@lock = Mutex.new(:unchecked)
31+
@sync_lock = Mutex.new(:unchecked)
3132
@followers = Array(Follower).new(4)
3233
@password : String
3334
@files = Hash(String, MFile?).new
@@ -184,7 +185,13 @@ module LavinMQ
184185
end
185186
@followers << follower # Starts in Syncing state
186187
end
187-
follower.full_sync # sync the bulk
188+
# Only allow one follower to do bulk sync at a time
189+
# The bandwidth between nodes should be very high, so
190+
# better with one fully synced than 2 partially synced followers
191+
# Also @files and @checksums are not protected by locks
192+
@sync_lock.synchronize do
193+
follower.full_sync # sync the bulk
194+
end
188195
@lock.synchronize do
189196
follower.full_sync # sync the last
190197
follower.mark_synced! # Change state to Synced

0 commit comments

Comments
 (0)