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
vttablet throttler feature flag: -enable-lag-throttler #6815
Changes from all commits
1d61aaf
2deb3b6
f6f2e27
8505312
f66c6b1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -47,8 +47,6 @@ vttablet \ | |
-health_check_interval 5s \ | ||
-enable_semi_sync \ | ||
-enable_replication_reporter \ | ||
-heartbeat_enable \ | ||
-heartbeat_interval 250ms \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These were committed earlier by mistake. |
||
-backup_storage_implementation file \ | ||
-file_backup_storage_root $VTDATAROOT/backups \ | ||
-restore_from_backup \ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -50,7 +50,8 @@ var ( | |
|
||
// ReplTracker tracks replication lag. | ||
type ReplTracker struct { | ||
mode string | ||
mode string | ||
forceHeartbeat bool | ||
|
||
mu sync.Mutex | ||
isMaster bool | ||
|
@@ -63,10 +64,11 @@ type ReplTracker struct { | |
// NewReplTracker creates a new ReplTracker. | ||
func NewReplTracker(env tabletenv.Env, alias topodatapb.TabletAlias) *ReplTracker { | ||
return &ReplTracker{ | ||
mode: env.Config().ReplicationTracker.Mode, | ||
hw: newHeartbeatWriter(env, alias), | ||
hr: newHeartbeatReader(env), | ||
poller: &poller{}, | ||
mode: env.Config().ReplicationTracker.Mode, | ||
forceHeartbeat: env.Config().EnableLagThrottler, | ||
hw: newHeartbeatWriter(env, alias), | ||
hr: newHeartbeatReader(env), | ||
poller: &poller{}, | ||
} | ||
} | ||
|
||
|
@@ -88,6 +90,9 @@ func (rt *ReplTracker) MakeMaster() { | |
rt.hr.Close() | ||
rt.hw.Open() | ||
} | ||
if rt.forceHeartbeat { | ||
rt.hw.Open() | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. when throttler runs, we need heartbeat writer to run. We do not need the reader because throttler reads directly from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a note that we could be calling Open and Close twice, but that is fine since they are idempotent. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 that was my understanding as well. |
||
} | ||
|
||
// MakeNonMaster must be called if the tablet type becomes non-MASTER. | ||
|
@@ -105,6 +110,9 @@ func (rt *ReplTracker) MakeNonMaster() { | |
// Run the status once to pre-initialize values. | ||
rt.poller.Status() | ||
} | ||
if rt.forceHeartbeat { | ||
rt.hw.Close() | ||
} | ||
} | ||
|
||
// Close closes ReplTracker. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,6 +73,11 @@ type heartbeatWriter struct { | |
// newHeartbeatWriter creates a new heartbeatWriter. | ||
func newHeartbeatWriter(env tabletenv.Env, alias topodatapb.TabletAlias) *heartbeatWriter { | ||
config := env.Config() | ||
|
||
// config.EnableLagThrottler is a feature flag for the throttler; if throttler runs, then heartbeat must also run | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is not clear why the changes in this file are required, but I'll trust the tests :) |
||
if config.ReplicationTracker.Mode != tabletenv.Heartbeat && !config.EnableLagThrottler { | ||
return &heartbeatWriter{} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @deepthi this change is actually reverting my previous change in the throttler PR. This There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (except of course now added |
||
heartbeatInterval := config.ReplicationTracker.HeartbeatIntervalSeconds.Get() | ||
return &heartbeatWriter{ | ||
env: env, | ||
|
@@ -182,6 +187,9 @@ func (w *heartbeatWriter) recordError(err error) { | |
|
||
// enableWrites actives or deactives heartbeat writes | ||
func (w *heartbeatWriter) enableWrites(enable bool) { | ||
if w.ticks == nil { | ||
return | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @deepthi this change is actually reverting my previous change in the throttler PR. This |
||
if enable { | ||
w.ticks.Start(w.writeHeartbeat) | ||
} else { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -186,7 +186,9 @@ func (throttler *Throttler) initThrottleTabletTypes() { | |
func (throttler *Throttler) InitDBConfig(keyspace, shard string) { | ||
throttler.keyspace = keyspace | ||
throttler.shard = shard | ||
go throttler.Operate(context.Background()) | ||
if throttler.env.Config().EnableLagThrottler { | ||
go throttler.Operate(context.Background()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} | ||
} | ||
|
||
// initThrottler initializes config | ||
|
@@ -692,6 +694,9 @@ func (throttler *Throttler) AppRequestMetricResult(ctx context.Context, appName | |
|
||
// Check is the main serving function of the throttler, and returns a check result for this cluster's lag | ||
func (throttler *Throttler) Check(ctx context.Context, appName string, remoteAddr string, flags *CheckFlags) (checkResult *CheckResult) { | ||
if !throttler.env.Config().EnableLagThrottler { | ||
return okMetricCheckResult | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When the throttler is disabled, Check() always responds with |
||
} | ||
return throttler.check.Check(ctx, appName, "mysql", localStoreName, remoteAddr, flags) | ||
} | ||
|
||
|
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.
These were committed earlier by mistake.