Skip to content

Commit abfcaf8

Browse files
committed
connection: tolerate clean startup close
1 parent bcc2d3d commit abfcaf8

6 files changed

Lines changed: 202 additions & 16 deletions

File tree

cassandra/cluster.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1753,6 +1753,7 @@ def connection_factory(self, endpoint, host_conn = None, *args, **kwargs):
17531753

17541754
def _make_connection_factory(self, host, *args, **kwargs):
17551755
kwargs = self._make_connection_kwargs(host.endpoint, kwargs)
1756+
kwargs['_raise_on_startup_close'] = True
17561757
return partial(self.connection_class.factory, host.endpoint, self.connect_timeout, *args, **kwargs)
17571758

17581759
def _make_connection_kwargs(self, endpoint, kwargs_dict):

cassandra/connection.py

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -965,10 +965,16 @@ def create_timer(cls, timeout, callback):
965965
@classmethod
966966
def factory(cls, endpoint, timeout, host_conn = None, *args, **kwargs):
967967
"""
968-
A factory function which returns connections which have
969-
succeeded in connecting and are ready for service (or
970-
raises an exception otherwise).
968+
A factory function which returns a connection once startup has
969+
completed, or raises an exception otherwise.
970+
971+
Direct callers may receive a closed connection when the server accepts
972+
the socket and then cleanly closes it during the startup handshake.
973+
Callers that own pool startup or reconnection probes pass ``host_conn``
974+
or ``_raise_on_startup_close`` and receive :exc:`ConnectionShutdown`
975+
for that clean startup close instead.
971976
"""
977+
raise_on_startup_close = kwargs.pop('_raise_on_startup_close', False)
972978
start = time.time()
973979
kwargs['connect_timeout'] = timeout
974980
conn = cls(endpoint, *args, **kwargs)
@@ -986,8 +992,10 @@ def factory(cls, endpoint, timeout, host_conn = None, *args, **kwargs):
986992
conn.close()
987993
raise OperationTimedOut("Timed out creating connection (%s seconds)" % timeout,
988994
timeout=timeout)
989-
elif conn.is_closed:
990-
raise ConnectionShutdown("Connection to %s was closed by server" % conn.endpoint)
995+
elif conn.is_closed and (host_conn is not None or raise_on_startup_close):
996+
raise ConnectionShutdown(
997+
"Connection to %s was closed during the startup handshake" % (endpoint,),
998+
endpoint)
991999
else:
9921000
return conn
9931001

cassandra/pool.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,10 @@ def __init__(self, host, host_distance, session):
426426
return
427427

428428
log.debug("Initializing connection for host %s", self.host)
429-
first_connection = session.cluster.connection_factory(self.host.endpoint, on_orphaned_stream_released=self.on_orphaned_stream_released)
429+
first_connection = session.cluster.connection_factory(
430+
self.host.endpoint,
431+
host_conn=self,
432+
on_orphaned_stream_released=self.on_orphaned_stream_released)
430433
log.debug("First connection created to %s for shard_id=%i", self.host, first_connection.features.shard_id)
431434
self._connections[first_connection.features.shard_id] = first_connection
432435
self._keyspace = session.keyspace
@@ -605,8 +608,10 @@ def _replace(self, connection):
605608
self._connecting.add(connection.features.shard_id)
606609
self._session.submit(self._open_connection_to_missing_shard, connection.features.shard_id)
607610
else:
608-
connection = self._session.cluster.connection_factory(self.host.endpoint,
609-
on_orphaned_stream_released=self.on_orphaned_stream_released)
611+
connection = self._session.cluster.connection_factory(
612+
self.host.endpoint,
613+
host_conn=self,
614+
on_orphaned_stream_released=self.on_orphaned_stream_released)
610615
if self._keyspace:
611616
connection.set_keyspace_blocking(self._keyspace)
612617
self._connections[connection.features.shard_id] = connection

tests/unit/test_cluster.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,8 +276,22 @@ def test_connection_factory_passes_compression_kwarg(self):
276276
assert conn == 'connection'
277277
assert factory.call_count == 1
278278
assert factory.call_args.kwargs['compression'] == expected
279+
assert '_raise_on_startup_close' not in factory.call_args.kwargs
279280
assert cluster.compression == expected
280281

282+
def test_reconnection_factory_rejects_startup_close(self):
283+
endpoint = Mock(address='127.0.0.1')
284+
host = Mock(endpoint=endpoint)
285+
286+
with patch.object(Cluster.connection_class, 'factory', autospec=True, return_value='connection') as factory:
287+
cluster = Cluster()
288+
conn_factory = cluster._make_connection_factory(host)
289+
conn = conn_factory()
290+
291+
assert conn == 'connection'
292+
assert factory.call_count == 1
293+
assert factory.call_args.kwargs['_raise_on_startup_close'] is True
294+
281295

282296
class SchedulerTest(unittest.TestCase):
283297
# TODO: this suite could be expanded; for now just adding a test covering a ticket

tests/unit/test_connection.py

