Skip to content

Commit

Permalink
Tablet throttler: remove LowPriority logic (#16013)
Browse files Browse the repository at this point in the history
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
  • Loading branch information
shlomi-noach committed May 31, 2024
1 parent 8c2ef94 commit 88a356c
Show file tree
Hide file tree
Showing 6 changed files with 3 additions and 41 deletions.
1 change: 0 additions & 1 deletion go/vt/vttablet/tabletmanager/rpc_throttler.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ func (tm *TabletManager) CheckThrottler(ctx context.Context, req *tabletmanagerd
req.AppName = throttlerapp.VitessName.String()
}
flags := &throttle.CheckFlags{
LowPriority: false,
SkipRequestHeartbeats: true,
}
checkResult := tm.QueryServiceControl.CheckThrottler(ctx, req.AppName, flags)
Expand Down
1 change: 0 additions & 1 deletion go/vt/vttablet/tabletserver/tabletserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -1896,7 +1896,6 @@ func (tsv *TabletServer) registerThrottlerCheckHandlers() {
appName = throttlerapp.DefaultName.String()
}
flags := &throttle.CheckFlags{
LowPriority: (r.URL.Query().Get("p") == "low"),
SkipRequestHeartbeats: (r.URL.Query().Get("s") == "true"),
}
checkResult := tsv.lagThrottler.CheckByType(ctx, appName, remoteAddr, flags, checkType)
Expand Down
14 changes: 0 additions & 14 deletions go/vt/vttablet/tabletserver/throttle/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ var (
type CheckFlags struct {
ReadCheck bool
OverrideThreshold float64
LowPriority bool
OKIfNotExists bool
SkipRequestHeartbeats bool
}
Expand All @@ -91,14 +90,6 @@ func NewThrottlerCheck(throttler *Throttler) *ThrottlerCheck {
func (check *ThrottlerCheck) checkAppMetricResult(ctx context.Context, appName string, storeType string, storeName string, metricResultFunc base.MetricResultFunc, flags *CheckFlags) (checkResult *CheckResult) {
// Handle deprioritized app logic
denyApp := false
metricName := fmt.Sprintf("%s/%s", storeType, storeName)
if flags.LowPriority {
if _, exists := check.throttler.nonLowPriorityAppRequestsThrottled.Get(metricName); exists {
// a non-deprioritized app, ie a "normal" app, has recently been throttled.
// This is now a deprioritized app. Deny access to this request.
denyApp = true
}
}
//
metricResult, threshold := check.throttler.AppRequestMetricResult(ctx, appName, metricResultFunc, denyApp)
if flags.OverrideThreshold > 0 {
Expand All @@ -125,11 +116,6 @@ func (check *ThrottlerCheck) checkAppMetricResult(ctx context.Context, appName s
// casual throttling
statusCode = http.StatusTooManyRequests // 429
err = base.ErrThresholdExceeded

if !flags.LowPriority && !flags.ReadCheck && throttlerapp.VitessName.Equals(appName) {
// low priority requests will henceforth be denied
go check.throttler.nonLowPriorityAppRequestsThrottled.SetDefault(metricName, true)
}
default:
// all good!
statusCode = http.StatusOK // 200
Expand Down
20 changes: 2 additions & 18 deletions go/vt/vttablet/tabletserver/throttle/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,30 +56,14 @@ type Client struct {
lastSuccessfulThrottle int64
}

// NewProductionClient creates a client suitable for foreground/production jobs, which have normal priority.
func NewProductionClient(throttler *Throttler, appName throttlerapp.Name, checkType ThrottleCheckType) *Client {
initThrottleTicker()
return &Client{
throttler: throttler,
appName: appName,
checkType: checkType,
flags: CheckFlags{
LowPriority: false,
},
}
}

// NewBackgroundClient creates a client suitable for background jobs, which have low priority over production traffic,
// e.g. migration, table pruning, vreplication
// NewBackgroundClient creates a client
func NewBackgroundClient(throttler *Throttler, appName throttlerapp.Name, checkType ThrottleCheckType) *Client {
initThrottleTicker()
return &Client{
throttler: throttler,
appName: appName,
checkType: checkType,
flags: CheckFlags{
LowPriority: true,
},
flags: CheckFlags{},
}
}

Expand Down
7 changes: 1 addition & 6 deletions go/vt/vttablet/tabletserver/throttle/throttler.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,6 @@ const (
aggregatedMetricsExpiration = 5 * time.Second
recentAppsExpiration = time.Hour * 24

nonDeprioritizedAppMapExpiration = time.Second

dormantPeriod = time.Minute // How long since last check to be considered dormant
DefaultAppThrottleDuration = time.Hour
DefaultThrottleRatio = 1.0
Expand Down Expand Up @@ -208,8 +206,7 @@ type Throttler struct {

readSelfThrottleMetric func(context.Context, *mysql.Probe) *mysql.MySQLThrottleMetric // overwritten by unit test

nonLowPriorityAppRequestsThrottled *cache.Cache
httpClient *http.Client
httpClient *http.Client
}

// ThrottlerStatus published some status values from the throttler
Expand Down Expand Up @@ -256,7 +253,6 @@ func NewThrottler(env tabletenv.Env, srvTopoServer srvtopo.Server, ts *topo.Serv
throttler.aggregatedMetrics = cache.New(aggregatedMetricsExpiration, 0)
throttler.recentApps = cache.New(recentAppsExpiration, 0)
throttler.metricsHealth = cache.New(cache.NoExpiration, 0)
throttler.nonLowPriorityAppRequestsThrottled = cache.New(nonDeprioritizedAppMapExpiration, 0)

throttler.httpClient = base.SetupHTTPClient(2 * mysqlCollectInterval)
throttler.initThrottleTabletTypes()
Expand Down Expand Up @@ -728,7 +724,6 @@ func (throttler *Throttler) Operate(ctx context.Context, wg *sync.WaitGroup) {
primaryStimulatorRateLimiter.Stop()
throttler.aggregatedMetrics.Flush()
throttler.recentApps.Flush()
throttler.nonLowPriorityAppRequestsThrottled.Flush()
wg.Done()
}()
// we do not flush throttler.throttledApps because this is data submitted by the user; the user expects the data to survive a disable+enable
Expand Down
1 change: 0 additions & 1 deletion go/vt/vttablet/tabletserver/throttle/throttler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,6 @@ func newTestThrottler() *Throttler {
throttler.aggregatedMetrics = cache.New(10*aggregatedMetricsExpiration, 0)
throttler.recentApps = cache.New(recentAppsExpiration, 0)
throttler.metricsHealth = cache.New(cache.NoExpiration, 0)
throttler.nonLowPriorityAppRequestsThrottled = cache.New(nonDeprioritizedAppMapExpiration, 0)
throttler.metricsQuery.Store(metricsQuery)
throttler.initThrottleTabletTypes()
throttler.check = NewThrottlerCheck(throttler)
Expand Down

0 comments on commit 88a356c

Please sign in to comment.