Skip to content

Commit 89b42dd

Browse files
committed
bug redis: disconnect slots on stop in topology holder
`ClusterTopologyHolder::Stop()` was not disconnecting the slots, resulting in new events apearing after the `Stop()`: ``` /home/runner/work/userver/userver/redis/src/storages/redis/impl/cluster_sentinel_impl.cpp:884:13: runtime error: member access within address 0x51100001e040 which does not point to an object of type 'userver_ns::storages::redis::impl::ClusterSentinelImpl' 0x51100001e040: note: object has invalid vptr 04 00 00 00 fb 42 00 00 03 00 00 00 d0 7d 01 00 30 51 00 00 80 39 06 00 60 51 00 00 00 00 00 00 ^~~~~~~ invalid vptr #0 0x5561b8eb5ae5 in userver_ns::v1::storages::redis::impl::ClusterSentinelImpl::Init()::$_0::operator()(std::string, userver_ns::v1::storages::redis::RedisState) const /userver/redis/src/storages/redis/impl/cluster_sentinel_impl.cpp:884:13 ``` Tests: протестировано CI commit_hash:52161d57a664620907e07767dcd8525d75081b4c
1 parent 1e83538 commit 89b42dd

2 files changed

Lines changed: 7 additions & 9 deletions

File tree

redis/functional_tests/cluster_auto_topology_pubsub/tests/test_redis_pubsub.py

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,8 @@
22

33
import redis
44

5-
# Some messages may be lost (it's a Redis limitation)
6-
REDIS_PORT = 6379
7-
REQUESTS_RETRIES = 42
8-
REQUESTS_RETRIES_NOT_EXPECTED = 3
5+
# Some messages may be lost (it's a Redis limitation). *_failover tests require more than 100 retries on slow CI
6+
REQUESTS_RETRIES = 9000
97
REQUESTS_RELAX_TIME = 1.0
108

119
# Target channel. Same constant in c++ code
@@ -89,8 +87,7 @@ async def _test_service_sharded_subscription(
8987

9088
async def _validate_service_publish(service_client, nodes, shards_count=0):
9189
"""
92-
Check publishing by service with 'publish' command in each shard is heard
93-
from any node from any shard
90+
Check publishing by service with 'publish' command in each shard is heard from any node from any shard
9491
"""
9592
redis_clients = [(node, node.get_client()) for node in nodes]
9693

@@ -325,8 +322,7 @@ async def test_cluster_happy_path(service_client, redis_cluster_topology):
325322

326323
async def test_cluster_add_shard(service_client, redis_cluster_topology):
327324
"""
328-
Check service still can receive messages published on any of cluster's node
329-
even after addition of new shard
325+
Check service still can receive messages published on any of cluster's node even after addition of new shard
330326
"""
331327

332328
await service_client.delete('/redis-cluster')

redis/src/storages/redis/impl/cluster_sentinel_impl.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,9 @@ class ClusterTopologyHolder final : public TopologyHolderBase,
282282
}
283283

284284
void Stop() override {
285+
signal_node_state_change_.disconnect_all_slots();
286+
signal_topology_changed_.disconnect_all_slots();
287+
285288
ev_thread_.RunInEvLoopBlocking([this] {
286289
update_topology_timer_.Stop();
287290
explore_nodes_timer_.Stop();
@@ -433,7 +436,6 @@ class ClusterTopologyHolder final : public TopologyHolderBase,
433436
bool IsInitialized() const { return is_nodes_received_.load() && is_topology_received_.load(); }
434437
///}
435438

436-
// NOLINTNEXTLINE(misc-non-private-member-variables-in-classes)
437439
boost::signals2::signal<void(HostPort, Redis::State)> signal_node_state_change_;
438440
boost::signals2::signal<void(size_t shards_count)> signal_topology_changed_;
439441
NodesStorage nodes_;

0 commit comments

Comments
 (0)