Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[enhancement] querier cache: WriteBackCache should be off query path #5083

Merged
merged 10 commits into from
Jan 11, 2022
Merged
67 changes: 61 additions & 6 deletions pkg/storage/chunk/chunk_store_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ import (
"sync"

"github.com/go-kit/log/level"
"github.com/pkg/errors"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promauto"
"github.com/prometheus/common/model"
"github.com/prometheus/prometheus/model/labels"
"github.com/prometheus/prometheus/promql"
Expand All @@ -16,6 +19,15 @@ import (
"github.com/grafana/loki/pkg/storage/chunk/cache"
)

var (
errMemcachedAsyncBufferFull = errors.New("the async buffer is full")
reasonAsyncBufferFull = "async-buffer-full"
skipped = promauto.NewCounterVec(prometheus.CounterOpts{
Name: "loki_chunk_fetcher_cache_skipped_total",
Help: "Total number of operations against cache that have been skipped.",
}, []string{"reason"})
dannykopping marked this conversation as resolved.
Show resolved Hide resolved
)

const chunkDecodeParallelism = 16

func filterChunksByTime(from, through model.Time, chunks []Chunk) []Chunk {
Expand Down Expand Up @@ -89,6 +101,12 @@ type Fetcher struct {

wait sync.WaitGroup
decodeRequests chan decodeRequest

maxAsyncConcurrency int
maxAsyncBufferSize int

asyncQueue chan []Chunk
stop chan struct{}
}

type decodeRequest struct {
Expand All @@ -105,26 +123,60 @@ type decodeResponse struct {
// NewChunkFetcher makes a new ChunkFetcher.
func NewChunkFetcher(cacher cache.Cache, cacheStubs bool, schema SchemaConfig, storage Client) (*Fetcher, error) {
c := &Fetcher{
schema: schema,
storage: storage,
cache: cacher,
cacheStubs: cacheStubs,
decodeRequests: make(chan decodeRequest),
schema: schema,
storage: storage,
cache: cacher,
cacheStubs: cacheStubs,
decodeRequests: make(chan decodeRequest),
maxAsyncConcurrency: 16,
maxAsyncBufferSize: 1000,
dannykopping marked this conversation as resolved.
Show resolved Hide resolved
stop: make(chan struct{}, 1),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to make this a buffered channel since you are just using it to notify goroutines to stop by closing it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done,thanks .

}

c.wait.Add(chunkDecodeParallelism)
for i := 0; i < chunkDecodeParallelism; i++ {
go c.worker()
}

// Start a number of goroutines - processing async operations - equal
// to the max concurrency we have.
c.asyncQueue = make(chan []Chunk, c.maxAsyncBufferSize)
for i := 0; i < c.maxAsyncConcurrency; i++ {
go c.asyncQueueProcessLoop()
}

return c, nil
}

func (c *Fetcher) writeBackCacheAsync(fromStorage []Chunk) error {
select {
case c.asyncQueue <- fromStorage:
return nil
default:
return errMemcachedAsyncBufferFull
dannykopping marked this conversation as resolved.
Show resolved Hide resolved
}
}

func (c *Fetcher) asyncQueueProcessLoop() {
dannykopping marked this conversation as resolved.
Show resolved Hide resolved
for {
select {
case fromStorage := <-c.asyncQueue:
cacheErr := c.writeBackCache(context.Background(), fromStorage)
if cacheErr != nil {
level.Warn(util_log.Logger).Log("msg", "could not store chunks in chunk cache", "err", cacheErr)
liguozhong marked this conversation as resolved.
Show resolved Hide resolved
}
case <-c.stop:
return
}
}
}

// Stop the ChunkFetcher.
func (c *Fetcher) Stop() {
close(c.decodeRequests)
c.wait.Wait()
c.cache.Stop()
close(c.stop)
}

func (c *Fetcher) worker() {
Expand Down Expand Up @@ -162,7 +214,10 @@ func (c *Fetcher) FetchChunks(ctx context.Context, chunks []Chunk, keys []string
}

// Always cache any chunks we did get
if cacheErr := c.writeBackCache(ctx, fromStorage); cacheErr != nil {
if cacheErr := c.writeBackCacheAsync(fromStorage); cacheErr != nil {
if cacheErr == errMemcachedAsyncBufferFull {
dannykopping marked this conversation as resolved.
Show resolved Hide resolved
skipped.WithLabelValues(reasonAsyncBufferFull).Inc()
}
level.Warn(log).Log("msg", "could not store chunks in chunk cache", "err", cacheErr)
}

Expand Down