receive: use gRPC for forwarding messages#1970
Merged
brancz merged 3 commits intothanos-io:masterfrom Jan 10, 2020
Merged
Conversation
blockloop
force-pushed
the
bjones/receive/use-grpc
branch
3 times, most recently
from
bdbc8d6 to
e1153af
Compare
squat
reviewed
Jan 8, 2020
Member
squat
left a comment
squat
left a comment
There was a problem hiding this comment.
Super fast progress! I have some questions @blockloop
blockloop
force-pushed
the
bjones/receive/use-grpc
branch
from
e1153af to
f54ecf6
Compare
brancz
reviewed
Jan 9, 2020
squat
reviewed
Jan 9, 2020
| // ReadWriteStoreServer is a StoreServer and a WriteableStoreServer. | ||
| type ReadWriteStoreServer interface { | ||
| storepb.StoreServer | ||
| storepb.WriteableStoreServer |
Member
There was a problem hiding this comment.
I am wondering again about exposing these on the same gRPC service. If we do, we have to make sure to document that exposing the receive gRPC to a WAN to enable writes means that one tenant could read any other tenant’s metrics
Member
There was a problem hiding this comment.
I guess that's acceptable for now 🤔 we could change it in the future if needed.
Member
|
It is today. We should change both (gRPC and http) to 1-indexed
El El jue, ene. 9, 2020 a las 15:18, Brett <notifications@github.com>
escribió:
… ***@***.**** commented on this pull request.
------------------------------
In pkg/store/storepb/rpc.proto
<#1970 (comment)>:
> @@ -39,6 +40,21 @@ service Store {
rpc LabelValues(LabelValuesRequest) returns (LabelValuesResponse);
}
+/// WriteableStore reprents API against instance that stores XOR encoded values with label set metadata (e.g Prometheus metrics).
+service WriteableStore {
+ // WriteRequest allows you to write metrics to this store via remote write
+ rpc RemoteWrite(WriteRequest) returns (WriteResponse) {}
+}
+
+message WriteResponse {
+}
+
+message WriteRequest {
+ string tenant = 1;
+ int64 replica = 2;
Is the replica header 0 indexed? I think 1 indexed would be the better
option given the zero value situation.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#1970?email_source=notifications&email_token=AE4JAPZRMBIF4NGT7S65KA3Q44W2VA5CNFSM4KEP4QEKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCRGGB7I#discussion_r364761558>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AE4JAP7Q3DP6SJB74ZH2GLLQ44W2VANCNFSM4KEP4QEA>
.
|
blockloop
force-pushed
the
bjones/receive/use-grpc
branch
3 times, most recently
from
4aa3200 to
ea911f5
Compare
squat
reviewed
Jan 9, 2020
Member
squat
left a comment
squat
left a comment
There was a problem hiding this comment.
So awesome. We already hacked together a lot offline so most comments are already addressed. I have some v small requests and then 💯
squat
reviewed
Jan 9, 2020
blockloop
force-pushed
the
bjones/receive/use-grpc
branch
from
6676444 to
0d039eb
Compare
2 tasks
GiedriusS
reviewed
Jan 9, 2020
Member
GiedriusS
left a comment
GiedriusS
left a comment
There was a problem hiding this comment.
At a first glance LGTM. Could you rebase after the recent changes before the final review?
blockloop
force-pushed
the
bjones/receive/use-grpc
branch
2 times, most recently
from
5748a8a to
6709020
Compare
Contributor
Author
|
Rebase complete @GiedriusS |
Signed-off-by: Brett Jones <blockloop@users.noreply.github.com>
blockloop
force-pushed
the
bjones/receive/use-grpc
branch
from
6709020 to
018d262
Compare
Signed-off-by: Lucas Servén Marín <lserven@gmail.com>
squat
force-pushed
the
bjones/receive/use-grpc
branch
from
d2706dc to
9d1df07
Compare
brancz
reviewed
Jan 10, 2020
Member
brancz
left a comment
brancz
left a comment
There was a problem hiding this comment.
Just one small comment, otherwise this lgtm. Very incredibly fast turn around I have to say! Thank you so much! :)
CHANGELOG.md
Outdated
| - [#1969](https://github.com/thanos-io/thanos/pull/1969) Sidecar: allow setting http connection pool size via flags | ||
| - [#1967](https://github.com/thanos-io/thanos/issues/1967) Receive: Allow local TSDB compaction | ||
| - [#1975](https://github.com/thanos-io/thanos/pull/1975) Store Gateway: fixed panic caused by memcached servers selector when there's 1 memcached node | ||
| - [#1970](https://github.com/thanos-io/thanos/issues/1970) Receive: Use gRPC for forwarding requests between peers |
Member
There was a problem hiding this comment.
I think we need an item here that indicates that people need to change their hashring files to be just addresses that the gRPC client is built from, instead of the full http endpoint.
Signed-off-by: Lucas Servén Marín <lserven@gmail.com>
squat
added a commit
to squat/thanos-receive-controller
that referenced
this pull request
Jan 13, 2020
This commit updates the controller to generate gRPC endpoints rather than HTTP endpoints, following thanos-io/thanos#1970. Signed-off-by: Lucas Servén Marín <lserven@gmail.com>
squat
added a commit
to squat/thanos-receive-controller
that referenced
this pull request
Jan 13, 2020
This commit updates the controller to generate gRPC endpoints rather than HTTP endpoints, following thanos-io/thanos#1970. Signed-off-by: Lucas Servén Marín <lserven@gmail.com>
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
Receive: use gRPC for forwarding messages between receive peers to significantly increase throughput. This also introduces the ability for users to use gRPC to write messages to Thanos via gRPC.
Verified
A working build can be run with this docker-compose configuration https://gist.github.com/blockloop/637bd0c8c9295178b67812f43e661419
The images used in that gist have been built from this branch. I have run it and verified that everything is working.
Here is a screenshot of Thanos query from that setup:
Fixes #1710