Skip to content

Commit b005809

Browse files
supercaracalsupercaracal
authored
ci: shorten blocking timeout seconds, add a test case for Valkey (#366)
1 parent 0b32144 commit b005809

File tree

10 files changed

+203
-32
lines changed

10 files changed

+203
-32
lines changed

‎.github/workflows/test.yaml‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ jobs:
2828
matrix:
2929
include:
3030
- {redis: '7.2', ruby: '3.3'}
31+
- {redis: '7.2', ruby: '3.3', compose: compose.valkey.yaml}
3132
- {redis: '7.2', ruby: '3.3', compose: compose.ssl.yaml}
3233
- {redis: '7.2', ruby: '3.3', driver: 'hiredis'}
3334
- {redis: '7.2', ruby: '3.3', driver: 'hiredis', compose: compose.ssl.yaml}
@@ -60,7 +61,7 @@ jobs:
6061
ruby-version: ${{ matrix.ruby || '3.3' }}
6162
bundler-cache: true
6263
- name: Pull Docker images
63-
run: docker pull redis:$REDIS_VERSION
64+
run: docker compose -f $DOCKER_COMPOSE_FILE pull
6465
- name: Run containers
6566
run: docker compose -f $DOCKER_COMPOSE_FILE up -d
6667
- name: Wait for Redis cluster to be ready
@@ -91,7 +92,7 @@ jobs:
9192
host_ip_addr=$(ip a | grep eth0 | grep inet | awk '{print $2}' | cut -d'/' -f1)
9293
echo "HOST_IP_ADDR=$host_ip_addr" >> $GITHUB_ENV
9394
- name: Pull Docker images
94-
run: docker pull redis:$REDIS_VERSION
95+
run: docker compose -f $DOCKER_COMPOSE_FILE pull
9596
- name: Run containers
9697
run: docker compose -f $DOCKER_COMPOSE_FILE up -d
9798
env:
@@ -162,7 +163,7 @@ jobs:
162163
ruby-version: '3.3'
163164
bundler-cache: true
164165
- name: Pull Docker images
165-
run: docker pull redis:$REDIS_VERSION
166+
run: docker compose -f $DOCKER_COMPOSE_FILE pull
166167
- name: Run containers
167168
run: docker compose -f $DOCKER_COMPOSE_FILE up -d
168169
- name: Wait for Redis cluster to be ready
@@ -222,7 +223,7 @@ jobs:
222223
ruby-version: '3.3'
223224
bundler-cache: true
224225
- name: Pull Docker images
225-
run: docker pull redis:$REDIS_VERSION
226+
run: docker compose -f $DOCKER_COMPOSE_FILE pull
226227
- name: Run containers
227228
run: docker compose -f $DOCKER_COMPOSE_FILE up -d
228229
- name: Wait for Redis cluster to be ready
@@ -260,7 +261,7 @@ jobs:
260261
ruby-version: '3.3'
261262
bundler-cache: true
262263
- name: Pull Docker images
263-
run: docker pull redis:$REDIS_VERSION
264+
run: docker compose -f $DOCKER_COMPOSE_FILE pull
264265
- name: Run containers
265266
run: docker compose -f $DOCKER_COMPOSE_FILE up -d
266267
- name: Wait for Redis cluster to be ready
@@ -316,7 +317,7 @@ jobs:
316317
sudo sysctl -w net.ipv4.tcp_max_syn_backlog=1024 # backlog setting
317318
sudo sysctl -w net.core.somaxconn=1024 # up the number of connections per port
318319
- name: Pull Docker images
319-
run: docker pull redis:$REDIS_VERSION
320+
run: docker compose -f $DOCKER_COMPOSE_FILE pull
320321
- name: Run containers
321322
run: docker compose -f $DOCKER_COMPOSE_FILE up -d
322323
- name: Print memory info

‎compose.valkey.yaml‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
---
2+
services:
3+
node1: &node
4+
image: "valkey/valkey:${REDIS_VERSION:-7}"
5+
command: >
6+
valkey-server
7+
--maxmemory 64mb
8+
--maxmemory-policy allkeys-lru
9+
--appendonly yes
10+
--cluster-enabled yes
11+
--cluster-config-file nodes.conf
12+
--cluster-node-timeout 5000
13+
restart: "${RESTART_POLICY:-always}"
14+
healthcheck:
15+
test: ["CMD", "valkey-cli", "ping"]
16+
interval: "7s"
17+
timeout: "5s"
18+
retries: 10
19+
ports:
20+
- "6379:6379"
21+
node2:
22+
<<: *node
23+
ports:
24+
- "6380:6379"
25+
node3:
26+
<<: *node
27+
ports:
28+
- "6381:6379"
29+
node4:
30+
<<: *node
31+
ports:
32+
- "6382:6379"
33+
node5:
34+
<<: *node
35+
ports:
36+
- "6383:6379"
37+
node6:
38+
<<: *node
39+
ports:
40+
- "6384:6379"
41+
clustering:
42+
image: "valkey/valkey:${REDIS_VERSION:-7}"
43+
command: >
44+
bash -c "apt-get update > /dev/null
45+
&& apt-get install --no-install-recommends --no-install-suggests -y dnsutils > /dev/null
46+
&& rm -rf /var/lib/apt/lists/*
47+
&& yes yes | valkey-cli --cluster create
48+
$$(dig node1 +short):6379
49+
$$(dig node2 +short):6379
50+
$$(dig node3 +short):6379
51+
$$(dig node4 +short):6379
52+
$$(dig node5 +short):6379
53+
$$(dig node6 +short):6379
54+
--cluster-replicas 1"
55+
depends_on:
56+
node1:
57+
condition: service_healthy
58+
node2:
59+
condition: service_healthy
60+
node3:
61+
condition: service_healthy
62+
node4:
63+
condition: service_healthy
64+
node5:
65+
condition: service_healthy
66+
node6:
67+
condition: service_healthy
68+
ruby:
69+
image: "ruby:${RUBY_VERSION:-3}"
70+
restart: always
71+
working_dir: /client
72+
volumes:
73+
- .:/client
74+
command:
75+
- ruby
76+
- "-e"
77+
- 'Signal.trap(:INT, "EXIT"); Signal.trap(:TERM, "EXIT"); loop { sleep 1 }'
78+
environment:
79+
REDIS_HOST: node1
80+
cap_drop:
81+
- ALL
82+
healthcheck:
83+
test: ["CMD", "ruby", "-e", "'puts 1'"]
84+
interval: "5s"
85+
timeout: "3s"
86+
retries: 3

‎test/benchmark_helper.rb‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,24 @@
1111
when 'hiredis' then require 'hiredis-client'
1212
end
1313

14-
class BenchmarkWrapper < Minitest::Benchmark; end
14+
class BenchmarkWrapper < Minitest::Benchmark
15+
private
16+
17+
def swap_timeout(client, timeout:)
18+
return if client.nil?
19+
20+
node = client.instance_variable_get(:@router)&.instance_variable_get(:@node)
21+
raise 'The client must be initialized.' if node.nil?
22+
23+
updater = lambda do |c, t|
24+
c.read_timeout = t
25+
c.config.instance_variable_set(:@read_timeout, t)
26+
end
27+
28+
regular_timeout = node.first.read_timeout
29+
node.each { |cli| updater.call(cli, timeout) }
30+
result = yield client
31+
node.each { |cli| updater.call(cli, regular_timeout) }
32+
result
33+
end
34+
end

‎test/benchmark_mixin.rb‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,9 @@ def bench_pipeline_get
8181
def wait_for_replication
8282
client_side_timeout = TEST_TIMEOUT_SEC + 1.0
8383
server_side_timeout = (TEST_TIMEOUT_SEC * 1000).to_i
84-
@client&.blocking_call(client_side_timeout, 'WAIT', TEST_REPLICA_SIZE, server_side_timeout)
84+
swap_timeout(@client, timeout: 0.1) do |client|
85+
client&.blocking_call(client_side_timeout, 'WAIT', TEST_REPLICA_SIZE, server_side_timeout)
86+
end
8587
end
8688
end
8789

@@ -109,6 +111,8 @@ def new_cluster_client
109111
def wait_for_replication
110112
client_side_timeout = TEST_TIMEOUT_SEC + 1.0
111113
server_side_timeout = (TEST_TIMEOUT_SEC * 1000).to_i
112-
@cluster_client&.blocking_call(client_side_timeout, 'WAIT', TEST_REPLICA_SIZE, server_side_timeout)
114+
swap_timeout(@cluster_client, timeout: 0.1) do |cluster_client|
115+
cluster_client&.blocking_call(client_side_timeout, 'WAIT', TEST_REPLICA_SIZE, server_side_timeout)
116+
end
113117
end
114118
end

‎test/cluster_controller.rb‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,9 @@ def wait_failover(clients, primary_node_key:, replica_node_key:, max_attempts:)
390390
def wait_replication_delay(clients, replica_size:, timeout:)
391391
timeout_msec = timeout.to_i * 1000
392392
wait_for_state(clients, max_attempts: clients.size + 1) do |client|
393-
client.blocking_call(timeout, 'WAIT', replica_size, timeout_msec - 100) if primary_client?(client)
393+
swap_timeout(client, timeout: 0.1) do |cli|
394+
cli.blocking_call(timeout, 'WAIT', replica_size, timeout_msec - 100) if primary_client?(cli)
395+
end
394396
true
395397
rescue ::RedisClient::ConnectionError
396398
true
@@ -503,4 +505,17 @@ def print_debug(msg)
503505

504506
p msg
505507
end
508+
509+
def swap_timeout(client, timeout:)
510+
updater = lambda do |c, t|
511+
c.read_timeout = t
512+
c.config.instance_variable_set(:@read_timeout, t)
513+
end
514+
515+
regular_timeout = client.read_timeout
516+
updater.call(client, timeout)
517+
result = yield client
518+
updater.call(client, regular_timeout)
519+
result
520+
end
506521
end

‎test/redis_client/test_cluster.rb‎

Lines changed: 35 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -72,18 +72,20 @@ def test_blocking_call
7272
client_side_timeout = TEST_REDIS_MAJOR_VERSION < 6 ? 2.0 : 1.5
7373
server_side_timeout = TEST_REDIS_MAJOR_VERSION < 6 ? '1' : '0.5'
7474

75-
assert_equal(%w[foo world], @client.blocking_call(client_side_timeout, 'BRPOP', 'foo', server_side_timeout), 'Case: 1st')
75+
swap_timeout(@client, timeout: 0.1) do |client|
76+
assert_equal(%w[foo world], client.blocking_call(client_side_timeout, 'BRPOP', 'foo', server_side_timeout), 'Case: 1st')
7677

77-
# FIXME: too flaky, just a workaround
78-
got = @client.blocking_call(client_side_timeout, 'BRPOP', 'foo', server_side_timeout)
79-
if got.nil?
80-
assert_nil(got, 'Case: 2nd')
81-
else
82-
assert_equal(%w[foo hello], got, 'Case: 2nd')
83-
end
78+
# FIXME: too flaky, just a workaround
79+
got = client.blocking_call(client_side_timeout, 'BRPOP', 'foo', server_side_timeout)
80+
if got.nil?
81+
assert_nil(got, 'Case: 2nd')
82+
else
83+
assert_equal(%w[foo hello], got, 'Case: 2nd')
84+
end
8485

85-
assert_nil(@client.blocking_call(client_side_timeout, 'BRPOP', 'foo', server_side_timeout), 'Case: 3rd')
86-
assert_raises(::RedisClient::ReadTimeoutError, 'Case: 4th') { @client.blocking_call(0.1, 'BRPOP', 'foo', 0) }
86+
assert_nil(client.blocking_call(client_side_timeout, 'BRPOP', 'foo', server_side_timeout), 'Case: 3rd')
87+
assert_raises(::RedisClient::ReadTimeoutError, 'Case: 4th') { client.blocking_call(0.1, 'BRPOP', 'foo', 0) }
88+
end
8789
end
8890

8991
def test_scan
@@ -146,12 +148,16 @@ def test_pipelined
146148
want = %w[PONG] + (0..9).map(&:to_s) + [%w[list 2]]
147149
client_side_timeout = TEST_REDIS_MAJOR_VERSION < 6 ? 1.5 : 1.0
148150
server_side_timeout = TEST_REDIS_MAJOR_VERSION < 6 ? '1' : '0.5'
149-
got = @client.pipelined do |pipeline|
150-
pipeline.call_once('PING')
151-
10.times { |i| pipeline.call('GET', "string#{i}") }
152-
pipeline.blocking_call(client_side_timeout, 'BRPOP', 'list', server_side_timeout)
151+
152+
swap_timeout(@client, timeout: 0.1) do |client|
153+
got = client.pipelined do |pipeline|
154+
pipeline.call_once('PING')
155+
10.times { |i| pipeline.call('GET', "string#{i}") }
156+
pipeline.blocking_call(client_side_timeout, 'BRPOP', 'list', server_side_timeout)
157+
end
158+
159+
assert_equal(want, got)
153160
end
154-
assert_equal(want, got)
155161
end
156162

157163
def test_pipelined_with_errors
@@ -815,8 +821,12 @@ def test_circuit_breakers
815821
)
816822
).new_client
817823

818-
assert_raises(::RedisClient::ReadTimeoutError) { cli.blocking_call(0.1, 'BRPOP', 'foo', 0) }
819-
assert_raises(::RedisClient::CircuitBreaker::OpenCircuitError) { cli.blocking_call(0.1, 'BRPOP', 'foo', 0) }
824+
cli.call('echo', 'init')
825+
826+
swap_timeout(cli, timeout: 0.1) do |c|
827+
assert_raises(::RedisClient::ReadTimeoutError) { c.blocking_call(0.1, 'BRPOP', 'foo', 0) }
828+
assert_raises(::RedisClient::CircuitBreaker::OpenCircuitError) { c.blocking_call(0.1, 'BRPOP', 'foo', 0) }
829+
end
820830

821831
cli&.close
822832
end
@@ -875,7 +885,9 @@ def test_initialization_delayed
875885
def wait_for_replication
876886
client_side_timeout = TEST_TIMEOUT_SEC + 1.0
877887
server_side_timeout = (TEST_TIMEOUT_SEC * 1000).to_i
878-
@client&.blocking_call(client_side_timeout, 'WAIT', TEST_REPLICA_SIZE, server_side_timeout)
888+
swap_timeout(@client, timeout: 0.1) do |client|
889+
client&.blocking_call(client_side_timeout, 'WAIT', TEST_REPLICA_SIZE, server_side_timeout)
890+
end
879891
end
880892

881893
def collect_messages(pubsub, size:, max_attempts: 30, timeout: 1.0)
@@ -912,6 +924,7 @@ def new_test_client(custom: { captured_commands: @captured_commands }, middlewar
912924
config = ::RedisClient::ClusterConfig.new(
913925
nodes: TEST_NODE_URIS,
914926
fixed_hostname: TEST_FIXED_HOSTNAME,
927+
slow_command_timeout: TEST_TIMEOUT_SEC,
915928
middlewares: middlewares,
916929
custom: custom,
917930
**TEST_GENERIC_OPTIONS,
@@ -930,6 +943,7 @@ def new_test_client(custom: { captured_commands: @captured_commands }, middlewar
930943
replica: true,
931944
replica_affinity: :random,
932945
fixed_hostname: TEST_FIXED_HOSTNAME,
946+
slow_command_timeout: TEST_TIMEOUT_SEC,
933947
middlewares: middlewares,
934948
custom: custom,
935949
**TEST_GENERIC_OPTIONS,
@@ -948,6 +962,7 @@ def new_test_client(custom: { captured_commands: @captured_commands }, middlewar
948962
replica: true,
949963
replica_affinity: :random_with_primary,
950964
fixed_hostname: TEST_FIXED_HOSTNAME,
965+
slow_command_timeout: TEST_TIMEOUT_SEC,
951966
middlewares: middlewares,
952967
custom: custom,
953968
**TEST_GENERIC_OPTIONS,
@@ -966,6 +981,7 @@ def new_test_client(custom: { captured_commands: @captured_commands }, middlewar
966981
replica: true,
967982
replica_affinity: :latency,
968983
fixed_hostname: TEST_FIXED_HOSTNAME,
984+
slow_command_timeout: TEST_TIMEOUT_SEC,
969985
middlewares: middlewares,
970986
custom: custom,
971987
**TEST_GENERIC_OPTIONS,
@@ -982,6 +998,7 @@ def new_test_client(custom: { captured_commands: @captured_commands }, middlewar
982998
config = ::RedisClient::ClusterConfig.new(
983999
nodes: TEST_NODE_URIS,
9841000
fixed_hostname: TEST_FIXED_HOSTNAME,
1001+
slow_command_timeout: TEST_TIMEOUT_SEC,
9851002
middlewares: middlewares,
9861003
custom: custom,
9871004
**TEST_GENERIC_OPTIONS,

‎test/test_against_cluster_broken.rb‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,9 @@ def test_a_primary_is_down
4040
def wait_for_replication
4141
client_side_timeout = TEST_TIMEOUT_SEC + 1.0
4242
server_side_timeout = (TEST_TIMEOUT_SEC * 1000).to_i
43-
@client.blocking_call(client_side_timeout, 'WAIT', TEST_REPLICA_SIZE, server_side_timeout)
43+
swap_timeout(@client, timeout: 0.1) do |client|
44+
client.blocking_call(client_side_timeout, 'WAIT', TEST_REPLICA_SIZE, server_side_timeout)
45+
end
4446
end
4547

4648
def do_test_a_node_is_down(sacrifice, number_of_keys:)

‎test/test_against_cluster_scale.rb‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,9 @@ def test_02_scale_in
7171
def wait_for_replication
7272
client_side_timeout = TEST_TIMEOUT_SEC + 1.0
7373
server_side_timeout = (TEST_TIMEOUT_SEC * 1000).to_i
74-
@client.blocking_call(client_side_timeout, 'WAIT', TEST_REPLICA_SIZE, server_side_timeout)
74+
swap_timeout(@client, timeout: 0.1) do |client|
75+
client.blocking_call(client_side_timeout, 'WAIT', TEST_REPLICA_SIZE, server_side_timeout)
76+
end
7577
end
7678

7779
def build_cluster_controller(nodes, shard_size:)

‎test/test_against_cluster_state.rb‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,11 @@ def test_the_state_of_cluster_resharding_with_pipelining_on_new_connection
163163
def wait_for_replication
164164
client_side_timeout = TEST_TIMEOUT_SEC + 1.0
165165
server_side_timeout = (TEST_TIMEOUT_SEC * 1000).to_i
166-
@client.blocking_call(client_side_timeout, 'WAIT', TEST_REPLICA_SIZE, server_side_timeout)
166+
swap_timeout(@client, timeout: 0.1) do |client|
167+
client.blocking_call(client_side_timeout, 'WAIT', TEST_REPLICA_SIZE, server_side_timeout)
168+
rescue RedisClient::Cluster::ErrorCollection => e
169+
raise unless e.errors.values.all? { |err| err.message.start_with?('ERR WAIT cannot be used with replica instances') }
170+
end
167171
end
168172

169173
def fetch_cluster_info(key)

0 commit comments

Comments
 (0)