Repository navigation
feat(google-cloud-ndb): add x-goog-is-transaction routing header - #18847
nikhilchowdary wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds transaction detection to Datastore RPC requests by appending an x-goog-is-transaction metadata header. It introduces helper functions to determine if a request is transactional and updates various Datastore API calls and unit tests accordingly. The review feedback suggests optimizing the metadata generation in _add_routing_info by using itertools.chain to avoid copying overhead from multiple tuple concatenations.
| result = tuple(metadata) | ||
| if header_params: | ||
| return tuple(metadata) + ( | ||
| result = result + ( | ||
| gapic_v1.routing_header.to_grpc_metadata(header_params), | ||
| ) | ||
|
|
||
| return tuple(metadata) | ||
| if is_transaction is None: | ||
| is_transaction = _is_transaction_request(request) | ||
|
|
||
| return result + ( | ||
| ("x-goog-is-transaction", "true" if is_transaction else "false"), | ||
| ) |
There was a problem hiding this comment.
In performance-critical paths like Datastore RPC metadata generation, multiple tuple concatenations (e.g., result = result + (...)) introduce unnecessary copying overhead. Using itertools.chain to combine the metadata collections and converting to a tuple once at the end is much more efficient.
| result = tuple(metadata) | |
| if header_params: | |
| return tuple(metadata) + ( | |
| result = result + ( | |
| gapic_v1.routing_header.to_grpc_metadata(header_params), | |
| ) | |
| return tuple(metadata) | |
| if is_transaction is None: | |
| is_transaction = _is_transaction_request(request) | |
| return result + ( | |
| ("x-goog-is-transaction", "true" if is_transaction else "false"), | |
| ) | |
| additional = [] | |
| if header_params: | |
| additional.append( | |
| gapic_v1.routing_header.to_grpc_metadata(header_params) | |
| ) | |
| if is_transaction is None: | |
| is_transaction = _is_transaction_request(request) | |
| additional.append( | |
| ("x-goog-is-transaction", "true" if is_transaction else "false") | |
| ) | |
| import itertools | |
| return tuple(itertools.chain(metadata, additional)) |
References
- In performance-critical paths, avoid combining collections by converting them to tuples and concatenating them (e.g.,
tuple(a) + tuple(b)), as this introduces copying overhead. Instead, use lazy iterators likeitertools.chainto iterate over multiple collections efficiently.
Attach the `x-goog-is-transaction` routing metadata header in `_add_routing_info`: - Set to `true` for transactional RPCs (`begin_transaction`, `rollback`, and transactional `commit`, `lookup`, `run_query`, `run_aggregation_query`). - Set to `false` for non-transactional RPCs (`allocate_ids`, `reserve_ids`, and non-transactional `commit`, `lookup`, `run_query`, `run_aggregation_query`).
eac13bf to
42e0414
Compare
Add
x-goog-is-transactionrouting header in Python ndb SDK to differentiate transactional requests and non-transactional requests from each other. Having a routing header for transactions helps in routing grpc requests at GFE.This commit attaches the x-goog-is-transaction routing header to all RPCs
Set to true for transactional RPCs
-- beginTransaction
-- rollback
-- transactional commit
-- transactional lookup
-- transactional runQuery
-- transactional runAggregationQuery.
Set to false for non transactional RPCs
-- allocateIds
-- reserveIds
-- non transactional commit
-- non transactional lookup
-- non transactional runQuery
-- non transactional runAggregationQuery.
cc: @MarkDuckworth