Skip to content

Commit

Permalink
review fixes
Browse files Browse the repository at this point in the history
Signed-off-by: Aditi Ahuja <ahuja.aditi@gmail.com>
  • Loading branch information
metonymic-smokey committed Aug 4, 2022
1 parent bda234f commit aac92ec
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 10 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re
- [#5493](https://github.com/thanos-io/thanos/pull/5493) Compact: Added `--compact.blocks-fetch-concurrency` allowing to configure number of go routines for download blocks during compactions.
- [#5527](https://github.com/thanos-io/thanos/pull/5527) Receive: Add per request limits for remote write.
- [#5520](https://github.com/thanos-io/thanos/pull/5520) Receive: Meta-monitoring based active series limiting
- [#5555](https://github.com/thanos-io/thanos/pull/5555) Query: Added `--query.active-query-path` flag, allowing the user to configure the directory to create an active query tracking file, `queries.active`.
- [#5555](https://github.com/thanos-io/thanos/pull/5555) Query: Added `--query.active-query-path` flag, allowing the user to configure the directory to create an active query tracking file, `queries.active`, for different resolution.

### Changed

Expand Down
34 changes: 27 additions & 7 deletions cmd/thanos/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"math"
"net/http"
"path/filepath"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -487,11 +488,6 @@ func runQuery(
}
)

if activeQueryDir != "" {
queryTracker := promql.NewActiveQueryTracker(activeQueryDir, maxConcurrentQueries, logger)
engineOpts.ActiveQueryTracker = queryTracker
}

// Periodically update the store set with the addresses we see in our cluster.
{
ctx, cancel := context.WithCancel(context.Background())
Expand Down Expand Up @@ -585,7 +581,8 @@ func runQuery(
grpcProbe,
prober.NewInstrumentation(comp, logger, extprom.WrapRegistererWithPrefix("thanos_", reg)),
)
engineCreator := engineFactory(promql.NewEngine, engineOpts, dynamicLookbackDelta)
engineCreator := engineFactory(promql.NewEngine, engineOpts, dynamicLookbackDelta, activeQueryDir,
maxConcurrentQueries, logger)

// Start query API + UI HTTP server.
{
Expand Down Expand Up @@ -745,6 +742,9 @@ func engineFactory(
newEngine func(promql.EngineOpts) *promql.Engine,
eo promql.EngineOpts,
dynamicLookbackDelta bool,
activeQueryDir string,
maxConcurrentQueries int,
logger log.Logger,
) func(int64) *promql.Engine {
resolutions := []int64{downsample.ResLevel0}
if dynamicLookbackDelta {
Expand All @@ -763,16 +763,23 @@ func engineFactory(
if ld < r {
lookbackDelta = time.Duration(r) * time.Millisecond
}

var queryTracker *promql.ActiveQueryTracker
if activeQueryDir != "" {
resActiveQueryDir := filepath.Join(activeQueryDir, getActiveQueryDirBasedOnResolution(r))
queryTracker = promql.NewActiveQueryTracker(resActiveQueryDir, maxConcurrentQueries, logger)
}

engines[i] = newEngine(promql.EngineOpts{
Logger: eo.Logger,
Reg: wrapReg(i),
MaxSamples: eo.MaxSamples,
Timeout: eo.Timeout,
ActiveQueryTracker: eo.ActiveQueryTracker,
LookbackDelta: lookbackDelta,
NoStepSubqueryIntervalFn: eo.NoStepSubqueryIntervalFn,
EnableAtModifier: eo.EnableAtModifier,
EnableNegativeOffset: eo.EnableNegativeOffset,
ActiveQueryTracker: queryTracker,
})
}
return func(maxSourceResolutionMillis int64) *promql.Engine {
Expand All @@ -788,3 +795,16 @@ func engineFactory(
return engines[0]
}
}

func getActiveQueryDirBasedOnResolution(resolution int64) string {
if resolution == downsample.ResLevel0 {
return "raw"
}
if resolution == downsample.ResLevel1 {
return "5m"
}
if resolution == downsample.ResLevel2 {
return "1h"
}
return ""
}
4 changes: 3 additions & 1 deletion cmd/thanos/query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"testing"
"time"

"github.com/go-kit/log"
"github.com/prometheus/prometheus/promql"

"github.com/thanos-io/thanos/pkg/testutil"
Expand Down Expand Up @@ -102,7 +103,8 @@ func TestEngineFactory(t *testing.T) {
}
)
for _, td := range tData {
e := engineFactory(mockNewEngine, promql.EngineOpts{LookbackDelta: td.lookbackDelta}, td.dynamicLookbackDelta)
e := engineFactory(mockNewEngine, promql.EngineOpts{LookbackDelta: td.lookbackDelta}, td.dynamicLookbackDelta,
"", 1, log.NewNopLogger())
for _, tc := range td.tcs {
got := e(tc.stepMillis)
testutil.Equals(t, tc.expect, got)
Expand Down
2 changes: 1 addition & 1 deletion docs/components/query.md
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ Example file SD file in YAML:

## Active Query Tracking

`--query.active-query-path` is an option which allows the user to specify a directory which will contain a `queries.active` file to track active queries. To enable this feature, the user has to specify a directory other than "", since that is skipped being the default. Currently, this feature is not covered under the primarily container-based E2E test suite due to container permission issues for creating files.
`--query.active-query-path` is an option which allows the user to specify a directory which will contain a `queries.active` file to track active queries. To enable this feature, the user has to specify a directory other than "", since that is skipped being the default.

## Flags

Expand Down

0 comments on commit aac92ec

Please sign in to comment.