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

Suggestion: always enable heartbeat on vttablet #6662

Closed
shlomi-noach opened this issue Sep 2, 2020 · 4 comments
Closed

Suggestion: always enable heartbeat on vttablet #6662

shlomi-noach opened this issue Sep 2, 2020 · 4 comments

Comments

@shlomi-noach
Copy link
Contributor

shlomi-noach commented Sep 2, 2020

vttablet has these flags:

  • -heartbeat_enable
  • -heartbeat_interval

The use of heartbeat is so important, that I suggest it must always be enabled. Plus, it should have a maximum interval that is subsecond.

Heartbeats are to be used by throttling services, by orchestrator, etc.

Is there any reason why we woudln't always enable heartbeat?

@sougou
Copy link
Contributor

sougou commented Sep 2, 2020

It was badly implemented and had bugs. But we've fixed them all over time. We should encourage using this feature now.

@deepthi
Copy link
Member

deepthi commented Sep 6, 2020

We document that only one of -heartbeat_enable and -enable_replication_reporter should be used. Right now they both default to false. From that POV it is safe to default heartbeat to true. However, it should be documented in the release notes as a possibly breaking change.

@shlomi-noach
Copy link
Contributor Author

We document that only one of -heartbeat_enable and -enable_replication_reporter should be used

I couldn't find where this is documented, searched in vitessio/vitess and in vitessio/website repos. The one related thing I found in vitessio/website is:

-heartbeat_enable and -heartbeat interval duration: cause vttablet to write heartbeats to the sidecar database. This information is also used by the replication reporter to assess replica lag.

@shlomi-noach
Copy link
Contributor Author

#6665 was created to close this issue. However, #6668, the throttler PR, implicitly enables heartbeat without changing configuration. So #6665 was closed without merging.

We're still in experimental mode for the tablet throttler. It's debatable whether we can call this issue "implicitly implemented" or "won't fix"... Opting for "won't fix" for now and closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants