Skip to content

[FIXED] NRG: Don't mark current/healthy while catching up#6247

Merged
derekcollison merged 1 commit intomainfrom
maurice/healthz-during-catchup
Dec 12, 2024
Merged

[FIXED] NRG: Don't mark current/healthy while catching up#6247
derekcollison merged 1 commit intomainfrom
maurice/healthz-during-catchup

Conversation

@MauriceVanVeen
Copy link
Copy Markdown
Member

@MauriceVanVeen MauriceVanVeen commented Dec 12, 2024

If we're actively catching up, but n.commit == n.applied, we would mark it as current/healthy. This would result in healthz reporting healthy too soon based on timing.

Also removed the cancellation of catchup in isCurrent. We shouldn't be able to influence catchup based on if and how often we call healthz. Also because we might need to do 2 catchups back-to-back if our first catchup takes longer and after that catchup is finished we get a heartbeat that re-triggers catchup. If we would cancel catchup due to the health check we would erroneously be marked healthy, whereas we require another catchup still.

Signed-off-by: Maurice van Veen github@mauricevanveen.com

@MauriceVanVeen MauriceVanVeen requested a review from a team as a code owner December 12, 2024 09:37
Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
@MauriceVanVeen MauriceVanVeen force-pushed the maurice/healthz-during-catchup branch from 2e84f51 to deb6765 Compare December 12, 2024 10:31
Copy link
Copy Markdown
Member

@neilalexander neilalexander left a comment

Choose a reason for hiding this comment

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

LGTM

@derekcollison derekcollison merged commit daf3e34 into main Dec 12, 2024
@derekcollison derekcollison deleted the maurice/healthz-during-catchup branch December 12, 2024 14:16
@wallyqs wallyqs changed the title NRG: Don't mark current/healthy while catching up [FIXED] NRG: Don't mark current/healthy while catching up Dec 13, 2024
neilalexander added a commit that referenced this pull request Dec 13, 2024
Includes the following:

- #6226
- #6232
- #6235
- #6064
- #6244
- #6246
- #6247
- #6248
- #6250

Signed-off-by: Neil Twigg <neil@nats.io>
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.

3 participants