Merge pull request #12003 from codesome/redundant-chunk-access

Remove unnecessary chunk fetch in Head queries
This commit is contained in:
Ganesh Vernekar 2023-02-22 12:57:38 +05:30 committed by GitHub
commit 66da1d51fd
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 29 additions and 37 deletions

View file

@ -308,8 +308,6 @@ func (h *headChunkReader) Chunk(meta chunks.Meta) (chunkenc.Chunk, error) {
s: s, s: s,
cid: cid, cid: cid,
isoState: h.isoState, isoState: h.isoState,
chunkDiskMapper: h.head.chunkDiskMapper,
memChunkPool: &h.head.memChunkPool,
}, nil }, nil
} }
@ -603,40 +601,21 @@ type safeChunk struct {
s *memSeries s *memSeries
cid chunks.HeadChunkID cid chunks.HeadChunkID
isoState *isolationState isoState *isolationState
chunkDiskMapper *chunks.ChunkDiskMapper
memChunkPool *sync.Pool
} }
func (c *safeChunk) Iterator(reuseIter chunkenc.Iterator) chunkenc.Iterator { func (c *safeChunk) Iterator(reuseIter chunkenc.Iterator) chunkenc.Iterator {
c.s.Lock() c.s.Lock()
it := c.s.iterator(c.cid, c.isoState, c.chunkDiskMapper, c.memChunkPool, reuseIter) it := c.s.iterator(c.cid, c.Chunk, c.isoState, reuseIter)
c.s.Unlock() c.s.Unlock()
return it return it
} }
// iterator returns a chunk iterator for the requested chunkID, or a NopIterator if the requested ID is out of range. // iterator returns a chunk iterator for the requested chunkID, or a NopIterator if the requested ID is out of range.
// It is unsafe to call this concurrently with s.append(...) without holding the series lock. // It is unsafe to call this concurrently with s.append(...) without holding the series lock.
func (s *memSeries) iterator(id chunks.HeadChunkID, isoState *isolationState, chunkDiskMapper *chunks.ChunkDiskMapper, memChunkPool *sync.Pool, it chunkenc.Iterator) chunkenc.Iterator { func (s *memSeries) iterator(id chunks.HeadChunkID, c chunkenc.Chunk, isoState *isolationState, it chunkenc.Iterator) chunkenc.Iterator {
c, garbageCollect, err := s.chunk(id, chunkDiskMapper, memChunkPool)
// TODO(fabxc): Work around! An error will be returns when a querier have retrieved a pointer to a
// series's chunk, which got then garbage collected before it got
// accessed. We must ensure to not garbage collect as long as any
// readers still hold a reference.
if err != nil {
return chunkenc.NewNopIterator()
}
defer func() {
if garbageCollect {
// Set this to nil so that Go GC can collect it after it has been used.
// This should be done always at the end.
c.chunk = nil
memChunkPool.Put(c)
}
}()
ix := int(id) - int(s.firstChunkID) ix := int(id) - int(s.firstChunkID)
numSamples := c.chunk.NumSamples() numSamples := c.NumSamples()
stopAfter := numSamples stopAfter := numSamples
if isoState != nil && !isoState.IsolationDisabled() { if isoState != nil && !isoState.IsolationDisabled() {
@ -681,9 +660,9 @@ func (s *memSeries) iterator(id chunks.HeadChunkID, isoState *isolationState, ch
return chunkenc.NewNopIterator() return chunkenc.NewNopIterator()
} }
if stopAfter == numSamples { if stopAfter == numSamples {
return c.chunk.Iterator(it) return c.Iterator(it)
} }
return makeStopIterator(c.chunk, it, stopAfter) return makeStopIterator(c, it, stopAfter)
} }
// stopIterator wraps an Iterator, but only returns the first // stopIterator wraps an Iterator, but only returns the first

View file

@ -537,11 +537,18 @@ func TestHead_ReadWAL(t *testing.T) {
require.NoError(t, c.Err()) require.NoError(t, c.Err())
return x return x
} }
require.Equal(t, []sample{{100, 2, nil, nil}, {101, 5, nil, nil}}, expandChunk(s10.iterator(0, nil, head.chunkDiskMapper, nil, nil)))
require.Equal(t, []sample{{101, 6, nil, nil}}, expandChunk(s50.iterator(0, nil, head.chunkDiskMapper, nil, nil))) c, _, err := s10.chunk(0, head.chunkDiskMapper, &head.memChunkPool)
require.NoError(t, err)
require.Equal(t, []sample{{100, 2, nil, nil}, {101, 5, nil, nil}}, expandChunk(c.chunk.Iterator(nil)))
c, _, err = s50.chunk(0, head.chunkDiskMapper, &head.memChunkPool)
require.NoError(t, err)
require.Equal(t, []sample{{101, 6, nil, nil}}, expandChunk(c.chunk.Iterator(nil)))
// The samples before the new series record should be discarded since a duplicate record // The samples before the new series record should be discarded since a duplicate record
// is only possible when old samples were compacted. // is only possible when old samples were compacted.
require.Equal(t, []sample{{101, 7, nil, nil}}, expandChunk(s100.iterator(0, nil, head.chunkDiskMapper, nil, nil))) c, _, err = s100.chunk(0, head.chunkDiskMapper, &head.memChunkPool)
require.NoError(t, err)
require.Equal(t, []sample{{101, 7, nil, nil}}, expandChunk(c.chunk.Iterator(nil)))
q, err := head.ExemplarQuerier(context.Background()) q, err := head.ExemplarQuerier(context.Background())
require.NoError(t, err) require.NoError(t, err)
@ -2566,7 +2573,13 @@ func TestIteratorSeekIntoBuffer(t *testing.T) {
require.True(t, ok, "sample append failed") require.True(t, ok, "sample append failed")
} }
it := s.iterator(s.headChunkID(len(s.mmappedChunks)), nil, chunkDiskMapper, nil, nil) c, _, err := s.chunk(0, chunkDiskMapper, &sync.Pool{
New: func() interface{} {
return &memChunk{}
},
})
require.NoError(t, err)
it := c.chunk.Iterator(nil)
// First point. // First point.
require.Equal(t, chunkenc.ValFloat, it.Seek(0)) require.Equal(t, chunkenc.ValFloat, it.Seek(0))

0
tsdb/test.txt Normal file
View file