-
Notifications
You must be signed in to change notification settings - Fork 0
Benchmark: grafana PR 97529 #20
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
base: cr-base-97529
Are you sure you want to change the base?
Changes from all commits
6abf2bf
05e864f
6cce48c
a73df83
99053d8
e1ffce4
29fc971
1c085eb
9603245
9b6abbb
5b876e3
29db0d3
1095580
71fcce6
6f4d708
45e0089
26fed31
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -85,9 +85,6 @@ func (b *bleveBackend) BuildIndex(ctx context.Context, | |
| // The builder will write all documents before returning | ||
| builder func(index resource.ResourceIndex) (int64, error), | ||
| ) (resource.ResourceIndex, error) { | ||
| b.cacheMu.Lock() | ||
| defer b.cacheMu.Unlock() | ||
|
|
||
| _, span := b.tracer.Start(ctx, tracingPrexfixBleve+"BuildIndex") | ||
| defer span.End() | ||
|
|
||
|
|
@@ -99,9 +96,9 @@ func (b *bleveBackend) BuildIndex(ctx context.Context, | |
| if size > b.opts.FileThreshold { | ||
| dir := filepath.Join(b.opts.Root, key.Namespace, fmt.Sprintf("%s.%s", key.Resource, key.Group)) | ||
| index, err = bleve.New(dir, mapper) | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why: When concurrent calls to BuildIndex occur for the same key after removing the b.cacheMu lock, both calls execute bleve.New on line 98 using the same directory path, leading to concurrent file access errors or index corruption. 🟠 Unsynchronized file-backed Bleve index creation leads to race conditions Removing agent: |
||
| if err == nil { | ||
| b.log.Info("TODO, check last RV so we can see if the numbers have changed", "dir", dir) | ||
| } | ||
|
|
||
| // TODO, check last RV so we can see if the numbers have changed | ||
|
|
||
| resource.IndexMetrics.IndexTenants.WithLabelValues(key.Namespace, "file").Inc() | ||
| } else { | ||
| index, err = bleve.NewMemOnly(mapper) | ||
|
|
@@ -137,7 +134,9 @@ func (b *bleveBackend) BuildIndex(ctx context.Context, | |
| return nil, err | ||
| } | ||
|
|
||
| b.cacheMu.Lock() | ||
| b.cache[key] = idx | ||
| b.cacheMu.Unlock() | ||
| return idx, nil | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why: ctx is not declared as a parameter or variable in NewResourceServer on line 255; calling s.Init(ctx) on line 258 results in a compile-time error undefined: ctx.
🔴 Undeclared variable
ctxpassed tos.InitNewResourceServeracceptsopts ResourceServerOptionsas its parameter and does not declare or receive actx context.Contextvariable. Invokings.Init(ctx)on line 258 fails compilation withundefined: ctx.agent:
defect· rule:defect.undefined-variable· confidence: 0.95