Skip to content

Commit 4223351

Browse files
authored
Fix: PineconeGrpcFuture blocks during construction (#478)
## Problem When using grpc with `async_req=True`, the construction of a `PineconeGrpcFuture` would call `_sync_state`, which would do a blocking call to `grpc_future.exception(...)`. This means that the async reqs were all blocking in practice. Describe the purpose of this change. What problem is being solved and why? ## Solution We can fix it by checking if the future is still running and not doing any blocking calls when it is. ## Type of Change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update - [ ] Infrastructure change (CI configs, etc) - [ ] Non-code change (docs, etc) - [ ] None of the above: (explain here) ## Test Plan How I tested it: - Before: Run a long query in a loop and hit ctrl-c. Observe the traceback containing `PineconeGrpcFuture()` - After: Run a long query in a loop and hit ctrl-c. Observe the traceback containing your `.result()` call instead.
2 parents 0027df0 + 15e3fa1 commit 4223351

File tree

2 files changed

+8
-5
lines changed

2 files changed

+8
-5
lines changed

pinecone/grpc/future.py

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,18 +30,17 @@ def _sync_state(self, grpc_future):
3030
if self.done():
3131
return
3232

33-
if grpc_future.cancelled():
33+
if grpc_future.running() and not self.running():
34+
if not self.set_running_or_notify_cancel():
35+
grpc_future.cancel()
36+
elif grpc_future.cancelled():
3437
self.cancel()
35-
elif grpc_future.exception(timeout=self._default_timeout):
36-
self.set_exception(grpc_future.exception())
3738
elif grpc_future.done():
3839
try:
3940
result = grpc_future.result(timeout=self._default_timeout)
4041
self.set_result(result)
4142
except Exception as e:
4243
self.set_exception(e)
43-
elif grpc_future.running():
44-
self.set_running_or_notify_cancel()
4544

4645
def set_result(self, result):
4746
if self._result_transformer:

tests/unit_grpc/test_futures.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ def mock_grpc_future(
1515
grpc_future.exception.return_value = exception
1616
grpc_future.running.return_value = running
1717
grpc_future.result.return_value = result
18+
if exception:
19+
grpc_future.result.side_effect = exception
1820
return grpc_future
1921

2022

@@ -309,6 +311,7 @@ def test_result_catch_grpc_exceptions(self, mocker):
309311
def test_exception_when_done_maps_grpc_exception(self, mocker):
310312
grpc_future = mock_grpc_future(mocker, done=True)
311313
grpc_future.exception.return_value = FakeGrpcError(mocker)
314+
grpc_future.result.side_effect = grpc_future.exception.return_value
312315

313316
future = PineconeGrpcFuture(grpc_future)
314317

@@ -467,6 +470,7 @@ def test_concurrent_futures_wait_first_exception(self, mocker):
467470

468471
grpc_future2 = mock_grpc_future(mocker, done=True)
469472
grpc_future2.exception.return_value = Exception("Simulated gRPC error")
473+
grpc_future2.result.side_effect = grpc_future2.exception.return_value
470474
future2 = PineconeGrpcFuture(grpc_future2)
471475

472476
from concurrent.futures import wait, FIRST_EXCEPTION

0 commit comments

Comments
 (0)