Skip to content

Commit

Permalink
Merge pull request #823 from grafana/revert-820-charleskorn/pfmc-span…
Browse files Browse the repository at this point in the history
…-logging

Revert "Include cache entry timestamp in trace events emitted by postings for matchers cache"
  • Loading branch information
charleskorn authored Jan 20, 2025
2 parents c9e2f54 + fe442cb commit a49debd
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 12 deletions.
13 changes: 2 additions & 11 deletions tsdb/postings_for_matchers_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,6 @@ type postingsForMatcherPromise struct {
// callers contexts get canceled.
callersCtxTracker *contextsTracker

// Keep track of the time this promise was evaluated, so we can understand the age of this cache entry in traces.
evaluatedAt time.Time

// The result of the promise is stored either in cloner or err (only of the two is valued).
// Do not access these fields until the done channel is closed.
done chan struct{}
Expand Down Expand Up @@ -172,9 +169,7 @@ func (c *PostingsForMatchersCache) postingsForMatchersPromise(ctx context.Contex
span := trace.SpanFromContext(ctx)

promiseCallersCtxTracker, promiseExecCtx := newContextsTracker()
ts := c.timeNow()
promise := &postingsForMatcherPromise{
evaluatedAt: ts,
done: make(chan struct{}),
callersCtxTracker: promiseCallersCtxTracker,
}
Expand Down Expand Up @@ -218,16 +213,12 @@ func (c *PostingsForMatchersCache) postingsForMatchersPromise(ctx context.Contex

span.AddEvent("using cached postingsForMatchers promise", trace.WithAttributes(
attribute.String("cache_key", key),
attribute.Int64("cache_entry_evaluated_at", promise.evaluatedAt.Unix()),
))

return oldPromise.result
}

span.AddEvent("no postingsForMatchers promise in cache, executing query", trace.WithAttributes(
attribute.String("cache_key", key),
attribute.Int64("cache_entry_evaluated_at", promise.evaluatedAt.Unix()),
))
span.AddEvent("no postingsForMatchers promise in cache, executing query", trace.WithAttributes(attribute.String("cache_key", key)))

// promise was stored, close its channel after fulfilment
defer close(promise.done)
Expand All @@ -250,7 +241,7 @@ func (c *PostingsForMatchersCache) postingsForMatchersPromise(ctx context.Contex

sizeBytes := int64(len(key) + size.Of(promise))

c.onPromiseExecutionDone(ctx, key, ts, sizeBytes, promise.err)
c.onPromiseExecutionDone(ctx, key, c.timeNow(), sizeBytes, promise.err)
return promise.result
}

Expand Down
2 changes: 1 addition & 1 deletion tsdb/postings_for_matchers_cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ func TestPostingsForMatchersCache(t *testing.T) {
t.Run("cached value is evicted because cache exceeds max bytes", func(t *testing.T) {
const (
maxItems = 100 // Never hit it.
maxBytes = 1300
maxBytes = 1250
numMatchers = 5
postingsListSize = 30 // 8 bytes per posting ref, so 30 x 8 = 240 bytes.
)
Expand Down

0 comments on commit a49debd

Please sign in to comment.