Merge pull request #798 from supercaracal/fix-cluster-tests-that-occasionally-fails-on-ci

Fix several test cases that occasionally fail on CI

Author: byroot

.travis.yml

@@ -26,6 +26,7 @@ env:
   global:
     - VERBOSE=true
     - TIMEOUT=30
+    - LOW_TIMEOUT=0.01
   matrix:
     - DRIVER=ruby      REDIS_BRANCH=3.0
     - DRIVER=ruby      REDIS_BRANCH=3.2
@@ -43,17 +44,14 @@ branches:
 
 matrix:
   exclude:
-    # hiredis
     - rvm: jruby-9.1.17.0
-      env: DRIVER=hiredis   REDIS_BRANCH=3.0
+  include:
     - rvm: jruby-9.1.17.0
-      env: DRIVER=hiredis   REDIS_BRANCH=3.2
-
-    # synchrony
+      env: DRIVER=ruby REDIS_BRANCH=3.0 LOW_TIMEOUT=0.1
     - rvm: jruby-9.1.17.0
-      env: DRIVER=synchrony REDIS_BRANCH=3.0
+      env: DRIVER=ruby REDIS_BRANCH=3.2 LOW_TIMEOUT=0.1
     - rvm: jruby-9.1.17.0
-      env: DRIVER=synchrony REDIS_BRANCH=3.2
+      env: DRIVER=ruby REDIS_BRANCH=4.0 LOW_TIMEOUT=0.1
 
 notifications:
   irc:

test/cluster_client_options_test.rb

@@ -38,7 +38,7 @@ def test_client_accepts_valid_node_configs
 
   def test_client_accepts_valid_options
     assert_nothing_raised do
-      build_another_client(timeout: 1.0)
+      build_another_client(timeout: TIMEOUT)
     end
   end
 

test/cluster_client_transactions_test.rb

@@ -40,7 +40,7 @@ def test_transaction_with_replicas
       100.times { |i| cli.set("{key}#{i}", i) }
     end
 
-    sleep 0.1
+    sleep 0.5
 
     100.times { |i| assert_equal i.to_s, rc1.get("{key}#{i}") }
     100.times { |i| assert_equal i.to_s, rc2.get("{key}#{i}") }

test/helper.rb

@@ -15,9 +15,11 @@
 require_relative "support/connection/#{ENV["DRIVER"]}"
 require_relative 'support/cluster/orchestrator'
 
-PORT    = 6381
-OPTIONS = {:port => PORT, :db => 15, :timeout => Float(ENV["TIMEOUT"] || 0.1)}
-NODES   = ["redis://127.0.0.1:#{PORT}/15"]
+PORT        = 6381
+DB          = 15
+TIMEOUT     = Float(ENV['TIMEOUT'] || 0.1)
+LOW_TIMEOUT = Float(ENV['LOW_TIMEOUT'] || 0.01) # for blocking-command tests
+OPTIONS     = { port: PORT, db: DB, timeout: TIMEOUT }.freeze
 
 def driver(*drivers, &blk)
   if drivers.map(&:to_s).include?(ENV["DRIVER"])
@@ -178,9 +180,10 @@ def _new_client(options = {})
   end
 
   module Distributed
-
     include Generic
 
+    NODES = ["redis://127.0.0.1:#{PORT}/15"].freeze
+
     def version
       Version.new(redis.info.first["redis_version"])
     end

test/lint/blocking_commands.rb

@@ -1,7 +1,5 @@
 module Lint
   module BlockingCommands
-    LOW_TIMEOUT = 0.01
-
     def setup
       super
 

test/support/cluster/orchestrator.rb

@@ -21,7 +21,6 @@ def rebuild
     replicate(@clients)
     save_config(@clients)
     wait_cluster_building(@clients)
-    sleep 3
   end
 
   def down
@@ -30,8 +29,9 @@ def down
   end
 
   def failover
-    take_slaves(@clients).last.cluster(:failover, :takeover)
-    sleep 3
+    master, slave = take_replication_pairs(@clients)
+    slave.cluster(:failover, :takeover)
+    wait_failover(to_node_key(master), to_node_key(slave), @clients)
   end
 
   def start_resharding(slot, src_node_key, dest_node_key)
@@ -157,20 +157,44 @@ def save_config(clients)
     clients.each { |c| c.cluster(:saveconfig) }
   end
 
-  def wait_cluster_building(clients)
-    first_cliient = clients.first
+  def wait_cluster_building(clients, max_attempts: 200)
+    attempt_count = 0
 
-    loop do
-      info = hashify_cluster_info(first_cliient)
-      break if info['cluster_state'] == 'ok'
-      sleep 0.1
+    clients.each do |client|
+      loop do
+        info = hashify_cluster_info(client)
+        attempt_count += 1
+        break if info['cluster_state'] == 'ok' || attempt_count > max_attempts
+        sleep 0.1
+      end
+    end
+  end
+
+  def wait_failover(master_key, slave_key, clients, max_attempts: 200)
+    attempt_count = 0
+
+    clients.each do |client|
+      loop do
+        flags = hashify_cluster_node_flags(client)
+        attempt_count += 1
+        break if (flags[master_key] == 'slave' && flags[slave_key] == 'master') || attempt_count > max_attempts
+        sleep 0.1
+      end
     end
   end
 
   def hashify_cluster_info(client)
     client.cluster(:info).split("\r\n").map { |str| str.split(':') }.to_h
   end
 
+  def hashify_cluster_node_flags(client)
+    client.cluster(:nodes)
+          .split("\n")
+          .map { |str| str.split(' ') }
+          .map { |arr| [arr[1].split('@').first, (arr[2].split(',') & %w[master slave]).first] }
+          .to_h
+  end
+
   def hashify_node_map(client)
     client.cluster(:nodes)
           .split("\n")
@@ -191,10 +215,16 @@ def take_slaves(clients)
     clients[size..size * 2]
   end
 
+  def take_replication_pairs(clients)
+    [take_masters(clients).last, take_slaves(clients).last]
+  end
+
   def find_client(clients, node_key)
-    clients.find do |cli|
-      con = cli.connection
-      node_key == "#{con.fetch(:host)}:#{con.fetch(:port)}"
-    end
+    clients.find { |cli| node_key == to_node_key(cli) }
+  end
+
+  def to_node_key(client)
+    con = client.connection
+    "#{con.fetch(:host)}:#{con.fetch(:port)}"
   end
 end