Skip to content

Commit 7cae6a1

Browse files
committed
cluster: reduce per-request lock overhead in ResponseFuture
Lazy Event: defer Event() creation until result() is actually called. For execute_concurrent (which never calls result()), this eliminates ~620ns of Event construction + Event.set() per request. Merged add_callbacks: register both callback and errback under a single _callback_lock acquisition instead of two separate ones (~80ns saved). _set_final_result/_set_final_exception: capture _event reference under _callback_lock for free-threaded Python safety; skip .set() when Event was never created. _wait_for_result: check result availability under _callback_lock before creating Event, avoiding Event creation entirely when the result arrived before the caller waits. _on_speculative_execute: check _final_result/_final_exception directly instead of relying on Event.is_set(), since Event may be None. All changes are safe under both GIL and free-threaded (PEP 703) Python.
1 parent b759d4a commit 7cae6a1

2 files changed

Lines changed: 97 additions & 41 deletions

File tree

cassandra/cluster.py

Lines changed: 95 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -4461,7 +4461,7 @@ def __init__(self, session, message, query, timeout, metrics=None, prepared_stat
44614461
self._host = host
44624462
self._spec_execution_plan = speculative_execution_plan or self._spec_execution_plan
44634463
self._make_query_plan()
4464-
self._event = Event()
4464+
self._event = None # lazily created on first wait/check
44654465
self._errors = {}
44664466
self._callbacks = []
44674467
self._errbacks = []
@@ -4546,25 +4546,31 @@ def _on_timeout(self, _attempts=0):
45464546

45474547
def _on_speculative_execute(self):
45484548
self._timer = None
4549-
if not self._event.is_set():
4550-
4551-
# PYTHON-836, the speculative queries must be after
4552-
# the query is sent from the main thread, otherwise the
4553-
# query from the main thread may raise NoHostAvailable
4554-
# if the _query_plan has been exhausted by the specualtive queries.
4555-
# This also prevents a race condition accessing the iterator.
4556-
# We reschedule this call until the main thread has succeeded
4557-
# making a query
4558-
if not self.attempted_hosts:
4559-
self._timer = self.session.cluster.connection_class.create_timer(0.01, self._on_speculative_execute)
4560-
return
4549+
# Unlocked reads: under free-threaded Python a stale read may
4550+
# allow a harmless redundant speculative query — acceptable
4551+
# because the duplicate response will be discarded.
4552+
if self._final_result is not _NOT_SET or self._final_exception is not None:
4553+
return # already done
4554+
if self._event is not None and self._event.is_set():
4555+
return # already done (Event path)
4556+
4557+
# PYTHON-836, the speculative queries must be after
4558+
# the query is sent from the main thread, otherwise the
4559+
# query from the main thread may raise NoHostAvailable
4560+
# if the _query_plan has been exhausted by the specualtive queries.
4561+
# This also prevents a race condition accessing the iterator.
4562+
# We reschedule this call until the main thread has succeeded
4563+
# making a query
4564+
if not self.attempted_hosts:
4565+
self._timer = self.session.cluster.connection_class.create_timer(0.01, self._on_speculative_execute)
4566+
return
45614567

4562-
if self._time_remaining is not None:
4563-
if self._time_remaining <= 0:
4564-
self._on_timeout()
4565-
return
4566-
self.send_request(error_no_hosts=False)
4567-
self._start_timer()
4568+
if self._time_remaining is not None:
4569+
if self._time_remaining <= 0:
4570+
self._on_timeout()
4571+
return
4572+
self.send_request(error_no_hosts=False)
4573+
self._start_timer()
45684574

45694575
def _make_query_plan(self):
45704576
# set the query_plan according to the load balancing policy,
@@ -4669,7 +4675,7 @@ def warnings(self):
46694675
Otherwise it may throw if the response has not been received.
46704676
"""
46714677
# TODO: When timers are introduced, just make this wait
4672-
if not self._event.is_set():
4678+
if self._event is None or not self._event.is_set():
46734679
raise DriverException("warnings cannot be retrieved before ResponseFuture is finalized")
46744680
return self._warnings
46754681

@@ -4687,7 +4693,7 @@ def custom_payload(self):
46874693
:return: :ref:`custom_payload`.
46884694
"""
46894695
# TODO: When timers are introduced, just make this wait
4690-
if not self._event.is_set():
4696+
if self._event is None or not self._event.is_set():
46914697
raise DriverException("custom_payload cannot be retrieved before ResponseFuture is finalized")
46924698
return self._custom_payload
46934699

@@ -4706,7 +4712,8 @@ def start_fetching_next_page(self):
47064712

47074713
self._make_query_plan()
47084714
self.message.paging_state = self._paging_state
4709-
self._event.clear()
4715+
if self._event is not None:
4716+
self._event.clear()
47104717
self._final_result = _NOT_SET
47114718
self._final_exception = None
47124719
self._start_timer()
@@ -4967,16 +4974,23 @@ def _set_final_result(self, response):
49674974
# -- prevents case where _final_result is set, then a callback is
49684975
# added and executed on the spot, then executed again as a
49694976
# registered callback
4970-
to_call = tuple(
4971-
partial(fn, response, *args, **kwargs)
4972-
for (fn, args, kwargs) in self._callbacks
4973-
)
4977+
callbacks = self._callbacks
4978+
if callbacks:
4979+
to_call = tuple(
4980+
partial(fn, response, *args, **kwargs)
4981+
for (fn, args, kwargs) in callbacks
4982+
)
4983+
else:
4984+
to_call = None
4985+
event = self._event # capture under lock for free-threaded safety
49744986

4975-
self._event.set()
4987+
if event is not None:
4988+
event.set()
49764989

49774990
# apply each callback
4978-
for callback_partial in to_call:
4979-
callback_partial()
4991+
if to_call:
4992+
for callback_partial in to_call:
4993+
callback_partial()
49804994

49814995
def _set_final_exception(self, response):
49824996
self._cancel_timer()
@@ -4989,15 +5003,22 @@ def _set_final_exception(self, response):
49895003
# prevents case where _final_exception is set, then an errback is
49905004
# added and executed on the spot, then executed again as a
49915005
# registered errback
4992-
to_call = tuple(
4993-
partial(fn, response, *args, **kwargs)
4994-
for (fn, args, kwargs) in self._errbacks
4995-
)
4996-
self._event.set()
5006+
errbacks = self._errbacks
5007+
if errbacks:
5008+
to_call = tuple(
5009+
partial(fn, response, *args, **kwargs)
5010+
for (fn, args, kwargs) in errbacks
5011+
)
5012+
else:
5013+
to_call = None
5014+
event = self._event # capture under lock for free-threaded safety
5015+
if event is not None:
5016+
event.set()
49975017

49985018
# apply each callback
4999-
for callback_partial in to_call:
5000-
callback_partial()
5019+
if to_call:
5020+
for callback_partial in to_call:
5021+
callback_partial()
50015022

50025023
def _handle_retry_decision(self, retry_decision, response, host):
50035024

@@ -5076,9 +5097,25 @@ def result(self):
50765097
... log.exception("Operation failed:")
50775098
50785099
"""
5079-
self._event.wait()
5100+
return ResultSet(self, self._wait_for_result())
5101+
5102+
def _wait_for_result(self):
5103+
# Check under lock whether the result is already available.
5104+
# If so, we can skip Event creation entirely.
5105+
with self._callback_lock:
5106+
if self._final_result is not _NOT_SET:
5107+
return self._final_result
5108+
if self._final_exception is not None:
5109+
raise self._final_exception
5110+
# Result not yet available — ensure Event exists so that
5111+
# _set_final_result/_set_final_exception (which also hold
5112+
# _callback_lock when capturing _event) will call .set().
5113+
if self._event is None:
5114+
self._event = Event()
5115+
event = self._event
5116+
event.wait()
50805117
if self._final_result is not _NOT_SET:
5081-
return ResultSet(self, self._final_result)
5118+
return self._final_result
50825119
else:
50835120
raise self._final_exception
50845121

@@ -5217,8 +5254,26 @@ def add_callbacks(self, callback, errback,
52175254
... errback=log_error, errback_args=(query,))
52185255
52195256
"""
5220-
self.add_callback(callback, *callback_args, **(callback_kwargs or {}))
5221-
self.add_errback(errback, *errback_args, **(errback_kwargs or {}))
5257+
cb_kwargs = callback_kwargs or {}
5258+
eb_kwargs = errback_kwargs or {}
5259+
run_callback = False
5260+
run_errback = False
5261+
with self._callback_lock:
5262+
if self._callbacks is None:
5263+
self._callbacks = []
5264+
self._callbacks.append((callback, callback_args, cb_kwargs))
5265+
if self._errbacks is None:
5266+
self._errbacks = []
5267+
self._errbacks.append((errback, errback_args, eb_kwargs))
5268+
if self._final_result is not _NOT_SET:
5269+
run_callback = True
5270+
if self._final_exception:
5271+
run_errback = True
5272+
if run_callback:
5273+
callback(self._final_result, *callback_args, **cb_kwargs)
5274+
if run_errback:
5275+
errback(self._final_exception, *errback_args, **eb_kwargs)
5276+
return self
52225277

52235278
def clear_callbacks(self):
52245279
with self._callback_lock:

tests/unit/test_response_future.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,8 @@ def test_multiple_errbacks(self):
530530
result = Mock(spec=UnavailableErrorMessage, info={"required_replicas":2, "alive_replicas": 1, "consistency": 1})
531531
result.to_exception.return_value = expected_exception
532532
rf._set_result(None, None, None, result)
533-
rf._event.set()
533+
if rf._event is not None:
534+
rf._event.set()
534535
with pytest.raises(Exception):
535536
rf.result()
536537

0 commit comments

Comments
 (0)