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

ring.Lifecycler: Ingester stuck in LEAVING state when ring tokens are increased. #289

Conversation

sarthaktyagi-505
Copy link

@sarthaktyagi-505 sarthaktyagi-505 commented Apr 29, 2023

What this PR does:
When starting a ring.Lifecycler, handle when previous ingester state is found in ring, its state is leaving and the number of tokens has changed. Tests are included.

What Changed?

  1. Old code was only check if number of tokens before and number of tokens after ingester is in LEAVING state is equal to allow it to transition to ACTIVE state. This PR works on the similar logic used by @DylanGuedes to add or remove tokens if there is a increase or decrease tokens.
  2. Also in the tests we weren't testing against number of tokens in the token file in the Dylan's PR, I have added a logic to add tokens in the temp file which is created to test the logic and test it against the tokens from the file.
  3. Updated the runutil.go file to update the deprecated io.ioutil package.

The PR is based on the work done in this PR #79.

Which issue(s) this PR fixes:

Fixes #73

Checklist

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

@CLAassistant
Copy link

CLAassistant commented Apr 29, 2023

CLA assistant check
All committers have signed the CLA.

@sarthaktyagi-505 sarthaktyagi-505 changed the title ring.Lifecycler: Handle when previous ring state is leaving and the n… ring.Lifecycler: Ingester stuck in LEAVING state when ring tokens are increased. Apr 29, 2023
@aknuds1
Copy link
Collaborator

aknuds1 commented May 26, 2023

Hey, sorry it took me so long to get to this. I've started reviewing your PR.

@sarthaktyagi-505
Copy link
Author

@aknuds1 no worries, You can let me know how it goes.

-Sar

@aknuds1
Copy link
Collaborator

aknuds1 commented Jun 3, 2023

Could you have a look at my corresponding PR again? I've revised it heavily, and I think it does the right thing now.

charleskorn pushed a commit that referenced this pull request Aug 3, 2023
Allow users to manually register metrics
@56quarters
Copy link
Contributor

Closing since this was finished up in #79 . Thank you!

@56quarters 56quarters closed this Jan 16, 2024
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.

Ring lifecycler stuck when increasing tokens and instance is already in the ring
4 participants