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

Series Index Store: fix race in GetSeries #10310

Merged
merged 2 commits into from
Aug 23, 2023
Merged

Conversation

bboreham
Copy link
Contributor

@bboreham bboreham commented Aug 22, 2023

What this PR does / why we need it:
Lock around data that is modified from multiple goroutines concurrently.
Replaced with the implementation from #9984 since @akhilanarayanan did it more efficiently.

Which issue(s) this PR fixes:
Relates to #8586

Checklist

  • Reviewed the CONTRIBUTING.md guide (required)
  • NA Documentation added
  • NA Tests updated
  • CHANGELOG.md updated
    • NA If the change is worth mentioning in the release notes, add add-to-release-notes label
  • NA Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
  • NA For Helm chart changes bump the Helm chart version in production/helm/loki/Chart.yaml and update production/helm/loki/CHANGELOG.md and production/helm/loki/README.md. Example PR

@bboreham bboreham requested a review from a team as a code owner August 22, 2023 16:33
@bboreham
Copy link
Contributor Author

I prefer the fix in #9984, unless this routine is unimportant for performance.

Collect the individual results per-job in a slice of slices, then
flatten them out when all jobs are finished.
Copy link
Contributor

@chaudum chaudum left a comment

Choose a reason for hiding this comment

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

LGTM

@kavirajk kavirajk merged commit cf353bb into grafana:main Aug 23, 2023
2 checks passed
@kavirajk kavirajk added type/bug Somehing is not working as expected backport release-2.9.x labels Aug 25, 2023
grafanabot pushed a commit that referenced this pull request Aug 25, 2023
kavirajk pushed a commit that referenced this pull request Aug 25, 2023
Backport cf353bb from #10310

---

**What this PR does / why we need it**:
~Lock around data that is modified from multiple goroutines
concurrently.~
Replaced with the implementation from #9984 since @akhilanarayanan did
it more efficiently.

**Which issue(s) this PR fixes**:
Relates to #8586

**Checklist**
- [x] Reviewed the
[`CONTRIBUTING.md`](https://github.com/grafana/loki/blob/main/CONTRIBUTING.md)
guide (**required**)
- NA Documentation added
- NA Tests updated
- [x] `CHANGELOG.md` updated
- NA If the change is worth mentioning in the release notes, add
`add-to-release-notes` label
- NA Changes that require user attention or interaction to upgrade are
documented in `docs/sources/setup/upgrade/_index.md`
- NA For Helm chart changes bump the Helm chart version in
`production/helm/loki/Chart.yaml` and update
`production/helm/loki/CHANGELOG.md` and
`production/helm/loki/README.md`. [Example
PR](d10549e)

Co-authored-by: Bryan Boreham <[email protected]>
@bboreham bboreham deleted the series-race branch September 9, 2024 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport release-2.9.x size/XS type/bug Somehing is not working as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants