Make the max grpc receive message size configurable, in Bigtable client #1138
Conversation
|
I feel like we need a "generic" gRPC client config struct to unify the frontend client, the ingester client and the bigtable client with the same options, as we're hitting these problems again and again. WDYT? |
bboreham
changed the title
Going to have a stab at this now. |
Signed-off-by: Goutham Veeramachaneni <[email protected]>
Signed-off-by: Tom Wilkie <[email protected]>
| cfg.GRPCClientConfig.RegisterFlags("ingester.client", f) | ||
|
|
||
| // Deprecated. | ||
| f.Int("ingester.client.max-recv-message-size", 64*1024*1024, "DEPRECATED. Maximum message size, in bytes, this client will receive.") |
There was a problem hiding this comment.
These options don't do anything as of now. Is it not better to fail when these are specified rather than silently not set them at all?
There was a problem hiding this comment.
This is currently "the way we do things" - we don't want an upgrade to fail because flags changed or went away. I think there is a discussion coming up about what our backwards compatibility guarantees will be, so let not do this now?
There was a problem hiding this comment.
In the case of the compression options I would have preferred an outright failure.
What is the precedent in Cortex to leave an option doing nothing?
|
LGTM from me |
We're hitting this limit and
100 << 20is the default in the bigtable SDK anyways.