Repository: prometheus/prometheus (language: go)

Issue:
Index symbols lookup spent too much CPU on ctx check
### What did you do?

<img width="663" alt="image" src="https://github.com/prometheus/prometheus/assets/25150124/9e42bda3-264c-478c-8526-93dac349a602">

I am running Cortex with latest version of Prometheus. `index.Symbols.Lookup` function took 7% CPU time on profile but about 4% CPU time was spent on checking context. The check seems too fine grained.

### What did you expect to see?

I expect context check should take a very small portion of CPU time.

### What did you see instead? Under which circumstances?

.


Discussion / hints:
@aknuds1 I think you added this at https://github.com/prometheus/prometheus/pull/12666/files#diff-fb0f63e7d2d24fa7aa1c18b2619b10a07c821b38f45c2351458b00edef63f995R1310
Thanks for the heads up @bboreham - I guess evidence points towards dropping that check?
I suggest you consider what is the maximum time each operation can take between checks for cancellation.

I'd guess we want this to be in milliseconds, maybe 10-100ms is ok, and not microseconds or nanoseconds.

Then you can reason where it is appropriate to check and where not.

Fix patch:
diff --git a/tsdb/index/index.go b/tsdb/index/index.go
index ccf06e8eab8..bbeceda0c8c 100644
--- a/tsdb/index/index.go
+++ b/tsdb/index/index.go
@@ -923,7 +923,7 @@ func (w *Writer) writePostingsToTmpFiles() error {
 			// Symbol numbers are in order, so the strings will also be in order.
 			slices.Sort(values)
 			for _, v := range values {
-				value, err := w.symbols.Lookup(w.ctx, v)
+				value, err := w.symbols.Lookup(v)
 				if err != nil {
 					return err
 				}
@@ -1295,7 +1295,7 @@ func NewSymbols(bs ByteSlice, version, off int) (*Symbols, error) {
 	return s, nil
 }
 
-func (s Symbols) Lookup(ctx context.Context, o uint32) (string, error) {
+func (s Symbols) Lookup(o uint32) (string, error) {
 	d := encoding.Decbuf{
 		B: s.bs.Range(0, s.bs.Len()),
 	}
@@ -1307,9 +1307,6 @@ func (s Symbols) Lookup(ctx context.Context, o uint32) (string, error) {
 		d.Skip(s.offsets[int(o/symbolFactor)])
 		// Walk until we find the one we want.
 		for i := o - (o / symbolFactor * symbolFactor); i > 0; i-- {
-			if ctx.Err() != nil {
-				return "", ctx.Err()
-			}
 			d.UvarintBytes()
 		}
 	} else {
@@ -1441,7 +1438,7 @@ func (r *Reader) lookupSymbol(ctx context.Context, o uint32) (string, error) {
 	if s, ok := r.nameSymbols[o]; ok {
 		return s, nil
 	}
-	return r.symbols.Lookup(ctx, o)
+	return r.symbols.Lookup(o)
 }
 
 // Symbols returns an iterator over the symbols that exist within the index.


Test patch:
diff --git a/tsdb/index/index_test.go b/tsdb/index/index_test.go
index abc14779762..7a6683da2bc 100644
--- a/tsdb/index/index_test.go
+++ b/tsdb/index/index_test.go
@@ -519,7 +519,6 @@ func TestNewFileReaderErrorNoOpenFiles(t *testing.T) {
 }
 
 func TestSymbols(t *testing.T) {
-	ctx := context.Background()
 	buf := encoding.Encbuf{}
 
 	// Add prefix to the buffer to simulate symbols as part of larger buffer.
@@ -542,11 +541,11 @@ func TestSymbols(t *testing.T) {
 	require.Equal(t, 32, s.Size())
 
 	for i := 99; i >= 0; i-- {
-		s, err := s.Lookup(ctx, uint32(i))
+		s, err := s.Lookup(uint32(i))
 		require.NoError(t, err)
 		require.Equal(t, string(rune(i)), s)
 	}
-	_, err = s.Lookup(ctx, 100)
+	_, err = s.Lookup(100)
 	require.Error(t, err)
 
 	for i := 99; i >= 0; i-- {