all: get rid of query pushdown to simplify query path

Signed-off-by: Michael Hoffmann <mhoffm@posteo.de>
thanos-io · Jan 22, 2024 · 0bf5c39 · 0bf5c39
1 parent ae93ed6
commit 0bf5c39
Show file tree

Hide file tree

Showing 21 changed files with 63 additions and 926 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -37,6 +37,8 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re
 
 ### Removed
 
+- [#7014](https://github.com/thanos-io/thanos/pull/7014) *: *breaking :warning:* Removed experimental query pushdown feature to simplify query path. This feature has had high complexity for too little benefits.
+
 ## [v0.33.0](https://github.com/thanos-io/thanos/tree/release-0.33) - 18.12.2023
 
 ### Fixed

diff --git a/cmd/thanos/query.go b/cmd/thanos/query.go
@@ -197,7 +197,7 @@ func registerQuery(app *extkingpin.App) {
 
 	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()
+	featureList := cmd.Flag("enable-feature", "Comma separated experimental feature names to enable.The current list of features is empty.").Hidden().Default("").Strings()
 
 	enableExemplarPartialResponse := cmd.Flag("exemplar.partial-response", "Enable partial response for exemplar endpoint. --no-exemplar.partial-response for disabling.").
 		Hidden().Default("true").Bool()
@@ -232,17 +232,16 @@ func registerQuery(app *extkingpin.App) {
 			return errors.Wrap(err, "parse federation labels")
 		}
 
-		var enableQueryPushdown bool
 		for _, feature := range *featureList {
-			if feature == queryPushdown {
-				enableQueryPushdown = true
-			}
 			if feature == promqlAtModifier {
 				level.Warn(logger).Log("msg", "This option for --enable-feature is now permanently enabled and therefore a no-op.", "option", promqlAtModifier)
 			}
 			if feature == promqlNegativeOffset {
 				level.Warn(logger).Log("msg", "This option for --enable-feature is now permanently enabled and therefore a no-op.", "option", promqlNegativeOffset)
 			}
+			if feature == queryPushdown {
+				level.Warn(logger).Log("msg", "This option for --enable-feature is now permanently deprecated and therefore ignored.", "option", queryPushdown)
+			}
 		}
 
 		httpLogOpts, err := logging.ParseHTTPOptions(reqLogConfig)
@@ -335,7 +334,6 @@ func registerQuery(app *extkingpin.App) {
 			*strictEndpoints,
 			*strictEndpointGroups,
 			*webDisableCORS,
-			enableQueryPushdown,
 			*alertQueryURL,
 			*grpcProxyStrategy,
 			component.Query,
@@ -417,7 +415,6 @@ func runQuery(
 	strictEndpoints []string,
 	strictEndpointGroups []string,
 	disableCORS bool,
-	enableQueryPushdown bool,
 	alertQueryURL string,
 	grpcProxyStrategy string,
 	comp component.Component,
@@ -708,7 +705,6 @@ func runQuery(
 			enableTargetPartialResponse,
 			enableMetricMetadataPartialResponse,
 			enableExemplarPartialResponse,
-			enableQueryPushdown,
 			queryReplicaLabels,
 			flagsMap,
 			defaultRangeQueryStep,

diff --git a/docs/components/query.md b/docs/components/query.md
@@ -286,9 +286,6 @@ Flags:
       --alert.query-url=ALERT.QUERY-URL
                                  The external Thanos Query URL that would be set
                                  in all alerts 'Source' field.
-      --enable-feature= ...      Comma separated experimental feature names
-                                 to enable.The current list of features is
-                                 query-pushdown.
       --endpoint=<endpoint> ...  Addresses of statically configured Thanos
                                  API servers (repeatable). The scheme may be
                                  prefixed with 'dns+' or 'dnssrv+' to detect

diff --git a/pkg/api/query/grpc.go b/pkg/api/query/grpc.go
@@ -97,7 +97,6 @@ func (g *GRPCAPI) Query(request *querypb.QueryRequest, server querypb.Query_Quer
 		storeMatchers,
 		maxResolution,
 		request.EnablePartialResponse,
-		request.EnableQueryPushdown,
 		false,
 		request.ShardInfo,
 		query.NoopSeriesStatsReporter,
@@ -195,7 +194,6 @@ func (g *GRPCAPI) QueryRange(request *querypb.QueryRangeRequest, srv querypb.Que
 		storeMatchers,
 		maxResolution,
 		request.EnablePartialResponse,
-		request.EnableQueryPushdown,
 		false,
 		request.ShardInfo,
 		query.NoopSeriesStatsReporter,

diff --git a/pkg/api/query/querypb/query.pb.go b/pkg/api/query/querypb/query.pb.go
diff --git a/pkg/api/query/querypb/query.proto b/pkg/api/query/querypb/query.proto
@@ -41,6 +41,7 @@ message QueryRequest {
 
   bool enableDedup = 7;
   bool enablePartialResponse = 8;
+  // Deprecated
   bool enableQueryPushdown = 9;
   bool skipChunks = 10;
 
@@ -80,6 +81,7 @@ message QueryRangeRequest {
 
   bool enableDedup = 9;
   bool enablePartialResponse = 10;
+  // Deprecated
   bool enableQueryPushdown = 11;
   bool skipChunks = 12;
 

diff --git a/pkg/api/query/v1.go b/pkg/api/query/v1.go
@@ -157,7 +157,6 @@ type QueryAPI struct {
 	enableTargetPartialResponse         bool
 	enableMetricMetadataPartialResponse bool
 	enableExemplarPartialResponse       bool
-	enableQueryPushdown                 bool
 	disableCORS                         bool
 
 	replicaLabels  []string
@@ -196,7 +195,6 @@ func NewQueryAPI(
 	enableTargetPartialResponse bool,
 	enableMetricMetadataPartialResponse bool,
 	enableExemplarPartialResponse bool,
-	enableQueryPushdown bool,
 	replicaLabels []string,
 	flagsMap map[string]string,
 	defaultRangeQueryStep time.Duration,
@@ -233,7 +231,6 @@ func NewQueryAPI(
 		enableTargetPartialResponse:            enableTargetPartialResponse,
 		enableMetricMetadataPartialResponse:    enableMetricMetadataPartialResponse,
 		enableExemplarPartialResponse:          enableExemplarPartialResponse,
-		enableQueryPushdown:                    enableQueryPushdown,
 		replicaLabels:                          replicaLabels,
 		endpointStatus:                         endpointStatus,
 		defaultRangeQueryStep:                  defaultRangeQueryStep,
@@ -565,7 +562,6 @@ func (qapi *QueryAPI) queryExplain(r *http.Request) (interface{}, []error, *api.
 			storeDebugMatchers,
 			maxSourceResolution,
 			enablePartialResponse,
-			qapi.enableQueryPushdown,
 			false,
 			shardInfo,
 			query.NewAggregateStatsReporter(&seriesStats),
@@ -668,7 +664,6 @@ func (qapi *QueryAPI) query(r *http.Request) (interface{}, []error, *api.ApiErro
 			storeDebugMatchers,
 			maxSourceResolution,
 			enablePartialResponse,
-			qapi.enableQueryPushdown,
 			false,
 			shardInfo,
 			query.NewAggregateStatsReporter(&seriesStats),
@@ -835,7 +830,6 @@ func (qapi *QueryAPI) queryRangeExplain(r *http.Request) (interface{}, []error,
 			storeDebugMatchers,
 			maxSourceResolution,
 			enablePartialResponse,
-			qapi.enableQueryPushdown,
 			false,
 			shardInfo,
 			query.NewAggregateStatsReporter(&seriesStats),
@@ -968,7 +962,6 @@ func (qapi *QueryAPI) queryRange(r *http.Request) (interface{}, []error, *api.Ap
 			storeDebugMatchers,
 			maxSourceResolution,
 			enablePartialResponse,
-			qapi.enableQueryPushdown,
 			false,
 			shardInfo,
 			query.NewAggregateStatsReporter(&seriesStats),
@@ -1062,7 +1055,6 @@ func (qapi *QueryAPI) labelValues(r *http.Request) (interface{}, []error, *api.A
 		storeDebugMatchers,
 		0,
 		enablePartialResponse,
-		qapi.enableQueryPushdown,
 		true,
 		nil,
 		query.NoopSeriesStatsReporter,
@@ -1155,7 +1147,6 @@ func (qapi *QueryAPI) series(r *http.Request) (interface{}, []error, *api.ApiErr
 		storeDebugMatchers,
 		math.MaxInt64,
 		enablePartialResponse,
-		qapi.enableQueryPushdown,
 		true,
 		nil,
 		query.NoopSeriesStatsReporter,
@@ -1212,7 +1203,6 @@ func (qapi *QueryAPI) labelNames(r *http.Request) (interface{}, []error, *api.Ap
 		storeDebugMatchers,
 		0,
 		enablePartialResponse,
-		qapi.enableQueryPushdown,
 		true,
 		nil,
 		query.NoopSeriesStatsReporter,