Fix Series API slowness #2123
Fix Series API slowness #2123
Conversation
|
This is just an attempt to fix the API slowness. I may be wrong |
Codecov Report
@@ Coverage Diff @@
## master #2123 +/- ##
==========================================
+ Coverage 61.50% 61.79% +0.28%
==========================================
Files 149 149
Lines 11935 12084 +149
==========================================
+ Hits 7341 7467 +126
- Misses 4012 4029 +17
- Partials 582 588 +6
|
There was a problem hiding this comment.
I think we want to solve it this way #2106 instead.
The problem with the current approach is that you may load a lot of chunks in memory that can cause the querier to OOM. Currently it's limited by s.cfg.MaxChunkBatchSize which is 50 at the time.
I'll defer to Owen, but I think we want instead to split queries by time.
Got your point. I didn't realize this |
|
Oh yes, I'd much rather we solve this via parallelization in the frontend the same way we do for other queries. Would you like to pick up that approach instead? |
adityacs
force-pushed
the
2121_series_api_fix
branch
7 times, most recently
from
da64194
to
7a1e827
Compare
adityacs
force-pushed
the
2121_series_api_fix
branch
4 times, most recently
from
5357012
to
f7187ec
Compare
| end := start.Add(interval) | ||
| if end.After(r.EndTs) { | ||
| end = r.EndTs | ||
| for start := req.GetStart(); start < req.GetEnd(); start = start + interval.Milliseconds() { |
There was a problem hiding this comment.
This change limits us to use only millisecond. Since, GetStart and GetEnd methods converts the time to milliseconds we won't able to measure anything below millisecond
Time with value 1575892860000000001 will be 1575892860000 upon calling GetStart or GetEnd
There was a problem hiding this comment.
Great point, I didn't think of that.
I think your original approach is better :)
|
@owen-d Have made the changes based on your comments. Kindly review. |
pkg/querier/queryrange/codec_test.go
Outdated
| @@ -168,6 +184,35 @@ func Test_codec_EncodeRequest(t *testing.T) { | |||
| require.Equal(t, "/loki/api/v1/query_range", req.(*LokiRequest).Path) | |||
| } | |||
|
|
|||
| func Test_codec_sereis_EncodeRequest(t *testing.T) { | |||
There was a problem hiding this comment.
| func Test_codec_sereis_EncodeRequest(t *testing.T) { | |
| func Test_codec_series_EncodeRequest(t *testing.T) { |
|
Thanks for addressing my comments; it looks great. Just a bit of cleanup then we can get this merged |
adityacs
force-pushed
the
2121_series_api_fix
branch
from
f7187ec
to
bb5fcf7
Compare
What this PR does / why we need it:
Fix Series API slowness
Which issue(s) this PR fixes:
Fixes #2121
Checklist