-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Add check in healthcheck to detect timeout. #1399
Conversation
@@ -38,7 +38,8 @@ import ( | |||
var ( | |||
healthcheckTopologyRefresh = flag.Duration("binlog_player_healthcheck_topology_refresh", 30*time.Second, "refresh interval for re-reading the topology when filtered replication is running") | |||
|
|||
retryDelay = flag.Duration("binlog_player_retry_delay", 5*time.Second, "delay before retrying a failed healthcheck or a failed binlog connection") | |||
retryDelay = flag.Duration("binlog_player_retry_delay", 5*time.Second, "delay before retrying a failed healthcheck or a failed binlog connection") | |||
healthCheckTimeout = flag.Duration("healthcheck_timeout", time.Minute, "the health check timeout period") |
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.
We should prefix this one with binlog_player_ as we may get more later.
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.
Updated.
@alainjobart Can you take a look? |
You can just submit this without calling Close() in binlog.go, and I'll do it. The life cycles of the healthchecks guys need to be fixed, I'll do that. In the meantime, having the resource leak is OK, I'll take a look on Monday. So LGTM |
Add check in healthcheck to detect timeout.
) * feat: fix checkMySQL and add tests and documentation (#11895) (#1391) Signed-off-by: Manan Gupta <[email protected]> Signed-off-by: Manan Gupta <[email protected]> Signed-off-by: Manan Gupta <[email protected]> * more unit tests for QueryMatchesTemplates() (#11894) (#1394) Signed-off-by: Shlomi Noach <[email protected]> Signed-off-by: Shlomi Noach <[email protected]> Signed-off-by: Shlomi Noach <[email protected]> * Simplify `getPlan` and `gen4CompareV3` (#11903) (#1400) * simplify getPlan and gen4CompareV3 Signed-off-by: Florent Poinsard <[email protected]> * fix error return in getPlan Signed-off-by: Florent Poinsard <[email protected]> Signed-off-by: Florent Poinsard <[email protected]> Signed-off-by: Florent Poinsard <[email protected]> Signed-off-by: Rohit Nayak <[email protected]> * Fix merge issue found during make Signed-off-by: Rohit Nayak <[email protected]> * Disable MIN() / MAX() support (#1399) It's trivially simple today to trigger a panic for these, because the invalidation steps are not there and they panic. So if you remove a value that's the maximum or minimum, we panic and crash. Better to block creating this for now and fix the underlying problem first and then allow it again. Signed-off-by: Dirkjan Bussink <[email protected]> Signed-off-by: Dirkjan Bussink <[email protected]> * Always use public id internally for mappings (#1398) * Always use public id internally for mappings We were using the `.Name` attribute of a query in a few places internally as a hash key, but that is broken. There's no guarantee that we always have a name, let alone that it's guaranteed to be unique. This switches to using the public id and adds additional validation checks that it's always set and that it's always unique for hash map lookups. The case where this was blowing up is that end-to-end tests would fail if there's no name for a query and no name through a special comment either. We also likely would have seen very strange bugs if someone named their query "source" since that's a special node name and we used the name as node names as well. Signed-off-by: Dirkjan Bussink <[email protected]> * integration: use default view ids Signed-off-by: Vicent Marti <[email protected]> * boost: do not use Public IDs as names Signed-off-by: Vicent Marti <[email protected]> * boost: do not use node IDs for table names either Signed-off-by: Vicent Marti <[email protected]> * boost: fix test issues Signed-off-by: Vicent Marti <[email protected]> * boost: fix protogen Signed-off-by: Vicent Marti <[email protected]> * incorporator: do not panic Signed-off-by: Vicent Marti <[email protected]> Signed-off-by: Dirkjan Bussink <[email protected]> Signed-off-by: Vicent Marti <[email protected]> Co-authored-by: Vicent Marti <[email protected]> * feat: add test and fix the error of not sending a ServerLost error on server side error (#11920) (#1405) Signed-off-by: Manan Gupta <[email protected]> Signed-off-by: Manan Gupta <[email protected]> Signed-off-by: Manan Gupta <[email protected]> * Onlineddl: formalize "immediate operations", respect `--postpone-completion` strategy flag (#11910) (#1411) * Added endtoend test to validate postponed REVERT of an ALTER VIEW statement: test indicates a bug Signed-off-by: Shlomi Noach <[email protected]> * ALTER VIEW now respects --postpone-completion flag Signed-off-by: Shlomi Noach <[email protected]> * Online DDL: formalize 'immediate operations', respect --postpone-completion Signed-off-by: Shlomi Noach <[email protected]> * typo Signed-off-by: Shlomi Noach <[email protected]> * only test instant ddl in capable versions (ie 8.0) Signed-off-by: Shlomi Noach <[email protected]> * more specific instant ddl capability Signed-off-by: Shlomi Noach <[email protected]> * Update go/vt/vttablet/onlineddl/executor.go Co-authored-by: Deepthi Sigireddi <[email protected]> Signed-off-by: Shlomi Noach <[email protected]> * Update go/vt/vttablet/onlineddl/executor.go Co-authored-by: Deepthi Sigireddi <[email protected]> Signed-off-by: Shlomi Noach <[email protected]> * Update go/vt/vttablet/onlineddl/executor.go Co-authored-by: Deepthi Sigireddi <[email protected]> Signed-off-by: Shlomi Noach <[email protected]> Signed-off-by: Shlomi Noach <[email protected]> Co-authored-by: Deepthi Sigireddi <[email protected]> Signed-off-by: Shlomi Noach <[email protected]> Co-authored-by: Deepthi Sigireddi <[email protected]> * log: also log error in DiscoverInstance when force discovery is specified (#11936) (#1410) Signed-off-by: Manan Gupta <[email protected]> Signed-off-by: Manan Gupta <[email protected]> Signed-off-by: Manan Gupta <[email protected]> Signed-off-by: Rohit Nayak <[email protected]> Signed-off-by: Manan Gupta <[email protected]> Signed-off-by: Shlomi Noach <[email protected]> Signed-off-by: Florent Poinsard <[email protected]> Signed-off-by: Rohit Nayak <[email protected]> Signed-off-by: Dirkjan Bussink <[email protected]> Signed-off-by: Vicent Marti <[email protected]> Co-authored-by: Manan Gupta <[email protected]> Co-authored-by: Shlomi Noach <[email protected]> Co-authored-by: FlorentP <[email protected]> Co-authored-by: Dirkjan Bussink <[email protected]> Co-authored-by: Vicent Marti <[email protected]> Co-authored-by: Deepthi Sigireddi <[email protected]>
@alainjobart @sougou