Lines changed: 138 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,11 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414
import itertools
15+
import socket
1516
import unittest
1617
from io import BytesIO
1718
import time
18-
from threading import Lock
19+
from threading import Event, Lock, Thread
1920
from unittest.mock import Mock, ANY, call, patch
2021

2122
from cassandra import OperationTimedOut
@@ -383,6 +384,142 @@ def test_wait_for_responses_shutdown_includes_last_error(self):
383384
assert "already closed" in error_message
384385
assert "Bad file descriptor" in error_message
385386

387+
def test_factory_returns_maintenance_mode_clean_startup_close(self):
388+
"""
389+
Maintenance mode accepts regular CQL sockets and closes them during
390+
startup. Direct callers keep that clean close observable, but
391+
pool-owned startup connections and reconnection probes must be rejected
392+
so unusable connections cannot be stored or reported as host recovery.
393+
"""
394+
395+
class MaintenanceModeCqlServer(object):
396+
def __init__(self):
397+
self._sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
398+
self._sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
399+
self._sock.bind(('127.0.0.1', 0))
400+
self._sock.listen(1)
401+
self._sock.settimeout(2)
402+
self.port = self._sock.getsockname()[1]
403+
self.first_frame = b''
404+
self.ready = Event()
405+
self.received_frame = Event()
406+
self.error = None
407+
self.thread = Thread(target=self._run)
408+
self.thread.daemon = True
409+
self.thread.start()
410+
411+
def _run(self):
412+
self.ready.set()
413+
try:
414+
client, _ = self._sock.accept()
415+
with client:
416+
client.settimeout(2)
417+
while len(self.first_frame) < 9:
418+
chunk = client.recv(9 - len(self.first_frame))
419+
if not chunk:
420+
break
421+
self.first_frame += chunk
422+
except Exception as exc:
423+
self.error = exc
424+
finally:
425+
self.received_frame.set()
426+
427+
def close(self):
428+
self._sock.close()
429+
self.thread.join(2)
430+
431+
class MaintenanceModeConnection(Connection):
432+
def __init__(self, *args, **kwargs):
433+
super(MaintenanceModeConnection, self).__init__(*args, **kwargs)
434+
self._reader = None
435+
self._connect_socket()
436+
self._send_options_message()
437+
self._reader = Thread(target=self._read_until_server_closes)
438+
self._reader.daemon = True
439+
self._reader.start()
440+
441+
def push(self, data):
442+
self._socket.sendall(data)
443+
444+
def close(self):
445+
with self.lock:
446+
if self.is_closed:
447+
return
448+
self.is_closed = True
449+
450+
if self._socket:
451+
self._socket.close()
452+
453+
if not self.is_defunct:
454+
self.error_all_requests(
455+
ConnectionShutdown("Connection to %s was closed" % self.endpoint))
456+
self.connected_event.set()
457+
458+
def _read_until_server_closes(self):
459+
try:
460+
while True:
461+
data = self._socket.recv(self.in_buffer_size)
462+
if not data:
463+
self.close()
464+
return
465+
self._iobuf.write(data)
466+
self.process_io_buffer()
467+
except socket.error as exc:
468+
if not self.is_closed:
469+
self.defunct(exc)
470+
471+
server = MaintenanceModeCqlServer()
472+
try:
473+
assert server.ready.wait(2)
474+
conn = MaintenanceModeConnection.factory(
475+
DefaultEndPoint('127.0.0.1', server.port), timeout=2)
476+
477+
assert conn.is_closed
478+
assert server.received_frame.wait(2)
479+
assert server.error is None
480+
assert server.first_frame[4] == 0x05 # OPTIONS
481+
finally:
482+
server.close()
483+
484+
server = MaintenanceModeCqlServer()
485+
try:
486+
assert server.ready.wait(2)
487+
host_conn = Mock()
488+
host_conn.is_shutdown = False
489+
host_conn._pending_connections = []
490+
491+
with pytest.raises(ConnectionShutdown) as exc_info:
492+
MaintenanceModeConnection.factory(
493+
DefaultEndPoint('127.0.0.1', server.port),
494+
timeout=2,
495+
host_conn=host_conn)
496+
497+
assert "closed during the startup handshake" in str(exc_info.value)
498+
assert server.received_frame.wait(2)
499+
assert server.error is None
500+
assert server.first_frame[4] == 0x05 # OPTIONS
501+
assert len(host_conn._pending_connections) == 1
502+
assert host_conn._pending_connections[0].is_closed
503+
finally:
504+
server.close()
505+
506+
server = MaintenanceModeCqlServer()
507+
try:
508+
assert server.ready.wait(2)
509+
510+
with pytest.raises(ConnectionShutdown) as exc_info:
511+
MaintenanceModeConnection.factory(
512+
DefaultEndPoint('127.0.0.1', server.port),
513+
timeout=2,
514+
_raise_on_startup_close=True)
515+
516+
assert "closed during the startup handshake" in str(exc_info.value)
517+
assert server.received_frame.wait(2)
518+
assert server.error is None
519+
assert server.first_frame[4] == 0x05 # OPTIONS
520+
finally:
521+
server.close()
522+
386523

