querier: Support store matchers and time range filter on labels API #3133
Conversation
Signed-off-by: Ben Ye <[email protected]> Add more unit tests in proxy store Signed-off-by: Ben Ye <[email protected]>
yeya24
force-pushed
the
support-store-matchers-labels
branch
from
8037d3a
to
7d3d7df
Compare
Signed-off-by: Ben Ye <[email protected]>
kakkoyun
changed the title
pkg/store/proxy.go
Outdated
| @@ -270,10 +270,10 @@ func (s *ProxyStore) Series(r *storepb.SeriesRequest, srv storepb.Store_SeriesSe | |||
| ok, _ = storeMatches(st, r.MinTime, r.MaxTime, storeMatcher, r.Matchers...) | |||
| }) | |||
| if !ok { | |||
| storeDebugMsgs = append(storeDebugMsgs, fmt.Sprintf("store %s filtered out", st)) | |||
| storeDebugMsgs = append(storeDebugMsgs, fmt.Sprintf("Store %s filtered out", st)) | |||
There was a problem hiding this comment.
Very minor thing but why was this word capitalized?
There was a problem hiding this comment.
Oh, I thought capitalized looks better.
Capitialized
level=warn ts=2020-09-08T20:59:57.731820598Z caller=proxy.go:310 err="No StoreAPIs matched for this query" stores="Store Addr: localhost:10905 LabelSets: [name:\"cluster\" value:\"test\" ] Mint: 1598450400000 Maxt: 1599408000000 filtered out;Store Addr: localhost:10903 LabelSets: [name:\"cluster\" value:\"test\" ] Mint: 1598299200000 Maxt: 9223372036854775807 filtered out"
Not Capitalized
level=warn ts=2020-09-08T21:02:28.654728738Z caller=proxy.go:310 err="No StoreAPIs matched for this query" stores="store Addr: localhost:10903 LabelSets: [name:\"cluster\" value:\"test\" ] Mint: 1598299200000 Maxt: 9223372036854775807 filtered out;store Addr: localhost:10905 LabelSets: [name:\"cluster\" value:\"test\" ] Mint: 1598450400000 Maxt: 1599537600000 filtered out"
stores="store Addr: localhost:10903
It looks weird to have store not capitalized but have Addr capitalized.
| @@ -515,7 +515,8 @@ func storeMatches(s Client, mint, maxt int64, storeMatcher [][]storepb.LabelMatc | |||
| return false, nil | |||
| } | |||
| match, err := storeMatchMetadata(s, storeMatcher) | |||
| if err != nil || !match { | |||
| // Return result here if no matchers set. | |||
| if len(matchers) == 0 || err != nil || !match { | |||
There was a problem hiding this comment.
Doesn't labelSetsMatch() already return true, nil if matchers is nil? In labelSetMatches we range over nil and because it's empty, we simply jump to the end and return true, nil, right?
There was a problem hiding this comment.
return labelSetsMatch(s.LabelSets(), matchers) If we don't return and then go to labelSetsMatch here, we need to first evaluate s.LabelSets() https://github.com/thanos-io/thanos/blob/master/pkg/query/storeset.go#L277.
I want to avoid this function because it needs to get the lock and alloc a slice.
|
Hello @GiedriusS I just update this pr. Please take a look again when you get a chance, thanks! |
Signed-off-by: Ben Ye <[email protected]>
yeya24
force-pushed
the
support-store-matchers-labels
branch
from
15f4951
to
c89ef0a
Compare
Signed-off-by: Ben Ye [email protected]
Changes
This pr supports store matches for Labels API. It also supports time range based filtering in Querier to reduce unnecessary calls for labels API.
Verification