Skip to content

Reuse request timeseries slice on error in the ingester#1863

Closed
pracucci wants to merge 1 commit intocortexproject:masterfrom
pracucci:fix-reuse-slice-on-error
Closed

Reuse request timeseries slice on error in the ingester#1863
pracucci wants to merge 1 commit intocortexproject:masterfrom
pracucci:fix-reuse-slice-on-error

Conversation

@pracucci
Copy link
Contributor

What this PR does:
If I'm not missing anything, this should be a safe change to do, to make sure a request timeseries slice gets reused (in the ingester) even in case of error.

Which issue(s) this PR fixes:
No issue

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Signed-off-by: Marco Pracucci <marco@pracucci.com>
@bboreham
Copy link
Contributor

Which kinds of error did you have in mind?
The benefit seems marginal, given defer has a cost on the happy path.

@pracucci
Copy link
Contributor Author

You're right. Re-checking the code, soft errors (like out of order timestamp) did reuse the slice, so slices were not reused only in case of hard errors, which are hard.

Closing it, thanks for pointing out.

@pracucci pracucci closed this Nov 29, 2019
@pracucci pracucci deleted the fix-reuse-slice-on-error branch November 29, 2019 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants