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

ingester: Improve error message when thanos.shipper.json is invalid #1242

Closed
wants to merge 1 commit into from

Conversation

aknuds1
Copy link
Contributor

@aknuds1 aknuds1 commented Feb 21, 2022

What this PR does

Return a more descriptive error message when the ingester fails to update cached shipped blocks due to thanos.shipper.json containing invalid JSON. Also adding tests.

Which issue(s) this PR fixes

Fixes #1241.

Checklist

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

@aknuds1 aknuds1 requested a review from a team February 21, 2022 04:31
@aknuds1 aknuds1 added the enhancement New feature or request label Feb 21, 2022
@aknuds1 aknuds1 changed the title ingester: Imprve error message when thanos.shipper.json is invalid ingester: Improve error message when thanos.shipper.json is invalid Feb 21, 2022
@aknuds1 aknuds1 force-pushed the chore/improve-shipper-read-meta branch 2 times, most recently from b96d7bf to b83b838 Compare February 21, 2022 05:22
@colega
Copy link
Contributor

colega commented Feb 21, 2022

Maybe it's worth upstreaming this change to Thanos and leaving a TODO here to update once it's merged there?

@aknuds1
Copy link
Contributor Author

aknuds1 commented Feb 21, 2022

Maybe it's worth upstreaming this change to Thanos and leaving a TODO here to update once it's merged there?

@colega could be - I'm also unsure if we want to keep depending on Thanos in the future?

@colega
Copy link
Contributor

colega commented Feb 21, 2022

I don't know 🤷

@aknuds1
Copy link
Contributor Author

aknuds1 commented Feb 21, 2022

Maybe it's worth upstreaming this change to Thanos and leaving a TODO here to update once it's merged there?

@colega @bboreham recommends upstreaming the change, so I'll make a PR.

@aknuds1 aknuds1 force-pushed the chore/improve-shipper-read-meta branch from b83b838 to 34dd389 Compare February 21, 2022 11:45
@aknuds1 aknuds1 marked this pull request as draft February 21, 2022 14:26
@aknuds1
Copy link
Contributor Author

aknuds1 commented Feb 21, 2022

Converting PR to draft, since I've made an upstream PR instead and consensus is to wait for upstream to review.

@pracucci
Copy link
Collaborator

This is a copy-paste message.

I've just cut the CHANGELOG in preparation to Mimir 2.0.0 release. Could you rebase main and make sure any CHANGELOG entry added by this PR is at the top of the CHANGELOG, under the "main / unreleased" section, please?

Return a more descriptive error message when the ingester fails to
update cached shipped blocks due to thanos.shipper.json containing
invalid JSON.

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
@aknuds1 aknuds1 force-pushed the chore/improve-shipper-read-meta branch from 34dd389 to d4fb463 Compare February 21, 2022 16:14
@aknuds1
Copy link
Contributor Author

aknuds1 commented Feb 22, 2022

Closing this since upstream PR is merged.

@aknuds1 aknuds1 closed this Feb 22, 2022
@aknuds1 aknuds1 deleted the chore/improve-shipper-read-meta branch February 22, 2022 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle when shipper metadata file is non-JSON
3 participants