Skip to content

Commit 1e5298e

Browse files
bborehampracucci
bboreham
and
pracucci
authored
ingester: don't log errors that cause OOMs, using interface (grafana#5581)
* Don't log errors that cause OOMs, using interface Using weaveworks/common#299 which is a more flexible approach. Note the check disappeared from `logging.go`, because it was a mistake to check that error. It comes from `io.Writer`, it won't be an app- level error. * Improved TestIngester_inflightPushRequests Signed-off-by: Marco Pracucci <[email protected]> --------- Signed-off-by: Marco Pracucci <[email protected]> Co-authored-by: Marco Pracucci <[email protected]>
1 parent 8bb9493 commit 1e5298e

File tree

10 files changed

+37
-27
lines changed

10 files changed

+37
-27
lines changed

‎go.mod‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ require (
3838
github.com/spf13/afero v1.9.5
3939
github.com/stretchr/testify v1.8.4
4040
github.com/uber/jaeger-client-go v2.30.0+incompatible
41-
github.com/weaveworks/common v0.0.0-20230714173453-d1f8877b91ce
41+
github.com/weaveworks/common v0.0.0-20230728070032-dd9e68f319d5
4242
go.uber.org/atomic v1.11.0
4343
go.uber.org/goleak v1.2.1
4444
golang.org/x/crypto v0.11.0
@@ -268,6 +268,3 @@ replace github.com/munnerz/goautoneg => github.com/charleskorn/goautoneg v0.0.0-
268268

269269
// Replace opentracing-contrib/go-stdlib with a fork until https://github.com/opentracing-contrib/go-stdlib/pull/68 is merged.
270270
replace github.com/opentracing-contrib/go-stdlib => github.com/grafana/opentracing-contrib-go-stdlib v0.0.0-20230509071955-f410e79da956
271-
272-
// Use a fork of weaveworks/common while we work out if there is a better design for https://github.com/weaveworks/common/pull/293
273-
replace github.com/weaveworks/common => github.com/weaveworks/common v0.0.0-20230714173453-d1f8877b91ce

‎go.sum‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1264,8 +1264,8 @@ github.com/uber/jaeger-lib v2.2.0+incompatible/go.mod h1:ComeNDZlWwrWnDv8aPp0Ba6
12641264
github.com/uber/jaeger-lib v2.4.1+incompatible h1:td4jdvLcExb4cBISKIpHuGoVXh+dVKhn2Um6rjCsSsg=
12651265
github.com/uber/jaeger-lib v2.4.1+incompatible/go.mod h1:ComeNDZlWwrWnDv8aPp0Ba6+uUTzImX/AauajbLI56U=
12661266
github.com/vultr/govultr/v2 v2.17.2 h1:gej/rwr91Puc/tgh+j33p/BLR16UrIPnSr+AIwYWZQs=
1267-
github.com/weaveworks/common v0.0.0-20230714173453-d1f8877b91ce h1:8Gh0psRT42deLI53RVwLq2idJtRtDbdjhdX5LqnVznU=
1268-
github.com/weaveworks/common v0.0.0-20230714173453-d1f8877b91ce/go.mod h1:rgbeLfJUtEr+G74cwFPR1k/4N0kDeaeSv/qhUNE4hm8=
1267+
github.com/weaveworks/common v0.0.0-20230728070032-dd9e68f319d5 h1:nORobjToZAvi54wcuUXLq+XG2Rsr0XEizy5aHBHvqWQ=
1268+
github.com/weaveworks/common v0.0.0-20230728070032-dd9e68f319d5/go.mod h1:rgbeLfJUtEr+G74cwFPR1k/4N0kDeaeSv/qhUNE4hm8=
12691269
github.com/weaveworks/promrus v1.2.0 h1:jOLf6pe6/vss4qGHjXmGz4oDJQA+AOCqEL3FvvZGz7M=
12701270
github.com/weaveworks/promrus v1.2.0/go.mod h1:SaE82+OJ91yqjrE1rsvBWVzNZKcHYFtMUyS1+Ogs/KA=
12711271
github.com/xdg-go/pbkdf2 v1.0.0/go.mod h1:jrpuAogTd400dnrH08LKmI/xc1MbPOebTwRqcT5RDeI=

‎pkg/distributor/distributor.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ import (
3333
"github.com/prometheus/prometheus/scrape"
3434
"github.com/weaveworks/common/httpgrpc"
3535
"github.com/weaveworks/common/instrument"
36-
"github.com/weaveworks/common/middleware"
3736
"github.com/weaveworks/common/mtime"
3837
"github.com/weaveworks/common/user"
3938
"go.uber.org/atomic"
@@ -45,6 +44,7 @@ import (
4544
"github.com/grafana/mimir/pkg/mimirpb"
4645
"github.com/grafana/mimir/pkg/util"
4746
"github.com/grafana/mimir/pkg/util/globalerror"
47+
util_log "github.com/grafana/mimir/pkg/util/log"
4848
util_math "github.com/grafana/mimir/pkg/util/math"
4949
"github.com/grafana/mimir/pkg/util/pool"
5050
"github.com/grafana/mimir/pkg/util/push"
@@ -1021,7 +1021,7 @@ func (d *Distributor) limitsMiddleware(next push.Func) push.Func {
10211021
il := d.getInstanceLimits()
10221022
if il.MaxInflightPushRequests > 0 && inflight > int64(il.MaxInflightPushRequests) {
10231023
d.rejectedRequests.WithLabelValues(reasonDistributorMaxInflightPushRequests).Inc()
1024-
return nil, middleware.DoNotLogError{Err: errMaxInflightRequestsReached}
1024+
return nil, util_log.DoNotLogError{Err: errMaxInflightRequestsReached}
10251025
}
10261026

10271027
if il.MaxIngestionRate > 0 {

‎pkg/ingester/ingester.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ import (
4747
"github.com/prometheus/prometheus/util/zeropool"
4848
"github.com/thanos-io/objstore"
4949
"github.com/weaveworks/common/httpgrpc"
50-
"github.com/weaveworks/common/middleware"
5150
"go.uber.org/atomic"
5251
"golang.org/x/exp/slices"
5352
"golang.org/x/sync/errgroup"
@@ -715,7 +714,7 @@ func (i *Ingester) PushWithCleanup(ctx context.Context, pushReq *push.Request) (
715714
defer pushReq.CleanUp()
716715

717716
if err := i.checkRunning(); err != nil {
718-
return nil, middleware.DoNotLogError{Err: err}
717+
return nil, util_log.DoNotLogError{Err: err}
719718
}
720719

721720
// We will report *this* request in the error too.

‎pkg/ingester/ingester_test.go‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ import (
6363
"github.com/grafana/mimir/pkg/storage/tsdb/block"
6464
"github.com/grafana/mimir/pkg/usagestats"
6565
"github.com/grafana/mimir/pkg/util"
66+
util_log "github.com/grafana/mimir/pkg/util/log"
6667
util_math "github.com/grafana/mimir/pkg/util/math"
6768
"github.com/grafana/mimir/pkg/util/push"
6869
util_test "github.com/grafana/mimir/pkg/util/test"
@@ -5621,7 +5622,8 @@ func TestIngester_inflightPushRequests(t *testing.T) {
56215622
cfg := defaultIngesterTestConfig(t)
56225623
cfg.InstanceLimitsFn = func() *InstanceLimits { return &limits }
56235624

5624-
i, err := prepareIngesterWithBlocksStorage(t, cfg, nil)
5625+
reg := prometheus.NewPedanticRegistry()
5626+
i, err := prepareIngesterWithBlocksStorage(t, cfg, reg)
56255627
require.NoError(t, err)
56265628
require.NoError(t, services.StartAndAwaitRunning(context.Background(), i))
56275629
defer services.StopAndAwaitTerminated(context.Background(), i) //nolint:errcheck
@@ -5688,10 +5690,21 @@ func TestIngester_inflightPushRequests(t *testing.T) {
56885690

56895691
_, err := i.Push(ctx, req)
56905692
require.Equal(t, errMaxInflightRequestsReached, err)
5693+
require.ErrorAs(t, err, &util_log.DoNotLogError{})
56915694
return nil
56925695
})
56935696

56945697
require.NoError(t, g.Wait())
5698+
5699+
// Ensure the rejected request has been tracked in a metric.
5700+
require.NoError(t, testutil.GatherAndCompare(reg, strings.NewReader(`
5701+
# HELP cortex_ingester_instance_rejected_requests_total Requests rejected for hitting per-instance limits
5702+
# TYPE cortex_ingester_instance_rejected_requests_total counter
5703+
cortex_ingester_instance_rejected_requests_total{reason="ingester_max_inflight_push_requests"} 1
5704+
cortex_ingester_instance_rejected_requests_total{reason="ingester_max_ingestion_rate"} 0
5705+
cortex_ingester_instance_rejected_requests_total{reason="ingester_max_series"} 0
5706+
cortex_ingester_instance_rejected_requests_total{reason="ingester_max_tenants"} 0
5707+
`), "cortex_ingester_instance_rejected_requests_total"))
56955708
}
56965709

56975710
func generateSamplesForLabel(baseLabels labels.Labels, series, samples int) *mimirpb.WriteRequest {

‎pkg/ingester/instance_limits.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,10 @@ import (
99
"flag"
1010

1111
"github.com/pkg/errors"
12-
"github.com/weaveworks/common/middleware"
1312
"gopkg.in/yaml.v3"
1413

1514
"github.com/grafana/mimir/pkg/util/globalerror"
15+
util_log "github.com/grafana/mimir/pkg/util/log"
1616
)
1717

1818
const (
@@ -27,7 +27,7 @@ var (
2727
errMaxIngestionRateReached = errors.New(globalerror.IngesterMaxIngestionRate.MessageWithPerInstanceLimitConfig("the write request has been rejected because the ingester exceeded the samples ingestion rate limit", maxIngestionRateFlag))
2828
errMaxTenantsReached = errors.New(globalerror.IngesterMaxTenants.MessageWithPerInstanceLimitConfig("the write request has been rejected because the ingester exceeded the allowed number of tenants", maxInMemoryTenantsFlag))
2929
errMaxInMemorySeriesReached = errors.New(globalerror.IngesterMaxInMemorySeries.MessageWithPerInstanceLimitConfig("the write request has been rejected because the ingester exceeded the allowed number of in-memory series", maxInMemorySeriesFlag))
30-
errMaxInflightRequestsReached = middleware.DoNotLogError{Err: errors.New(globalerror.IngesterMaxInflightPushRequests.MessageWithPerInstanceLimitConfig("the write request has been rejected because the ingester exceeded the allowed number of inflight push requests", maxInflightPushRequestsFlag))}
30+
errMaxInflightRequestsReached = util_log.DoNotLogError{Err: errors.New(globalerror.IngesterMaxInflightPushRequests.MessageWithPerInstanceLimitConfig("the write request has been rejected because the ingester exceeded the allowed number of inflight push requests", maxInflightPushRequestsFlag))}
3131
)
3232

3333
// InstanceLimits describes limits used by ingester. Reaching any of these will result in Push method to return

‎pkg/util/log/log.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
package log
77

88
import (
9+
"context"
910
"fmt"
1011
"io"
1112
"os"
@@ -105,3 +106,9 @@ func Flush() error {
105106

106107
return nil
107108
}
109+
110+
type DoNotLogError struct{ Err error }
111+
112+
func (i DoNotLogError) Error() string { return i.Err.Error() }
113+
func (i DoNotLogError) Unwrap() error { return i.Err }
114+
func (i DoNotLogError) ShouldLog(_ context.Context, _ time.Duration) bool { return false }

‎vendor/github.com/weaveworks/common/middleware/grpc_logging.go‎

Lines changed: 7 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎vendor/github.com/weaveworks/common/middleware/logging.go‎

Lines changed: 0 additions & 11 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎vendor/modules.txt‎

Lines changed: 1 addition & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)