Skip to content

Commit 921e0b4

Browse files
authored
gRPC: parse_query_response: Skip parsing empty Usage (#301)
## Problem When parsing the result of a gRPC query() call, we unconditionally create a Usage model object, even if no usage information was returned (e.g. non-serverless index). ## Solution This adds a small but not insignificant cost to every query() call - mostly due to the fact we use OpenAPI auto-generated model code for the Usage and QueryResponse objects. Benchmarks using a simple PineconeGRPC-based program making query() calls against a p2.x4 pod show a 1.05x improvment in QPS by only constructing a Usage class (and associating it to QueryResponse) if a 'usage' field is present in the protobuf response: Before: Type Name # reqs # fails | Avg Min Max Med | req/s failures/s --------|----------------------------------------------------------------------------|-------|-------------|-------|-------|-------|-------|--------|----------- grpc query_pinecone_no_filter 3223 0(0.00%) | 17 17 139 18 | 55.01 0.00 --------|----------------------------------------------------------------------------|-------|-------------|-------|-------|-------|-------|--------|----------- After: Type Name # reqs # fails | Avg Min Max Med | req/s failures/s --------|----------------------------------------------------------------------------|-------|-------------|-------|-------|-------|-------|--------|----------- grpc query_pinecone_no_filter 3408 0(0.00%) | 17 16 96 17 | 57.55 0.00 --------|----------------------------------------------------------------------------|-------|-------------|-------|-------|-------|-------|--------|----------- ## Type of Change - [ ] Bug fix (non-breaking change which fixes an issue) - [x] 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 - Existing unit tests - Tested manually against pinecone-field/pinecone-stress-test
1 parent 61c19ad commit 921e0b4

File tree

1 file changed

+14
-11
lines changed

1 file changed

+14
-11
lines changed

pinecone/grpc/utils.py

+14-11
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,13 @@ def parse_fetch_response(response: dict):
5050
return FetchResponse(
5151
vectors=vd,
5252
namespace=namespace,
53-
usage=parse_usage(response),
53+
usage=parse_usage(response.get("usage", {})),
5454
_check_type=False
5555
)
5656

57-
def parse_usage(response):
58-
u = response.get("usage", {})
59-
return Usage(read_units=int(u.get("readUnits", 0)))
57+
58+
def parse_usage(usage: dict):
59+
return Usage(read_units=int(usage.get("readUnits", 0)))
6060

6161

6262
def parse_query_response(response: dict, _check_type: bool = False):
@@ -72,13 +72,16 @@ def parse_query_response(response: dict, _check_type: bool = False):
7272
)
7373
matches.append(sc)
7474

75-
return QueryResponse(
76-
namespace=response.get("namespace", ""),
77-
matches=matches,
78-
usage = parse_usage(response),
79-
_check_type=_check_type
80-
)
81-
75+
# Due to OpenAPI model classes / actual parsing cost, we want to avoid
76+
# creating empty `Usage` objects and then passing them into QueryResponse
77+
# when they are not actually present in the response from the server.
78+
args = {'namespace': response.get("namespace", ""),
79+
'matches': matches,
80+
'_check_type': _check_type}
81+
usage = response.get("usage")
82+
if usage:
83+
args['usage'] = parse_usage(usage)
84+
return QueryResponse(**args)
8285

8386
def parse_stats_response(response: dict):
8487
fullness = response.get("indexFullness", 0.0)

0 commit comments

Comments
 (0)