387524
@patch('cassandra.connection.ConnectionHeartbeat._raise_if_stopped')
388525
class ConnectionHeartbeatTest(unittest.TestCase):

tests/unit/test_host_connection_pool.py

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,10 @@ def test_borrow_and_return(self):
5050
session.cluster.connection_factory.return_value = conn
5151

5252
pool = self.PoolImpl(host, HostDistance.LOCAL, session)
53-
session.cluster.connection_factory.assert_called_once_with(host.endpoint, on_orphaned_stream_released=pool.on_orphaned_stream_released)
53+
session.cluster.connection_factory.assert_called_once_with(
54+
host.endpoint,
55+
host_conn=pool,
56+
on_orphaned_stream_released=pool.on_orphaned_stream_released)
5457

5558
c, request_id = pool.borrow_connection(timeout=0.01)
5659
assert c is conn
@@ -69,7 +72,10 @@ def test_failed_wait_for_connection(self):
6972
session.cluster.connection_factory.return_value = conn
7073

7174
pool = self.PoolImpl(host, HostDistance.LOCAL, session)
72-
session.cluster.connection_factory.assert_called_once_with(host.endpoint, on_orphaned_stream_released=pool.on_orphaned_stream_released)
75+
session.cluster.connection_factory.assert_called_once_with(
76+
host.endpoint,
77+
host_conn=pool,
78+
on_orphaned_stream_released=pool.on_orphaned_stream_released)
7379

7480
pool.borrow_connection(timeout=0.01)
7581
assert 1 == conn.in_flight
@@ -89,7 +95,10 @@ def test_successful_wait_for_connection(self):
8995
session.cluster.connection_factory.return_value = conn
9096

9197
pool = self.PoolImpl(host, HostDistance.LOCAL, session)
92-
session.cluster.connection_factory.assert_called_once_with(host.endpoint, on_orphaned_stream_released=pool.on_orphaned_stream_released)
98+
session.cluster.connection_factory.assert_called_once_with(
99+
host.endpoint,
100+
host_conn=pool,
101+
on_orphaned_stream_released=pool.on_orphaned_stream_released)
93102

94103
pool.borrow_connection(timeout=0.01)
95104
assert 1 == conn.in_flight
@@ -114,7 +123,10 @@ def test_spawn_when_at_max(self):
114123
session.cluster.connection_factory.return_value = conn
115124

116125
pool = self.PoolImpl(host, HostDistance.LOCAL, session)
117-
session.cluster.connection_factory.assert_called_once_with(host.endpoint, on_orphaned_stream_released=pool.on_orphaned_stream_released)
126+
session.cluster.connection_factory.assert_called_once_with(
127+
host.endpoint,
128+
host_conn=pool,
129+
on_orphaned_stream_released=pool.on_orphaned_stream_released)
118130

119131
pool.borrow_connection(timeout=0.01)
120132
assert 1 == conn.in_flight
@@ -138,7 +150,10 @@ def test_return_defunct_connection(self):
138150
session.cluster.connection_factory.return_value = conn
139151

140152
pool = self.PoolImpl(host, HostDistance.LOCAL, session)
141-
session.cluster.connection_factory.assert_called_once_with(host.endpoint, on_orphaned_stream_released=pool.on_orphaned_stream_released)
153+
session.cluster.connection_factory.assert_called_once_with(
154+
host.endpoint,
155+
host_conn=pool,
156+
on_orphaned_stream_released=pool.on_orphaned_stream_released)
142157

143158
pool.borrow_connection(timeout=0.01)
144159
conn.is_defunct = True
@@ -160,7 +175,10 @@ def test_return_defunct_connection_on_down_host(self):
160175
session.cluster.shard_aware_options = ShardAwareOptions()
161176

162177
pool = self.PoolImpl(host, HostDistance.LOCAL, session)
163-
session.cluster.connection_factory.assert_called_once_with(host.endpoint, on_orphaned_stream_released=pool.on_orphaned_stream_released)
178+
session.cluster.connection_factory.assert_called_once_with(
179+
host.endpoint,
180+
host_conn=pool,
181+
on_orphaned_stream_released=pool.on_orphaned_stream_released)
164182

165183
pool.borrow_connection(timeout=0.01)
166184
conn.is_defunct = True
@@ -187,7 +205,10 @@ def test_return_closed_connection(self):
187205
session.cluster.connection_factory.return_value = conn
188206

189207
pool = self.PoolImpl(host, HostDistance.LOCAL, session)
190-
session.cluster.connection_factory.assert_called_once_with(host.endpoint, on_orphaned_stream_released=pool.on_orphaned_stream_released)
208+
session.cluster.connection_factory.assert_called_once_with(
209+
host.endpoint,
210+
host_conn=pool,
211+
on_orphaned_stream_released=pool.on_orphaned_stream_released)
191212

192213
pool.borrow_connection(timeout=0.01)
193214
conn.is_closed = True

0 commit comments

Comments
 (0)