-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Start inBackupHealthReport
during migrations
#13120
Start inBackupHealthReport
during migrations
#13120
Conversation
core/store/migrate/migrate.go
Outdated
ibhr := services.NewInBackupHealthReport(healthReportPort, lggr) | ||
ibhr.Start() | ||
defer ibhr.Stop() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we do this from the caller instead? Otherwise ensureMigrated
is not covered.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, maybe skip several callers. I think it is important to include as much of the startup time as possible, plus this will avoid the leaking in to the administrative commands that don't need to be reporting health in the first place, like ResetDatabase
.
I see you added a changeset file but it does not contain a tag. Please edit the text include at least one of the following tags:
|
core/cmd/shell.go
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is just handleNodeVersioning
going to be enough? How much time passes between the end of this call and the start of the normal health report?
Quality Gate passedIssues Measures |
This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days. |
Quality Gate passedIssues Measures |
InBackupHealthReport
toStartUpHealthReport
StartUpHealthReport
in thehandleNodeVersioning
method so it would cover both db migrations and backupsDefaultServeMux
to avoid issues when testing migrations and rollbacks