Skip to content

Update to allow configurable max message size for querier.#1052

Merged
tomwilkie merged 1 commit intocortexproject:masterfrom
grafana:20181002_query_max_message_size
Oct 3, 2018
Merged

Update to allow configurable max message size for querier.#1052
tomwilkie merged 1 commit intocortexproject:masterfrom
grafana:20181002_query_max_message_size

Conversation

@jtlisi
Copy link
Contributor

@jtlisi jtlisi commented Oct 2, 2018

No description provided.

@jtlisi jtlisi force-pushed the 20181002_query_max_message_size branch from 351e7bc to 75ae548 Compare October 2, 2018 18:38
@tomwilkie
Copy link
Contributor

LGTM!

@tomwilkie tomwilkie changed the title update to allow configurable max message size for querier Update to allow configurable max message size for querier. Oct 2, 2018
@jtlisi jtlisi force-pushed the 20181002_query_max_message_size branch 2 times, most recently from 10235ad to 4443a13 Compare October 2, 2018 20:28
@tomwilkie
Copy link
Contributor

Ideally this would be part of weaveworks/common/server, but there is precedent for this: #679

@tomwilkie
Copy link
Contributor

LGTM x2.

@bboreham
Copy link
Contributor

bboreham commented Oct 2, 2018

Is it the max send size or max recv size? The code says both.
We already have the latter in the ingester -

f.IntVar(&cfg.MaxRecvMsgSize, "ingester.client.max-recv-message-size", 64*1024*1024, "Maximum message size, in bytes, this client will receive.")

@jtlisi jtlisi force-pushed the 20181002_query_max_message_size branch 2 times, most recently from 68be779 to b9c36a2 Compare October 2, 2018 21:06
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to mention the units, i.e, size of a grpc message, in bytes, ...

@bboreham
Copy link
Contributor

bboreham commented Oct 3, 2018

Why is it the server receiving size you care about, as opposed to the client? (Or server send)

@tomwilkie
Copy link
Contributor

Why is it the server receiving size you care about, as opposed to the client? (Or server send)

When using the query-frontend, the querier creates a gRPC streaming connection to it, requests are pushed down from server (frontend) to client (querier) and responses are pushed from client (querier) to server (frontend). We hit an issue where large query responses exceeded the limit for messages received by the frontend.

@bboreham
Copy link
Contributor

bboreham commented Oct 3, 2018

Can we have a commit message that more closely describes the intent, e.g. "add command-line option to ..." please.

… messages from querier.

Signed-off-by: Jacob Lisi <jacob.t.lisi@gmail.com>
@tomwilkie tomwilkie force-pushed the 20181002_query_max_message_size branch from b9c36a2 to 2e2effa Compare October 3, 2018 09:51
@tomwilkie tomwilkie merged commit d8c5de6 into cortexproject:master Oct 3, 2018
@tomwilkie tomwilkie deleted the 20181002_query_max_message_size branch October 3, 2018 10:25
@bboreham
Copy link
Contributor

bboreham commented Oct 3, 2018

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants