From 88a356cbed732a6cd205d40ed2833d7fe85b714c Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Fri, 31 May 2024 22:41:24 +0300 Subject: [PATCH] Tablet throttler: remove `LowPriority` logic (#16013) Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/vttablet/tabletmanager/rpc_throttler.go | 1 - go/vt/vttablet/tabletserver/tabletserver.go | 1 - go/vt/vttablet/tabletserver/throttle/check.go | 14 ------------- .../vttablet/tabletserver/throttle/client.go | 20 ++----------------- .../tabletserver/throttle/throttler.go | 7 +------ .../tabletserver/throttle/throttler_test.go | 1 - 6 files changed, 3 insertions(+), 41 deletions(-) diff --git a/go/vt/vttablet/tabletmanager/rpc_throttler.go b/go/vt/vttablet/tabletmanager/rpc_throttler.go index c961761c5f2..ab153b3ef43 100644 --- a/go/vt/vttablet/tabletmanager/rpc_throttler.go +++ b/go/vt/vttablet/tabletmanager/rpc_throttler.go @@ -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) diff --git a/go/vt/vttablet/tabletserver/tabletserver.go b/go/vt/vttablet/tabletserver/tabletserver.go index d74bcb09952..db01e6f2912 100644 --- a/go/vt/vttablet/tabletserver/tabletserver.go +++ b/go/vt/vttablet/tabletserver/tabletserver.go @@ -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) diff --git a/go/vt/vttablet/tabletserver/throttle/check.go b/go/vt/vttablet/tabletserver/throttle/check.go index 98d887e8342..c15d8963d05 100644 --- a/go/vt/vttablet/tabletserver/throttle/check.go +++ b/go/vt/vttablet/tabletserver/throttle/check.go @@ -67,7 +67,6 @@ var ( type CheckFlags struct { ReadCheck bool OverrideThreshold float64 - LowPriority bool OKIfNotExists bool SkipRequestHeartbeats bool } @@ -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 { @@ -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 diff --git a/go/vt/vttablet/tabletserver/throttle/client.go b/go/vt/vttablet/tabletserver/throttle/client.go index 546d75c040d..5194ba19f98 100644 --- a/go/vt/vttablet/tabletserver/throttle/client.go +++ b/go/vt/vttablet/tabletserver/throttle/client.go @@ -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{}, } } diff --git a/go/vt/vttablet/tabletserver/throttle/throttler.go b/go/vt/vttablet/tabletserver/throttle/throttler.go index 0aed6bf6075..803f331cef4 100644 --- a/go/vt/vttablet/tabletserver/throttle/throttler.go +++ b/go/vt/vttablet/tabletserver/throttle/throttler.go @@ -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 @@ -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 @@ -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() @@ -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 diff --git a/go/vt/vttablet/tabletserver/throttle/throttler_test.go b/go/vt/vttablet/tabletserver/throttle/throttler_test.go index dadd6f59991..a745ca66fe7 100644 --- a/go/vt/vttablet/tabletserver/throttle/throttler_test.go +++ b/go/vt/vttablet/tabletserver/throttle/throttler_test.go @@ -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)