From acddff807753662fc675bca2e9aeec5eb7fb943d Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Sat, 31 Aug 2019 17:24:36 +0000 Subject: [PATCH 1/2] Protect two data structures with one lock, for performance Since `stopped` is only used in one place, that is already taking a lock, re-use that lock and skip the effort to check the other lock and defer unlock it. Signed-off-by: Bryan Boreham --- pkg/ingester/ingester.go | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/pkg/ingester/ingester.go b/pkg/ingester/ingester.go index 0531a58b558..a58616aa067 100644 --- a/pkg/ingester/ingester.go +++ b/pkg/ingester/ingester.go @@ -152,13 +152,12 @@ type Ingester struct { lifecycler *ring.Lifecycler limits *validation.Overrides - stopLock sync.RWMutex - stopped bool - quit chan struct{} - done sync.WaitGroup + quit chan struct{} + done sync.WaitGroup - userStatesMtx sync.RWMutex + userStatesMtx sync.RWMutex // protects userStates and stopped userStates *userStates + stopped bool // protected by userStatesMtx // One queue per flush thread. Fingerprint is used to // pick a queue. @@ -247,8 +246,8 @@ func (i *Ingester) Shutdown() { // StopIncomingRequests is called during the shutdown process. func (i *Ingester) StopIncomingRequests() { - i.stopLock.Lock() - defer i.stopLock.Unlock() + i.userStatesMtx.Lock() + defer i.userStatesMtx.Unlock() i.stopped = true } @@ -286,14 +285,11 @@ func (i *Ingester) Push(ctx old_ctx.Context, req *client.WriteRequest) (*client. func (i *Ingester) append(ctx context.Context, userID string, labels labelPairs, timestamp model.Time, value model.SampleValue, source client.WriteRequest_SourceEnum) error { labels.removeBlanks() - i.stopLock.RLock() - defer i.stopLock.RUnlock() + i.userStatesMtx.RLock() + defer i.userStatesMtx.RUnlock() if i.stopped { return fmt.Errorf("ingester stopping") } - - i.userStatesMtx.RLock() - defer i.userStatesMtx.RUnlock() state, fp, series, err := i.userStates.getOrCreateSeries(ctx, userID, labels) if err != nil { return err From f54a9c0c9ccaaff38f5b2726b001ec3bf053e894 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Sat, 31 Aug 2019 17:31:47 +0000 Subject: [PATCH 2/2] Combine two defers into one, for performance Each defer involves some memory allocation and housekeeping - since they are right next to one another we can save half the effort. Signed-off-by: Bryan Boreham --- pkg/ingester/ingester.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/pkg/ingester/ingester.go b/pkg/ingester/ingester.go index a58616aa067..43bda108eeb 100644 --- a/pkg/ingester/ingester.go +++ b/pkg/ingester/ingester.go @@ -285,18 +285,25 @@ func (i *Ingester) Push(ctx old_ctx.Context, req *client.WriteRequest) (*client. func (i *Ingester) append(ctx context.Context, userID string, labels labelPairs, timestamp model.Time, value model.SampleValue, source client.WriteRequest_SourceEnum) error { labels.removeBlanks() + var ( + state *userState + fp model.Fingerprint + ) i.userStatesMtx.RLock() - defer i.userStatesMtx.RUnlock() + defer func() { + i.userStatesMtx.RUnlock() + if state != nil { + state.fpLocker.Unlock(fp) + } + }() if i.stopped { return fmt.Errorf("ingester stopping") } state, fp, series, err := i.userStates.getOrCreateSeries(ctx, userID, labels) if err != nil { + state = nil // don't want to unlock the fp if there is an error return err } - defer func() { - state.fpLocker.Unlock(fp) - }() prevNumChunks := len(series.chunkDescs) if i.cfg.SpreadFlushes && prevNumChunks > 0 {