-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Hitless handshake #3735
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Hitless handshake #3735
Changes from all commits
092e33b
41a199e
63d0c45
5c71733
96c6e5d
8691475
7b57a22
bed2e40
0744ee5
4c536f3
6768d5d
ce31ec7
d73cd35
788cf52
6d496f0
a8ba5ce
2d3731f
d2288e9
b294db2
a82cbfe
2cdfa75
822fccd
2736aaa
1e2b96d
fb487c0
67bbee9
70688cc
f9eec35
5fd2ddb
e8785de
c3caf6a
8d7cc00
76eba1b
579d032
8d27a86
32a16f0
8bfdf13
33d7295
346097f
f3a9a71
c0438c8
6ca514f
778abdf
667109b
ef1742a
7b43890
08f1585
9a31a71
602bbe9
953b41a
2210fed
1427d99
a2744f3
260b34e
4c6eb44
10ded34
241be74
ba7cb87
c01d331
7a2c3fc
5024b5f
5fd93c5
efa9852
8a6402f
07402d0
b9afaf0
058be2c
e4a8646
51d24ba
66c1fe0
67aee8c
a7dd150
1388cb9
97db940
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -282,6 +282,13 @@ def should_reconnect(self): | |||||||||||||||||||||
""" | ||||||||||||||||||||||
pass | ||||||||||||||||||||||
|
||||||||||||||||||||||
@abstractmethod | ||||||||||||||||||||||
def get_resolved_ip(self): | ||||||||||||||||||||||
""" | ||||||||||||||||||||||
Get resolved ip address for the connection. | ||||||||||||||||||||||
""" | ||||||||||||||||||||||
pass | ||||||||||||||||||||||
|
||||||||||||||||||||||
@abstractmethod | ||||||||||||||||||||||
def update_current_socket_timeout(self, relax_timeout: Optional[float] = None): | ||||||||||||||||||||||
""" | ||||||||||||||||||||||
|
@@ -421,32 +428,16 @@ def __init__( | |||||||||||||||||||||
parser_class = _RESP3Parser | ||||||||||||||||||||||
self.set_parser(parser_class) | ||||||||||||||||||||||
|
||||||||||||||||||||||
if maintenance_events_config and maintenance_events_config.enabled: | ||||||||||||||||||||||
if maintenance_events_pool_handler: | ||||||||||||||||||||||
maintenance_events_pool_handler.set_connection(self) | ||||||||||||||||||||||
self._parser.set_node_moving_push_handler( | ||||||||||||||||||||||
maintenance_events_pool_handler.handle_event | ||||||||||||||||||||||
) | ||||||||||||||||||||||
self._maintenance_event_connection_handler = ( | ||||||||||||||||||||||
MaintenanceEventConnectionHandler(self, maintenance_events_config) | ||||||||||||||||||||||
) | ||||||||||||||||||||||
self._parser.set_maintenance_push_handler( | ||||||||||||||||||||||
self._maintenance_event_connection_handler.handle_event | ||||||||||||||||||||||
) | ||||||||||||||||||||||
self.maintenance_events_config = maintenance_events_config | ||||||||||||||||||||||
|
||||||||||||||||||||||
# Set up maintenance events if enabled | ||||||||||||||||||||||
self._configure_maintenance_events( | ||||||||||||||||||||||
maintenance_events_pool_handler, | ||||||||||||||||||||||
orig_host_address, | ||||||||||||||||||||||
orig_socket_timeout, | ||||||||||||||||||||||
orig_socket_connect_timeout, | ||||||||||||||||||||||
) | ||||||||||||||||||||||
|
||||||||||||||||||||||
self.orig_host_address = ( | ||||||||||||||||||||||
orig_host_address if orig_host_address else self.host | ||||||||||||||||||||||
) | ||||||||||||||||||||||
self.orig_socket_timeout = ( | ||||||||||||||||||||||
orig_socket_timeout if orig_socket_timeout else self.socket_timeout | ||||||||||||||||||||||
) | ||||||||||||||||||||||
self.orig_socket_connect_timeout = ( | ||||||||||||||||||||||
orig_socket_connect_timeout | ||||||||||||||||||||||
if orig_socket_connect_timeout | ||||||||||||||||||||||
else self.socket_connect_timeout | ||||||||||||||||||||||
) | ||||||||||||||||||||||
else: | ||||||||||||||||||||||
self._maintenance_event_connection_handler = None | ||||||||||||||||||||||
self._should_reconnect = False | ||||||||||||||||||||||
self.maintenance_state = maintenance_state | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
@@ -505,6 +496,46 @@ def set_parser(self, parser_class): | |||||||||||||||||||||
""" | ||||||||||||||||||||||
self._parser = parser_class(socket_read_size=self._socket_read_size) | ||||||||||||||||||||||
|
||||||||||||||||||||||
def _configure_maintenance_events( | ||||||||||||||||||||||
self, | ||||||||||||||||||||||
maintenance_events_pool_handler=None, | ||||||||||||||||||||||
orig_host_address=None, | ||||||||||||||||||||||
orig_socket_timeout=None, | ||||||||||||||||||||||
orig_socket_connect_timeout=None, | ||||||||||||||||||||||
): | ||||||||||||||||||||||
"""Enable maintenance events by setting up handlers and storing original connection parameters.""" | ||||||||||||||||||||||
if ( | ||||||||||||||||||||||
not self.maintenance_events_config | ||||||||||||||||||||||
or not self.maintenance_events_config.enabled | ||||||||||||||||||||||
): | ||||||||||||||||||||||
self._maintenance_event_connection_handler = None | ||||||||||||||||||||||
return | ||||||||||||||||||||||
|
||||||||||||||||||||||
# Set up pool handler if available | ||||||||||||||||||||||
if maintenance_events_pool_handler: | ||||||||||||||||||||||
self._parser.set_node_moving_push_handler( | ||||||||||||||||||||||
maintenance_events_pool_handler.handle_event | ||||||||||||||||||||||
) | ||||||||||||||||||||||
|
||||||||||||||||||||||
# Set up connection handler | ||||||||||||||||||||||
self._maintenance_event_connection_handler = MaintenanceEventConnectionHandler( | ||||||||||||||||||||||
self, self.maintenance_events_config | ||||||||||||||||||||||
) | ||||||||||||||||||||||
self._parser.set_maintenance_push_handler( | ||||||||||||||||||||||
self._maintenance_event_connection_handler.handle_event | ||||||||||||||||||||||
) | ||||||||||||||||||||||
|
||||||||||||||||||||||
# Store original connection parameters | ||||||||||||||||||||||
self.orig_host_address = orig_host_address if orig_host_address else self.host | ||||||||||||||||||||||
self.orig_socket_timeout = ( | ||||||||||||||||||||||
orig_socket_timeout if orig_socket_timeout else self.socket_timeout | ||||||||||||||||||||||
) | ||||||||||||||||||||||
self.orig_socket_connect_timeout = ( | ||||||||||||||||||||||
orig_socket_connect_timeout | ||||||||||||||||||||||
if orig_socket_connect_timeout | ||||||||||||||||||||||
else self.socket_connect_timeout | ||||||||||||||||||||||
) | ||||||||||||||||||||||
|
||||||||||||||||||||||
def set_maintenance_event_pool_handler( | ||||||||||||||||||||||
self, maintenance_event_pool_handler: MaintenanceEventPoolHandler | ||||||||||||||||||||||
): | ||||||||||||||||||||||
|
@@ -652,6 +683,39 @@ def on_connect_check_health(self, check_health: bool = True): | |||||||||||||||||||||
): | ||||||||||||||||||||||
raise ConnectionError("Invalid RESP version") | ||||||||||||||||||||||
|
||||||||||||||||||||||
# Send maintenance notifications handshake if RESP3 is active and maintenance events are enabled | ||||||||||||||||||||||
# and we have a host to determine the endpoint type from | ||||||||||||||||||||||
if ( | ||||||||||||||||||||||
self.protocol not in [2, "2"] | ||||||||||||||||||||||
and self.maintenance_events_config | ||||||||||||||||||||||
and self.maintenance_events_config.enabled | ||||||||||||||||||||||
and self._maintenance_event_connection_handler | ||||||||||||||||||||||
and hasattr(self, "host") | ||||||||||||||||||||||
): | ||||||||||||||||||||||
try: | ||||||||||||||||||||||
endpoint_type = self.maintenance_events_config.get_endpoint_type( | ||||||||||||||||||||||
self.host, self | ||||||||||||||||||||||
) | ||||||||||||||||||||||
self.send_command( | ||||||||||||||||||||||
"CLIENT", | ||||||||||||||||||||||
"MAINT_NOTIFICATIONS", | ||||||||||||||||||||||
"ON", | ||||||||||||||||||||||
"moving-endpoint-type", | ||||||||||||||||||||||
endpoint_type.value, | ||||||||||||||||||||||
check_health=check_health, | ||||||||||||||||||||||
) | ||||||||||||||||||||||
response = self.read_response() | ||||||||||||||||||||||
if str_if_bytes(response) != "OK": | ||||||||||||||||||||||
raise ConnectionError( | ||||||||||||||||||||||
"The server doesn't support maintenance notifications" | ||||||||||||||||||||||
) | ||||||||||||||||||||||
except Exception as e: | ||||||||||||||||||||||
# Log warning but don't fail the connection | ||||||||||||||||||||||
import logging | ||||||||||||||||||||||
|
||||||||||||||||||||||
logger = logging.getLogger(__name__) | ||||||||||||||||||||||
logger.warning(f"Failed to enable maintenance notifications: {e}") | ||||||||||||||||||||||
|
||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The broad exception handling catches all exceptions during the handshake process. Consider being more specific about which exceptions to handle and potentially re-raise authentication or connection errors that shouldn't be silently ignored.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||||||||||||||
# if a client_name is given, set it | ||||||||||||||||||||||
if self.client_name: | ||||||||||||||||||||||
self.send_command( | ||||||||||||||||||||||
|
@@ -888,6 +952,56 @@ def re_auth(self): | |||||||||||||||||||||
self.read_response() | ||||||||||||||||||||||
self._re_auth_token = None | ||||||||||||||||||||||
|
||||||||||||||||||||||
def get_resolved_ip(self) -> Optional[str]: | ||||||||||||||||||||||
""" | ||||||||||||||||||||||
Extract the resolved IP address from an | ||||||||||||||||||||||
established connection or resolve it from the host. | ||||||||||||||||||||||
|
||||||||||||||||||||||
First tries to get the actual IP from the socket (most accurate), | ||||||||||||||||||||||
then falls back to DNS resolution if needed. | ||||||||||||||||||||||
|
||||||||||||||||||||||
Args: | ||||||||||||||||||||||
connection: The connection object to extract the IP from | ||||||||||||||||||||||
|
||||||||||||||||||||||
Returns: | ||||||||||||||||||||||
str: The resolved IP address, or None if it cannot be determined | ||||||||||||||||||||||
""" | ||||||||||||||||||||||
|
||||||||||||||||||||||
# Method 1: Try to get the actual IP from the established socket connection | ||||||||||||||||||||||
# This is most accurate as it shows the exact IP being used | ||||||||||||||||||||||
try: | ||||||||||||||||||||||
if self._sock is not None: | ||||||||||||||||||||||
peer_addr = self._sock.getpeername() | ||||||||||||||||||||||
if peer_addr and len(peer_addr) >= 1: | ||||||||||||||||||||||
# For TCP sockets, peer_addr is typically (host, port) tuple | ||||||||||||||||||||||
# Return just the host part | ||||||||||||||||||||||
return peer_addr[0] | ||||||||||||||||||||||
except (AttributeError, OSError): | ||||||||||||||||||||||
# Socket might not be connected or getpeername() might fail | ||||||||||||||||||||||
pass | ||||||||||||||||||||||
|
||||||||||||||||||||||
# Method 2: Fallback to DNS resolution of the host | ||||||||||||||||||||||
# This is less accurate but works when socket is not available | ||||||||||||||||||||||
try: | ||||||||||||||||||||||
host = getattr(self, "host", "localhost") | ||||||||||||||||||||||
port = getattr(self, "port", 6379) | ||||||||||||||||||||||
if host: | ||||||||||||||||||||||
# Use getaddrinfo to resolve the hostname to IP | ||||||||||||||||||||||
# This mimics what the connection would do during _connect() | ||||||||||||||||||||||
addr_info = socket.getaddrinfo( | ||||||||||||||||||||||
host, port, socket.AF_UNSPEC, socket.SOCK_STREAM | ||||||||||||||||||||||
) | ||||||||||||||||||||||
if addr_info: | ||||||||||||||||||||||
# Return the IP from the first result | ||||||||||||||||||||||
# addr_info[0] is (family, socktype, proto, canonname, sockaddr) | ||||||||||||||||||||||
# sockaddr[0] is the IP address | ||||||||||||||||||||||
return addr_info[0][4][0] | ||||||||||||||||||||||
except (AttributeError, OSError, socket.gaierror): | ||||||||||||||||||||||
# DNS resolution might fail | ||||||||||||||||||||||
pass | ||||||||||||||||||||||
|
||||||||||||||||||||||
return None | ||||||||||||||||||||||
|
||||||||||||||||||||||
@property | ||||||||||||||||||||||
def maintenance_state(self) -> MaintenanceState: | ||||||||||||||||||||||
return self._maintenance_state | ||||||||||||||||||||||
|
Uh oh!
There was an error while loading. Please reload this page.