Skip to content
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

Flag to add active query tracker. #5555

Merged
merged 7 commits into from Aug 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -27,6 +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`, for different resolution.

### Changed

Expand Down
41 changes: 37 additions & 4 deletions cmd/thanos/query.go
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"math"
"net/http"
"path/filepath"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -161,6 +162,8 @@ func registerQuery(app *extkingpin.App) {
enableMetricMetadataPartialResponse := cmd.Flag("metric-metadata.partial-response", "Enable partial response for metric metadata endpoint. --no-metric-metadata.partial-response for disabling.").
Hidden().Default("true").Bool()

activeQueryDir := cmd.Flag("query.active-query-path", "Directory to log currently active queries in the queries.active file.").Default("").String()

featureList := cmd.Flag("enable-feature", "Comma separated experimental feature names to enable.The current list of features is "+queryPushdown+".").Default("").Strings()

enableExemplarPartialResponse := cmd.Flag("exemplar.partial-response", "Enable partial response for exemplar endpoint. --no-exemplar.partial-response for disabling.").
Expand Down Expand Up @@ -271,6 +274,7 @@ func registerQuery(app *extkingpin.App) {
*enableTargetPartialResponse,
*enableMetricMetadataPartialResponse,
*enableExemplarPartialResponse,
*activeQueryDir,
fileSD,
time.Duration(*dnsSDInterval),
*dnsSDResolver,
Expand Down Expand Up @@ -338,6 +342,7 @@ func runQuery(
enableTargetPartialResponse bool,
enableMetricMetadataPartialResponse bool,
enableExemplarPartialResponse bool,
activeQueryDir string,
fileSD *file.Discovery,
dnsSDInterval time.Duration,
dnsSDResolver string,
Expand Down Expand Up @@ -467,6 +472,7 @@ func runQuery(
maxConcurrentSelects,
queryTimeout,
)

engineOpts = promql.EngineOpts{
Logger: logger,
Reg: reg,
Expand Down Expand Up @@ -575,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 @@ -735,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 @@ -753,17 +763,27 @@ func engineFactory(
if ld < r {
lookbackDelta = time.Duration(r) * time.Millisecond
}
engines[i] = newEngine(promql.EngineOpts{

newEngineOpts := 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,
})
}
// An active query tracker will be added only if the user specifies a non-default path.
// Otherwise, the nil active query tracker from existing engine options will be used.
if activeQueryDir != "" {
resActiveQueryDir := filepath.Join(activeQueryDir, getActiveQueryDirBasedOnResolution(r))
newEngineOpts.ActiveQueryTracker = promql.NewActiveQueryTracker(resActiveQueryDir, maxConcurrentQueries, logger)
} else {
newEngineOpts.ActiveQueryTracker = eo.ActiveQueryTracker
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this else block? If activeQueryDir is empty string then the ActiveQueryTracker is just nil if I understand correctly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yeya24 in the current implementation, this line explicitly adds eo.ActiveQueryTracker

ActiveQueryTracker: eo.ActiveQueryTracker,
.
I have attempted to retain that when I added the else.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove it to avoid confusion.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or you could add a comment about it is nil.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, done.
If possible could you please retrigger the CI tests once again?
Thanks!

}

engines[i] = newEngine(newEngineOpts)
}
return func(maxSourceResolutionMillis int64) *promql.Engine {
for i := len(resolutions) - 1; i >= 1; i-- {
Expand All @@ -778,3 +798,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
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
7 changes: 7 additions & 0 deletions docs/components/query.md
Expand Up @@ -246,6 +246,10 @@ Example file SD file in YAML:
- thanos-store.infra:10901
```

## 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.

## Flags

```$ mdox-exec="thanos query --help"
Expand Down Expand Up @@ -323,6 +327,9 @@ Flags:
LogStartAndFinishCall: Logs the start and
finish call of the requests. NoLogCall: Disable
request logging.
--query.active-query-path=""
Directory to log currently active queries in
the queries.active file.
--query.auto-downsampling Enable automatic adjustment (step / 5) to what
source of data should be used in store gateways
if no max_source_resolution param is specified.
Expand Down