Revert "Ingester.QueryStream: Add support for ignoring context cancel…

…lation for chunk queriers (#6408)"

This reverts commit f737a96.

Signed-off-by: Arve Knudsen <[email protected]>

CHANGELOG.md

@@ -23,7 +23,6 @@
 * [CHANGE] Upgrade Node.js to v20. #6540
 * [FEATURE] Query-frontend: add experimental support for query blocking. Queries are blocked on a per-tenant basis and is configured via the limit `blocked_queries`. #5609
 * [FEATURE] Vault: Added support for new Vault authentication methods: `AppRole`, `Kubernetes`, `UserPass` and `Token`. #6143
-* [FEATURE] Ingester: Experimental support for ignoring context cancellation when querying chunks, useful in ruling out the query engine's potential role in unexpected query cancellations. Enable with `-ingester.chunks-query-ignore-cancellation`. #6408
 * [ENHANCEMENT] Ingester: exported summary `cortex_ingester_inflight_push_requests_summary` tracking total number of inflight requests in percentile buckets. #5845
 * [ENHANCEMENT] Query-scheduler: add `cortex_query_scheduler_enqueue_duration_seconds` metric that records the time taken to enqueue or reject a query request. #5879
 * [ENHANCEMENT] Query-frontend: add `cortex_query_frontend_enqueue_duration_seconds` metric that records the time taken to enqueue or reject a query request. When query-scheduler is in use, the metric has the `scheduler_address` label to differentiate the enqueue duration by query-scheduler backend. #5879 #6087 #6120

cmd/mimir/config-descriptor.json

@@ -2958,17 +2958,6 @@
           "fieldType": "int",
           "fieldCategory": "experimental"
         },
