diff --git a/storage/remote/intern.go b/storage/remote/intern.go index 8dc2de4a12..23047acd9b 100644 --- a/storage/remote/intern.go +++ b/storage/remote/intern.go @@ -23,8 +23,6 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" - "github.com/prometheus/prometheus/model/labels" - "github.com/prometheus/prometheus/tsdb/chunks" "go.uber.org/atomic" ) @@ -36,33 +34,53 @@ var noReferenceReleases = promauto.NewCounter(prometheus.CounterOpts{ }) type pool struct { - mtx sync.RWMutex - pool map[chunks.HeadSeriesRef]*entry - shouldIntern bool + mtx sync.RWMutex + pool map[string]*entry } type entry struct { refs atomic.Int64 - lset labels.Labels + + s string } -func newEntry(lset labels.Labels) *entry { - return &entry{lset: lset} +func newEntry(s string) *entry { + return &entry{s: s} } -func newPool(shouldIntern bool) *pool { +func newPool() *pool { return &pool{ - pool: map[chunks.HeadSeriesRef]*entry{}, - shouldIntern: shouldIntern, + pool: map[string]*entry{}, } } -func (p *pool) release(ref chunks.HeadSeriesRef) { - if !p.shouldIntern { - return +func (p *pool) intern(s string) string { + if s == "" { + return "" } + p.mtx.RLock() - interned, ok := p.pool[ref] + interned, ok := p.pool[s] + p.mtx.RUnlock() + if ok { + interned.refs.Inc() + return interned.s + } + p.mtx.Lock() + defer p.mtx.Unlock() + if interned, ok := p.pool[s]; ok { + interned.refs.Inc() + return interned.s + } + + p.pool[s] = newEntry(s) + p.pool[s].refs.Store(1) + return s +} + +func (p *pool) release(s string) { + p.mtx.RLock() + interned, ok := p.pool[s] p.mtx.RUnlock() if !ok { @@ -80,33 +98,5 @@ func (p *pool) release(ref chunks.HeadSeriesRef) { if interned.refs.Load() != 0 { return } - delete(p.pool, ref) -} - -func (p *pool) intern(ref chunks.HeadSeriesRef, lset labels.Labels) labels.Labels { - if !p.shouldIntern { - return lset - } - - p.mtx.RLock() - interned, ok := p.pool[ref] - p.mtx.RUnlock() - if ok { - interned.refs.Inc() - return interned.lset - } - p.mtx.Lock() - defer p.mtx.Unlock() - if interned, ok := p.pool[ref]; ok { - interned.refs.Inc() - return interned.lset - } - - if lset.Len() == 0 { - return labels.EmptyLabels() - } - - p.pool[ref] = newEntry(lset) - p.pool[ref].refs.Store(1) - return p.pool[ref].lset + delete(p.pool, s) } diff --git a/storage/remote/intern_test.go b/storage/remote/intern_test.go index 1acc9189a5..917c42e912 100644 --- a/storage/remote/intern_test.go +++ b/storage/remote/intern_test.go @@ -24,82 +24,67 @@ import ( "time" "github.com/stretchr/testify/require" - - "github.com/prometheus/prometheus/model/labels" - "github.com/prometheus/prometheus/tsdb/chunks" ) func TestIntern(t *testing.T) { - interner := newPool(true) - testString := "TestIntern_DeleteRef" - ref := chunks.HeadSeriesRef(1234) - - lset := labels.FromStrings("name", testString) - interner.intern(ref, lset) - interned, ok := interner.pool[ref] + interner := newPool() + testString := "TestIntern" + interner.intern(testString) + interned, ok := interner.pool[testString] require.True(t, ok) - require.Equal(t, lset, interned.lset) require.Equal(t, int64(1), interned.refs.Load(), fmt.Sprintf("expected refs to be 1 but it was %d", interned.refs.Load())) } func TestIntern_MultiRef(t *testing.T) { - interner := newPool(true) - testString := "TestIntern_DeleteRef" - ref := chunks.HeadSeriesRef(1234) + interner := newPool() + testString := "TestIntern_MultiRef" - lset := labels.FromStrings("name", testString) - interner.intern(ref, lset) - interned, ok := interner.pool[ref] + interner.intern(testString) + interned, ok := interner.pool[testString] require.True(t, ok) - require.Equal(t, lset, interned.lset) require.Equal(t, int64(1), interned.refs.Load(), fmt.Sprintf("expected refs to be 1 but it was %d", interned.refs.Load())) - interner.intern(ref, lset) - interned, ok = interner.pool[ref] + interner.intern(testString) + interned, ok = interner.pool[testString] require.True(t, ok) - require.NotNil(t, interned) require.Equal(t, int64(2), interned.refs.Load(), fmt.Sprintf("expected refs to be 2 but it was %d", interned.refs.Load())) } func TestIntern_DeleteRef(t *testing.T) { - interner := newPool(true) + interner := newPool() testString := "TestIntern_DeleteRef" - ref := chunks.HeadSeriesRef(1234) - interner.intern(ref, labels.FromStrings("name", testString)) - interned, ok := interner.pool[ref] - require.NotNil(t, interned) + interner.intern(testString) + interned, ok := interner.pool[testString] + require.True(t, ok) require.Equal(t, int64(1), interned.refs.Load(), fmt.Sprintf("expected refs to be 1 but it was %d", interned.refs.Load())) - interner.release(ref) - _, ok = interner.pool[ref] + interner.release(testString) + _, ok = interner.pool[testString] require.False(t, ok) } func TestIntern_MultiRef_Concurrent(t *testing.T) { - interner := newPool(true) + interner := newPool() testString := "TestIntern_MultiRef_Concurrent" - ref := chunks.HeadSeriesRef(1234) - interner.intern(ref, labels.FromStrings("name", testString)) - interned, ok := interner.pool[ref] - - require.NotNil(t, interned) + interner.intern(testString) + interned, ok := interner.pool[testString] require.True(t, ok) require.Equal(t, int64(1), interned.refs.Load(), fmt.Sprintf("expected refs to be 1 but it was %d", interned.refs.Load())) - go interner.release(ref) + go interner.release(testString) - interner.intern(ref, labels.FromStrings("name", testString)) + interner.intern(testString) time.Sleep(time.Millisecond) interner.mtx.RLock() - interned, ok = interner.pool[ref] + interned, ok = interner.pool[testString] interner.mtx.RUnlock() require.True(t, ok) require.Equal(t, int64(1), interned.refs.Load(), fmt.Sprintf("expected refs to be 1 but it was %d", interned.refs.Load()))