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

health: add warming-up warnable #12553

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

agottardo
Copy link
Contributor

Updates #4136

Creates and exposes a 'fake' warnable that persists for the first 5 seconds of lifetime of the backend. This is used to silence spurious warnings while the backend is starting up.

@agottardo agottardo requested a review from bradfitz June 20, 2024 00:45
@agottardo agottardo self-assigned this Jun 20, 2024
@agottardo
Copy link
Contributor Author

agottardo commented Jun 20, 2024

@bradfitz could I perhaps improve this by setting the Warnable to healthy as soon as IPNState is set to Running, if that verifies before 5 seconds have passed? Does Running mean that everything is ready to go?

Updates #4136

Creates and exposes a 'fake' warnable that persists for the first 5 seconds of lifetime of the backend. This is used to silence spurious warnings while the backend is starting up.

Signed-off-by: Andrea Gottardo <andrea@gottardo.me>
Comment on lines +709 to +714
if t.ipnStateFirstSet.IsZero() {
// The first time we see an IPNState getting set, it means the backend is starting up. We store this timestamp.
// We use it to silence some warnings that are expected during startup.
t.ipnStateFirstSet = time.Now()
t.setUnhealthyLocked(warmingUpWarnable, nil)
}
Copy link
Member

Choose a reason for hiding this comment

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

I think we want to do this on any transition from wantRunning false to wantRunning true, not just once.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aargh, you're right. I'm used to our Apple platforms where there's always only one wantRunning for each process lifetime.

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.

None yet

2 participants