Skip to content

Conversation

pstibrany
Copy link
Contributor

@pstibrany pstibrany commented Mar 6, 2020

What this PR does: This PR moves WAL replays into Ingester's Starting method. Ingester cannot operate until these are finished, so all public methods now check if Ingester is in Running or Stopping state. Why Stopping? Because querying methods still work while it is stopping.

Checklist

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

/cc @codesome

Copy link
Contributor

@khaines khaines left a comment

Choose a reason for hiding this comment

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

LGTM. I appreciate the starting & stopping state checks.

Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

Thanks Peter for this! Changes LGTM, but please take a look at the comments cause there may be few things we can improve.

Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

I double checked the changes and still LGTM 🎉

Ingester cannot operate until these are finished, so
all public methods now check if Ingester is in Running or Stopping
state. Why Stopping? Because querying methods still work
while it is stopping.

Signed-off-by: Peter Štibraný <[email protected]>
Signed-off-by: Peter Štibraný <[email protected]>
Signed-off-by: Peter Štibraný <[email protected]>
Signed-off-by: Peter Štibraný <[email protected]>
@pstibrany pstibrany merged commit ac646af into cortexproject:master Mar 9, 2020
@pstibrany pstibrany deleted the run-wal-in-background branch March 9, 2020 08:27
@codesome codesome mentioned this pull request Mar 10, 2020
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants