feat: add pooled grpc transport #748
Conversation
google/cloud/bigtable/client.py
Outdated
| except RuntimeError: | ||
| warnings.warn( | ||
| "Table should be created in an asyncio event loop." | ||
| " Instance will not be registered with client for refresh", | ||
| RuntimeWarning, | ||
| stacklevel=2, | ||
| ) |
There was a problem hiding this comment.
should this error instead of just warn? Being able to initialize a Client outside an async context could make sense but should we allow going as far as initializing a Table without being in an async context?
There was a problem hiding this comment.
Is there are main use cases that have benefits where a user would want to initialize these outside of an async context and then manually register? Seems much more difficult and more work for the user.
There was a problem hiding this comment.
Yeah, I've been a little unsure about this too, but I think making the Table require an async context is probably a good middle ground. And that will allow us to change the registration methods into private methods
I'll make that change, thanks for the input
google/cloud/bigtable/client.py
Outdated
| next_sleep = max(first_refresh - time.time(), 0) | ||
| if next_sleep > 0: | ||
| # warm the current channel immediately | ||
| channel = self.transport._grpc_channel._pool[channel_idx] |
There was a problem hiding this comment.
There are a couple places where pooled transport's _grpc_channel._pool private attribute is accessed. Perhaps make it public, or alternatively fold the functionality into the transport class itself as a public method. I get the impression that the transport classes intend to hide away most/some of these channel level management details from its user (in this case bigtable/client.py).
There was a problem hiding this comment.
Good points, I added pool_size and channels as public properties on the pooled transport, and am using them here. Thanks!
Co-authored-by: Jack Wotherspoon <[email protected]>
Co-authored-by: Jack Wotherspoon <[email protected]>
Co-authored-by: Jack Wotherspoon <[email protected]>
* feat: add new v3.0.0 API skeleton (#745) * feat: improve rows filters (#751) * feat: read rows query model class (#752) * feat: implement row and cell model classes (#753) * feat: add pooled grpc transport (#748) * feat: implement read_rows (#762) * feat: implement mutate rows (#769) * feat: literal value filter (#767) * feat: row_exists and read_row (#778) * feat: read_modify_write and check_and_mutate_row (#780) * feat: sharded read rows (#766) * feat: ping and warm with metadata (#810) * feat: mutate rows batching (#770) * chore: restructure module paths (#816) * feat: improve timeout structure (#819) * fix: api errors apply to all bulk mutations * chore: reduce public api surface (#820) * feat: improve error group tracebacks on < py11 (#825) * feat: optimize read_rows (#852) * chore: add user agent suffix (#842) * feat: optimize retries (#854) * feat: add test proxy (#836) * chore(tests): add conformance tests to CI for v3 (#870) * chore(tests): turn off fast fail for conformance tets (#882) * feat: add TABLE_DEFAULTS enum for table method arguments (#880) * fix: pass None for retry in gapic calls (#881) * feat: replace internal dictionaries with protos in gapic calls (#875) * chore: optimize gapic calls (#863) * feat: expose retryable error codes to users (#879) * chore: update api_core submodule (#897) * chore: merge main into experimental_v3 (#900) * chore: pin conformance tests to v0.0.2 (#903) * fix: bulk mutation eventual success (#909) --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
PooledBigtableGrpcAsyncIOTransportclass, that manages a pool of grpc channels and round-robins requests between themPooledBigtableGrpcAsyncIOTransportis implemented in a branch off https://github.com/googleapis/gapic-generator-python. The branch is added as a submodule, so changes can be tracked in this repoGooglers see go/python-bigtable-v3-design, specifically the section titled
Generator-Based Retries