Skip to content

Commit

Permalink
[release-2.8.x] Ruler: protect overrides map with mutex when accessin…
Browse files Browse the repository at this point in the history
…g tenant configs (#11602)

Backport cd3cf62 from #11601
  • Loading branch information
grafanabot committed Jan 8, 2024
1 parent 1dfdc43 commit 815e0f5
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
##### Fixes

* [10375](https://github.com/grafana/loki/pull/10375) **trevorwhitney**: Fix ingester query when getting label values by passing matchers
* [11601](https://github.com/grafana/loki/pull/11601) **dannykopping** Ruler: Fixed a panic that can be caused by concurrent read-write access of tenant configs when there are a large amount of rules.

### All Changes

Expand Down
7 changes: 6 additions & 1 deletion pkg/ruler/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"net/url"
"strings"
"sync"
"time"

"github.com/go-kit/log"
Expand Down Expand Up @@ -33,7 +34,8 @@ type walRegistry struct {
logger log.Logger
manager instance.Manager

metrics *storageRegistryMetrics
metrics *storageRegistryMetrics
overridesMu sync.Mutex

config Config
overrides RulesLimits
Expand Down Expand Up @@ -223,6 +225,9 @@ func (r *walRegistry) getTenantConfig(tenant string) (instance.Config, error) {
}

func (r *walRegistry) getTenantRemoteWriteConfig(tenant string, base RemoteWriteConfig) (*RemoteWriteConfig, error) {
r.overridesMu.Lock()
defer r.overridesMu.Unlock()

overrides, err := base.Clone()
if err != nil {
return nil, fmt.Errorf("error generating tenant remote-write config: %w", err)
Expand Down
27 changes: 27 additions & 0 deletions pkg/ruler/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"net/url"
"strings"
"sync"
"testing"
"time"

Expand Down Expand Up @@ -376,6 +377,32 @@ func TestTenantRemoteWriteConfigWithOverride(t *testing.T) {
assert.ElementsMatch(t, actual, expected, "QueueConfig capacity do not match")
}

func TestTenantRemoteWriteConfigWithOverrideConcurrentAccess(t *testing.T) {
require.NotPanics(t, func() {
reg := setupRegistry(t, cfg, newFakeLimits())
var wg sync.WaitGroup
for i := 0; i < 1000; i++ {
wg.Add(1)
go func(reg *walRegistry) {
defer wg.Done()

_, err := reg.getTenantConfig(enabledRWTenant)
require.NoError(t, err)
}(reg)

wg.Add(1)
go func(reg *walRegistry) {
defer wg.Done()

_, err := reg.getTenantConfig(additionalHeadersRWTenant)
require.NoError(t, err)
}(reg)
}

wg.Wait()
})
}

func TestTenantRemoteWriteConfigWithoutOverride(t *testing.T) {
reg := setupRegistry(t, backCompatCfg, newFakeLimitsBackwardCompat())

Expand Down

0 comments on commit 815e0f5

Please sign in to comment.