Skip to content

Commit aa38fc4

Browse files
test(maint-notifications): reshard-aware cluster scale tests (add/remove) + generic node_add
The FI's SLOT_MIGRATE_ASM toggle routes the generic cluster tests through the ASM `reshard` trigger. ASM scale (add/remove) is a cascade of SMIGRATING/SMIGRATED, so the client reconciles its node cache over several notifications after the op completes: - Add a `_wait_for_node_cache_delta` settle-poll helper; use it in node_remove and the new node_add test before the node-delta assertion (single-step migrate/failover return immediately). Relax node_remove reconnect count to >= 1 (a cascade may touch more than one). - Add generic `test_notification_handling_with_node_add` (+1) mirroring node_remove. - Skip `test_new_connections_receive_last_smigrating_smigrated_notification` for reshard add/remove (catching the in-flight notification on a new connection is a single-window property; ASM scale is a multi-window cascade). slot-shuffle keeps a single window.
1 parent 74977e4 commit aa38fc4

1 file changed

Lines changed: 164 additions & 9 deletions

File tree

tests/test_scenario/test_maint_notifications.py

Lines changed: 164 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1416,6 +1416,32 @@ def setup_env(
14161416
)
14171417
return self._client, cluster_endpoint_config
14181418

1419+
def _wait_for_node_cache_delta(
1420+
self, client, connections, initial_count, expected_delta, timeout=SMIGRATED_TIMEOUT
1421+
):
1422+
"""Poll the client node cache until it reflects the expected +/- delta.
1423+
1424+
A topology change may arrive as several push notifications (in particular an ASM scale
1425+
add/remove is a cascade of SMIGRATING/SMIGRATED as the decision engine moves slots), so
1426+
the client reconciles its node cache over multiple notifications after the server-side
1427+
operation finishes. Drain pending notifications on the given connections and poll until
1428+
the cache reaches initial_count + expected_delta, or the timeout elapses. Single-step
1429+
triggers (migrate/failover) reach the target immediately and return on the first check.
1430+
"""
1431+
expected = initial_count + expected_delta
1432+
deadline = time.time() + timeout
1433+
while time.time() < deadline:
1434+
if len(client.nodes_manager.nodes_cache) == expected:
1435+
return
1436+
for conn in connections:
1437+
try:
1438+
if conn._sock and conn.can_read(timeout=0.2):
1439+
conn.read_response(push_request=True)
1440+
except Exception:
1441+
# A connection to a node being removed may error; that is expected.
1442+
pass
1443+
time.sleep(1)
1444+
14191445
@pytest.fixture(autouse=True)
14201446
def setup_and_cleanup(
14211447
self,
@@ -1765,6 +1791,18 @@ def test_notification_handling_with_node_remove(
17651791
connection=con_to_read_smigrated,
17661792
)
17671793

1794+
logging.info("Waiting for the operation to finish server-side...")
1795+
trigger_effect_thread.join()
1796+
self.maintenance_ops_threads.remove(trigger_effect_thread)
1797+
1798+
logging.info("Draining notifications until the node cache reflects -1 node...")
1799+
self._wait_for_node_cache_delta(
1800+
cluster_client_maint_notifications,
1801+
list(in_use_connections.values()),
1802+
len(initial_cluster_nodes),
1803+
expected_delta=-1,
1804+
)
1805+
17681806
logging.info("Validating connection state after SMIGRATED ...")
17691807

17701808
updated_cluster_nodes = (
@@ -1788,25 +1826,129 @@ def test_notification_handling_with_node_remove(
17881826
assert conn is not None
17891827
assert conn.should_reconnect() is True
17901828

1791-
# validate no other connections are marked for reconnect
1792-
marked_conns_for_reconnect = 0
1793-
for conn in in_use_connections.values():
1794-
if conn.should_reconnect():
1795-
marked_conns_for_reconnect += 1
1796-
# only one connection should be marked for reconnect
1797-
# onle the one that belongs to the node that was from
1798-
# the src address of the maintenance
1799-
assert marked_conns_for_reconnect == 1
1829+
# At least the removed node's connection is marked for reconnect. A single-step migrate
1830+
# marks exactly one; an ASM scale-in cascade may touch more, so accept >= 1.
1831+
marked_conns_for_reconnect = sum(
1832+
1 for conn in in_use_connections.values() if conn.should_reconnect()
1833+
)
1834+
assert marked_conns_for_reconnect >= 1
18001835

18011836
logging.info("Releasing connections back to the pool...")
18021837
for node, conn in in_use_connections.items():
18031838
if node.redis_connection is None:
18041839
continue
18051840
node.redis_connection.connection_pool.release(conn)
18061841

1842+
@pytest.mark.timeout(300) # 5 minutes timeout for this test
1843+
@pytest.mark.parametrize(
1844+
"effect_name, trigger, db_config, db_name",
1845+
generate_params(
1846+
_FAULT_INJECTOR_CLIENT_OSS_API,
1847+
[
1848+
SlotMigrateEffects.ADD,
1849+
],
1850+
),
1851+
)
1852+
def test_notification_handling_with_node_add(
1853+
self,
1854+
fault_injector_client_oss_api: FaultInjectorClient,
1855+
effect_name: SlotMigrateEffects,
1856+
trigger: str,
1857+
db_config: dict[str, Any],
1858+
db_name: str,
1859+
):
1860+
"""
1861+
Test push notifications when a cluster operation moves slots such that a previously
1862+
empty node gains a master shard / endpoint (the client's node cache grows by one).
1863+
Mirrors test_notification_handling_with_node_remove for the opposite (+1) delta.
1864+
"""
1865+
logging.info(f"DB name: {db_name}")
1866+
1867+
cluster_client_maint_notifications, cluster_endpoint_config = self.setup_env(
1868+
fault_injector_client_oss_api, db_config
1869+
)
1870+
1871+
logging.info("Creating one connection in each node's pool.")
1872+
initial_cluster_nodes = (
1873+
cluster_client_maint_notifications.nodes_manager.nodes_cache.copy()
1874+
)
1875+
in_use_connections = {}
1876+
for node in initial_cluster_nodes.values():
1877+
in_use_connections[node] = (
1878+
node.redis_connection.connection_pool.get_connection()
1879+
)
1880+
1881+
logging.info("Executing FI command that triggers the desired effect...")
1882+
trigger_effect_thread = Thread(
1883+
target=self._trigger_effect,
1884+
name="trigger_effect_thread",
1885+
args=(
1886+
fault_injector_client_oss_api,
1887+
cluster_endpoint_config,
1888+
effect_name,
1889+
trigger,
1890+
),
1891+
)
1892+
self.maintenance_ops_threads.append(trigger_effect_thread)
1893+
trigger_effect_thread.start()
1894+
1895+
logging.info("Waiting for SMIGRATING push notifications in all connections...")
1896+
for conn in in_use_connections.values():
1897+
ClientValidations.wait_push_notification(
1898+
cluster_client_maint_notifications,
1899+
timeout=int(SLOT_SHUFFLE_TIMEOUT / 2),
1900+
connection=conn,
1901+
)
1902+
1903+
logging.info("Validating connection maintenance state...")
1904+
for conn in in_use_connections.values():
1905+
assert conn.maintenance_state == MaintenanceState.MAINTENANCE
1906+
assert conn._sock.gettimeout() == RELAXED_TIMEOUT
1907+
assert conn.should_reconnect() is False
1908+
1909+
logging.info("Waiting for SMIGRATED push notifications...")
1910+
con_to_read_smigrated = random.choice(list(in_use_connections.values()))
1911+
ClientValidations.wait_push_notification(
1912+
cluster_client_maint_notifications,
1913+
timeout=SMIGRATED_TIMEOUT,
1914+
connection=con_to_read_smigrated,
1915+
)
1916+
1917+
logging.info("Waiting for the operation to finish server-side...")
18071918
trigger_effect_thread.join()
18081919
self.maintenance_ops_threads.remove(trigger_effect_thread)
18091920

1921+
logging.info("Draining notifications until the node cache reflects +1 node...")
1922+
self._wait_for_node_cache_delta(
1923+
cluster_client_maint_notifications,
1924+
list(in_use_connections.values()),
1925+
len(initial_cluster_nodes),
1926+
expected_delta=1,
1927+
)
1928+
1929+
logging.info("Validating a node was added to the client's node cache...")
1930+
updated_cluster_nodes = (
1931+
cluster_client_maint_notifications.nodes_manager.nodes_cache.copy()
1932+
)
1933+
added_nodes = set(updated_cluster_nodes.values()) - set(
1934+
initial_cluster_nodes.values()
1935+
)
1936+
assert len(added_nodes) == 1
1937+
assert len(updated_cluster_nodes) == len(initial_cluster_nodes) + 1
1938+
1939+
# An add moves slots FROM an existing shard onto the previously empty node, so the
1940+
# connection to that source shard is marked for reconnect.
1941+
marked_conns_for_reconnect = sum(
1942+
1 for conn in in_use_connections.values() if conn.should_reconnect()
1943+
)
1944+
assert marked_conns_for_reconnect >= 1
1945+
1946+
logging.info("Releasing connections back to the pool...")
1947+
for node, conn in in_use_connections.items():
1948+
if node.redis_connection is None:
1949+
continue
1950+
node.redis_connection.connection_pool.release(conn)
1951+
18101952
@pytest.mark.timeout(300) # 5 minutes timeout for this test
18111953
@pytest.mark.skipif(
18121954
use_mock_proxy(),
@@ -1841,6 +1983,19 @@ def test_new_connections_receive_last_smigrating_smigrated_notification(
18411983
"""
18421984
logging.info(f"DB name: {db_name}")
18431985

1986+
# A new connection catching the in-flight SMIGRATING relies on a single maintenance
1987+
# window. ASM scale (add/remove) is a cascade of SMIGRATING/SMIGRATED windows, so a new
1988+
# connection made between windows sees no maintenance state - the check is racy.
1989+
# slot-shuffle (single migrate_slots) keeps a single window and is exercised here.
1990+
if trigger == "reshard" and effect_name in (
1991+
SlotMigrateEffects.ADD,
1992+
SlotMigrateEffects.REMOVE,
1993+
):
1994+
pytest.skip(
1995+
"new-connection-catches-window is a single-window property; ASM scale "
1996+
"(reshard add/remove) is a multi-window cascade"
1997+
)
1998+
18441999
cluster_client_maint_notifications, cluster_endpoint_config = self.setup_env(
18452000
fault_injector_client_oss_api, db_config
18462001
)

0 commit comments

Comments
 (0)