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

Auto-set 5s on-demand heartbeat if --enable_heartbeat is disabled #15099

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go/flags/endtoend/vtcombo.txt
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ Flags:
--healthcheck_timeout duration the health check timeout period (default 1m0s)
--heartbeat_enable If true, vttablet records (if master) or checks (if replica) the current time of a replication heartbeat in the sidecar database's heartbeat table. The result is used to inform the serving state of the vttablet via healthchecks.
--heartbeat_interval duration How frequently to read and write replication heartbeat. (default 1s)
--heartbeat_on_demand_duration duration If non-zero, heartbeats are only written upon consumer request, and only run for up to given duration following the request. Frequent requests can keep the heartbeat running consistently; when requests are infrequent heartbeat may completely stop between requests
--heartbeat_on_demand_duration duration If non-zero, heartbeats are only written upon consumer request, and only run for up to given duration following the request. Frequent requests can keep the heartbeat running consistently. Automatically set to 5s when --heartbeat_enable is unset.
-h, --help help for vtcombo
--hot_row_protection_concurrent_transactions int Number of concurrent transactions let through to the txpool/MySQL for the same hot row. Should be > 1 to have enough 'ready' transactions in MySQL and benefit from a pipelining effect. (default 5)
--hot_row_protection_max_global_queue_size int Global queue limit across all row (ranges). Useful to prevent that the queue can grow unbounded. (default 1000)
Expand Down
2 changes: 1 addition & 1 deletion go/flags/endtoend/vttablet.txt
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ Flags:
--health_check_interval duration Interval between health checks (default 20s)
--heartbeat_enable If true, vttablet records (if master) or checks (if replica) the current time of a replication heartbeat in the sidecar database's heartbeat table. The result is used to inform the serving state of the vttablet via healthchecks.
--heartbeat_interval duration How frequently to read and write replication heartbeat. (default 1s)
--heartbeat_on_demand_duration duration If non-zero, heartbeats are only written upon consumer request, and only run for up to given duration following the request. Frequent requests can keep the heartbeat running consistently; when requests are infrequent heartbeat may completely stop between requests
--heartbeat_on_demand_duration duration If non-zero, heartbeats are only written upon consumer request, and only run for up to given duration following the request. Frequent requests can keep the heartbeat running consistently. Automatically set to 5s when --heartbeat_enable is unset.
-h, --help help for vttablet
--hot_row_protection_concurrent_transactions int Number of concurrent transactions let through to the txpool/MySQL for the same hot row. Should be > 1 to have enough 'ready' transactions in MySQL and benefit from a pipelining effect. (default 5)
--hot_row_protection_max_global_queue_size int Global queue limit across all row (ranges). Useful to prevent that the queue can grow unbounded. (default 1000)
Expand Down
13 changes: 12 additions & 1 deletion go/vt/vttablet/tabletserver/tabletenv/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func registerTabletEnvFlags(fs *pflag.FlagSet) {

fs.BoolVar(&enableHeartbeat, "heartbeat_enable", false, "If true, vttablet records (if master) or checks (if replica) the current time of a replication heartbeat in the sidecar database's heartbeat table. The result is used to inform the serving state of the vttablet via healthchecks.")
fs.DurationVar(&heartbeatInterval, "heartbeat_interval", 1*time.Second, "How frequently to read and write replication heartbeat.")
fs.DurationVar(&heartbeatOnDemandDuration, "heartbeat_on_demand_duration", 0, "If non-zero, heartbeats are only written upon consumer request, and only run for up to given duration following the request. Frequent requests can keep the heartbeat running consistently; when requests are infrequent heartbeat may completely stop between requests")
fs.DurationVar(&heartbeatOnDemandDuration, "heartbeat_on_demand_duration", 0, "If non-zero, heartbeats are only written upon consumer request, and only run for up to given duration following the request. Frequent requests can keep the heartbeat running consistently. Automatically set to 5s when --heartbeat_enable is unset.")
Comment on lines 185 to +186
Copy link
Contributor

Choose a reason for hiding this comment

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

should we make the two flags mutually exclusive in pFlag? That might simplify the logic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a great idea, irrespective of this PR! I only wonder if it's going to break someone's existing setup?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, it might... 😢 We could potentially add it to the 19.0.0 changelog as a breaking change though.

Maybe we should take a documentation driven approach here. How would we document the new/proposed configuration here? https://vitess.io/docs/19.0/reference/features/tablet-throttler/#configuration

  1. IMO we should remove all of the old tablet level config parts/noise there in the v19 docs
  2. That seems a little misleading now as it says "Enabling the lag throttler also automatically enables heartbeat injection." and it does not note that you must set --heartbeat_enable=false (which I think was true before given the noted issues reported with the examples) and --heartbeat_on_demand_duration to a non-zero value — although it does describe what it is and recommend values for it


fs.BoolVar(&currentConfig.EnforceStrictTransTables, "enforce_strict_trans_tables", defaultConfig.EnforceStrictTransTables, "If true, vttablet requires MySQL to run with STRICT_TRANS_TABLES or STRICT_ALL_TABLES on. It is recommended to not turn this flag off. Otherwise MySQL may alter your supplied values before saving them to the database.")
flagutil.DualFormatBoolVar(fs, &enableConsolidator, "enable_consolidator", true, "This option enables the query consolidator.")
Expand Down Expand Up @@ -251,6 +251,17 @@ func Init() {
if heartbeatOnDemandDuration < 0 {
heartbeatOnDemandDuration = 0
}
if heartbeatOnDemandDuration == 0 && !enableHeartbeat {
// Neither heartbeat-related flag was set. We want to always have some heartbeat capability,
// because the tablet throttler depends on the availability of a heartbeat.
// To that effect, we choose to set a minimal and relaxed heartbeat setup: a 5s on-demand lease.
// Ad on-demand heartbeats go, this has no impact when the throttler is disabled, and no impact
// when the throttler is enabled but otherwise not engaged with some consumer. Heartbeats are
// only leased and generated when the throttler is requested by some consumer (e.g. vreplication)
// to provide throttling advice. We keep the lease to a very low 5s, so that heartbeats are only
// generated while an active consumer is requesting them, and only up to 5s afterwards.
heartbeatOnDemandDuration = 5 * time.Second
}
currentConfig.ReplicationTracker.HeartbeatInterval = heartbeatInterval
currentConfig.ReplicationTracker.HeartbeatOnDemand = heartbeatOnDemandDuration

Expand Down
16 changes: 16 additions & 0 deletions go/vt/vttablet/tabletserver/tabletenv/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ func TestFlags(t *testing.T) {
want.Healthcheck.UnhealthyThreshold = 2 * time.Hour
want.ReplicationTracker.HeartbeatInterval = time.Second
want.ReplicationTracker.Mode = Disable
want.ReplicationTracker.HeartbeatOnDemand = 5 * time.Second
assert.Equal(t, want.DB, currentConfig.DB)
assert.Equal(t, want, currentConfig)

Expand Down Expand Up @@ -266,21 +267,36 @@ func TestFlags(t *testing.T) {
assert.Equal(t, want, currentConfig)

enableHeartbeat = true
heartbeatOnDemandDuration = 0
heartbeatInterval = 1 * time.Second
currentConfig.ReplicationTracker.Mode = ""
currentConfig.ReplicationTracker.HeartbeatInterval = 0
Init()
want.ReplicationTracker.Mode = Heartbeat
want.ReplicationTracker.HeartbeatInterval = time.Second
want.ReplicationTracker.HeartbeatOnDemand = 0
assert.Equal(t, want, currentConfig)

enableHeartbeat = false
heartbeatOnDemandDuration = 0
heartbeatInterval = 1 * time.Second
currentConfig.ReplicationTracker.Mode = ""
currentConfig.ReplicationTracker.HeartbeatInterval = 0
Init()
want.ReplicationTracker.Mode = Disable
want.ReplicationTracker.HeartbeatInterval = time.Second
want.ReplicationTracker.HeartbeatOnDemand = 5 * time.Second
assert.Equal(t, want, currentConfig)

enableHeartbeat = false
heartbeatOnDemandDuration = 7 * time.Second
heartbeatInterval = 1 * time.Second
currentConfig.ReplicationTracker.Mode = ""
currentConfig.ReplicationTracker.HeartbeatInterval = 0
Init()
want.ReplicationTracker.Mode = Disable
want.ReplicationTracker.HeartbeatInterval = time.Second
want.ReplicationTracker.HeartbeatOnDemand = 7 * time.Second
assert.Equal(t, want, currentConfig)

enableReplicationReporter = true
Expand Down
Loading