-        {
-          "kind": "field",
-          "name": "chunks_query_ignore_cancellation",
-          "required": false,
-          "desc": "Ignore cancellation when querying chunks.",
-          "fieldValue": null,
-          "fieldDefaultValue": false,
-          "fieldFlag": "ingester.chunks-query-ignore-cancellation",
-          "fieldType": "boolean",
-          "fieldCategory": "experimental"
-        },
         {
           "kind": "field",
           "name": "return_only_grpc_errors",

cmd/mimir/help-all.txt.tmpl

@@ -1237,8 +1237,6 @@ Usage of ./cmd/mimir/mimir:
     	After what time a series is considered to be inactive. (default 10m0s)
   -ingester.active-series-metrics-update-period duration
     	How often to update active series metrics. (default 1m0s)
-  -ingester.chunks-query-ignore-cancellation
-    	[experimental] Ignore cancellation when querying chunks.
   -ingester.client.backoff-max-period duration
     	Maximum delay when backing off. (default 10s)
   -ingester.client.backoff-min-period duration

docs/sources/mimir/configure/about-versioning.md

@@ -110,8 +110,6 @@ The following features are currently experimental:
     - `-ingester.client.circuit-breaker.failure-execution-threshold`
     - `-ingester.client.circuit-breaker.period`
     - `-ingester.client.circuit-breaker.cooldown-period`
-- Ignoring chunks query cancellation
-  - `-ingester.chunks-query-ignore-cancellation`
 - Querier
   - Use of Redis cache backend (`-blocks-storage.bucket-store.metadata-cache.backend=redis`)
   - Streaming chunks from ingester to querier (`-querier.prefer-streaming-chunks-from-ingesters`, `-querier.streaming-chunks-per-ingester-buffer-size`)

docs/sources/mimir/references/configuration-parameters/index.md

@@ -1129,10 +1129,6 @@ instance_limits:
 # CLI flag: -ingester.error-sample-rate
 [error_sample_rate: <int> | default = 0]
 
-# (experimental) Ignore cancellation when querying chunks.
-# CLI flag: -ingester.chunks-query-ignore-cancellation
-[chunks_query_ignore_cancellation: <boolean> | default = false]
-
 # (experimental) When enabled only gRPC errors will be returned by the ingester.
 # CLI flag: -ingester.return-only-grpc-errors
 [return_only_grpc_errors: <boolean> | default = false]

pkg/ingester/ingester.go

@@ -182,8 +182,6 @@ type Config struct {
 
 	ErrorSampleRate int64 `yaml:"error_sample_rate" json:"error_sample_rate" category:"experimental"`
 
-	ChunksQueryIgnoreCancellation bool `yaml:"chunks_query_ignore_cancellation" category:"experimental"`
-
 	ReturnOnlyGRPCErrors bool `yaml:"return_only_grpc_errors" json:"return_only_grpc_errors" category:"experimental"`
 }
 
@@ -203,7 +201,6 @@ func (cfg *Config) RegisterFlags(f *flag.FlagSet, logger log.Logger) {
 	f.BoolVar(&cfg.LogUtilizationBasedLimiterCPUSamples, "ingester.log-utilization-based-limiter-cpu-samples", false, "Enable logging of utilization based limiter CPU samples.")
 	f.BoolVar(&cfg.LimitInflightRequestsUsingGrpcMethodLimiter, "ingester.limit-inflight-requests-using-grpc-method-limiter", false, "Use experimental method of limiting push requests.")
 	f.Int64Var(&cfg.ErrorSampleRate, "ingester.error-sample-rate", 0, "Each error will be logged once in this many times. Use 0 to log all of them.")
-	f.BoolVar(&cfg.ChunksQueryIgnoreCancellation, "ingester.chunks-query-ignore-cancellation", false, "Ignore cancellation when querying chunks.")
 	f.BoolVar(&cfg.ReturnOnlyGRPCErrors, "ingester.return-only-grpc-errors", false, "When enabled only gRPC errors will be returned by the ingester.")
 }
 
@@ -1651,8 +1648,6 @@ const queryStreamBatchMessageSize = 1 * 1024 * 1024
 
 // QueryStream streams metrics from a TSDB. This implements the client.IngesterServer interface
 func (i *Ingester) QueryStream(req *client.QueryRequest, stream client.Ingester_QueryStreamServer) error {
-	start := time.Now()
-
 	if err := i.checkRunning(); err != nil {
 		return err
 	}
@@ -1708,23 +1703,12 @@ func (i *Ingester) QueryStream(req *client.QueryRequest, stream client.Ingester_
 	}
 
 	if streamType == QueryStreamChunks {
-		chunksCtx := ctx
-		if i.cfg.ChunksQueryIgnoreCancellation {
-			// Pass an independent context, to help investigating a problem with ingester queries
-			// getting canceled. Prior to https://github.com/grafana/mimir/pull/6085, Prometheus chunk
-			// queriers actually ignored context, so we are emulating that behavior.
-			chunksCtx = context.WithoutCancel(ctx)
-		}
 		if req.StreamingChunksBatchSize > 0 {
 			spanlog.DebugLog("msg", "using executeStreamingQuery")
-			numSeries, numSamples, err = i.executeStreamingQuery(chunksCtx, db, int64(from), int64(through), matchers, shard, stream, req.StreamingChunksBatchSize, spanlog)
+			numSeries, numSamples, err = i.executeStreamingQuery(ctx, db, int64(from), int64(through), matchers, shard, stream, req.StreamingChunksBatchSize, spanlog)
 		} else {
 			spanlog.DebugLog("msg", "using executeChunksQuery")
-			numSeries, numSamples, err = i.executeChunksQuery(chunksCtx, db, int64(from), int64(through), matchers, shard, stream)
-		}
-
-		if i.cfg.ChunksQueryIgnoreCancellation && (ctx.Err() != nil || errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded)) {
-			dumpContextError(ctx, err, start, spanlog)
+			numSeries, numSamples, err = i.executeChunksQuery(ctx, db, int64(from), int64(through), matchers, shard, stream)
 		}
 	} else {
 		spanlog.DebugLog("msg", "using executeSamplesQuery")
@@ -1740,19 +1724,6 @@ func (i *Ingester) QueryStream(req *client.QueryRequest, stream client.Ingester_
 	return nil
 }
 
-// Dump context error for diagnosis.
-func dumpContextError(ctx context.Context, err error, start time.Time, spanlog *spanlogger.SpanLogger) {
-	deadline, deadlineSet := ctx.Deadline()
-	var timeout string
-	if deadlineSet {
-		timeout = fmt.Sprintf("%.2f seconds", deadline.Sub(start).Seconds())
-	} else {
-		timeout = "not set"
-	}
-	level.Debug(spanlog).Log("msg", "query context error", "cause", context.Cause(ctx), "timeout", timeout,
-		"err", err)
-}
-
 func (i *Ingester) executeSamplesQuery(ctx context.Context, db *userTSDB, from, through int64, matchers []*labels.Matcher, shard *sharding.ShardSelector, stream client.Ingester_QueryStreamServer) (numSeries, numSamples int, _ error) {
 	q, err := db.Querier(from, through)
 	if err != nil {