-
Notifications
You must be signed in to change notification settings - Fork 9.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add streaming to Remote Read endpoint #4517
Comments
I can take it if no one started working on this. Design doc: https://docs.google.com/document/d/1JqrU3NjM9HoGLSTPYOvR217f5HBKBiJTqikEB9UiJL0/edit?usp=sharing Dev-mailing topic: https://groups.google.com/forum/#!topic/prometheus-developers/79b6j_6F_Vc |
According to #4655 it seems this has been resolved in prometheus 2.5.0 |
Well, the memory was reduced (not sure how largely), but there still is a need for streamed remote read, potentially with chunk encoding to reduce used memory on query to minium. |
I would like to add that, as a vendor implementing the server-side of remote read API, the lack of streaming adds similar memory pressure on the marshaling side. @bwplotka I notice that your already have a PR for streaming support in #4591. Whenever this is approved for merge, we would be happy to implement it on our storage and validate. |
Yes. This is working code for streaming with raw samples, but we have some internal discussion how it should be done. I closed it to reduce confusion for now: #4591 TL;DR: It would be nice to have remote read stream with Prometheus chunk encoded samples (instead of row samples), to avoid unnecessary decoding & encoding. Does it
Did not have much time to jump into it yet, but I am happy to hear that you are ok to help with this! |
Is that essentially "Alternative 1" in the design doc ? |
Exactly, but need to reorder ;p |
Just FYI, since we got concensus on this, expect more finalized design doc (same link: https://docs.google.com/document/d/1JqrU3NjM9HoGLSTPYOvR217f5HBKBiJTqikEB9UiJL0/edit#) and initial benchmarks soon. I scheduled some time for this work in June. |
Back to business! Server side changes: #5703 Living document: https://docs.google.com/document/d/1JqrU3NjM9HoGLSTPYOvR217f5HBKBiJTqikEB9UiJL0/edit# |
First MVP should be safe to use. I build 2 docker images if someone wants to try:
If you use sidecar, I updated caller side as well:
No config changes, just move docker images to that Prometheus and Thanos sidecar tags and enjoy! 2 big requests against Prometheus + sidecar in upgraded version look like this memory and CPU wise: Versus old one with non-streaming sampled read (3GB of mem!): |
…hunked, checksumed reader (#5703) Part of: #4517 and thanos-io/thanos#488 Changes: * Extended protobuf for chunked remote read and negotation. * Added checksumed, chunked Writer/Reader. * Added Server side implementation for chunked streamed remote-read. Signed-off-by: Bartek Plotka <bwplotka@gmail.com>
Remote read now supports streaming. |
Follow up tasks:
|
Remote read is quite inefficient right now and could be made much better through streaming. This has been discussed in the dev-summit: https://docs.google.com/document/d/1-C5PycocOZEVIPrmM1hn8fBelShqtqiAmFptoG4yK70/edit#heading=h.5d2mc4i0bnnp
The text was updated successfully, but these errors were encountered: