fluencelabs/redis
1
0
Fork 0
mirror of https://github.com/fluencelabs/redis synced 2025-03-17 16:10:50 +00:00
Code Issues Projects Releases Wiki Activity

Cluster: Fix trib create when masters==replicas

Browse Source 
This bug was introduced in 2e5c394f during a refactor.

It took me a while to understand what was going on with
the code, so I've refactored it further by:
  - Replacing boolean values with meaningful symbols
  - Replacing 'i' with a meaningful variable name
  - Adding the proper abort check
  - Factoring out now duplicated conditionals
  - Adding optional verbose logging (we're inside *four*
    different looping constructs, so it takes a while to
    figure out where all the moving parts are)
  - Updating comment for the section

This fixes a problem when the number of master instances
equaled the number of replica instances.  Before, when
there were equal numbers of both, nodes_count would go to
zero, but the while loop would spin in i < @replicas because
i would never be updated (because the nodes_list of each ip
was length == 0, which triggered an endless loop of
next -> i = 0 -> 0 < 1? -> true -> next -> i = 0 ...)

Thanks to carlo who found this problem at:
https://groups.google.com/forum/#!topic/redis-db/_WVVqDw5B7c
This commit is contained in:
Matt Stancliff 2014-03-21 13:09:23 -04:00
parent 93253c2762
commit df4bdbf688
1 changed files with 22 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
src/redis-trib.rb
View File

@ -554,14 +554,28 @@ class RedisTrib
# We try to split the replicas among all the IPs with spare nodes
# trying to avoid the host where the master is running, if possible.
#
# Note that we loop two times, the first with spare_loop set to false,
# the second with the var set to true. The second loop changes the
# while condition so to assign remaining slaves.
[false,true].each{|spare_loop|
# Note we loop two times. The first loop assigns the requested
# number of replicas to each master. The second loop assigns any
# remaining instances as extra replicas to masters. Some masters
# may end up with more than their requested number of replicas, but
# all nodes will be used.
assignment_verbose = false
[:requested,:unused].each{|assign|
masters.each{|m|
i = 0
while (!spare_loop && i < @replicas) || \
(spare_loop && nodes_count > 0)
assigned_replicas = 0
while assigned_replicas < @replicas
break if nodes_count == 0
if assignment_verbose
if assign == :requested
puts "Requesting total of #{@replicas} replicas " \
"(#{assigned_replicas} replicas assigned " \
"so far with #{nodes_count} total remaining)."
elsif assign == :unused
puts "Assigning extra instance to replication " \
"role too (#{nodes_count} remaining)."
end
end
ips.each{|ip,nodes_list|
next if nodes_list.length == 0
# Skip instances with the same IP as the master if we
@ -570,7 +584,7 @@ class RedisTrib
slave = nodes_list.shift
slave.set_as_replica(m.info[:name])
nodes_count -= 1
i += 1
assigned_replicas += 1
puts "Adding replica #{slave} to #{m}"
}
end