[QueryThrottler] Implement Query Throttler #18449

siddharth16396 · 2025-07-15T07:19:59Z

Description

This PR introduces a comprehensive incoming query throttling system for Vitess tablets outlined in RFC #18412 with the following key components:

Core throttling framework - IncomingQueryThrottler with strategy pattern for pluggable throttling algorithms
Configuration system - File-based config loader reading from /config/throttler-config.json with enabled/disabled flag and strategy selection
Strategy implementations - NoOp strategy (default) and Cinnamon strategy placeholder with lifecycle management.
Background config refresh - Automatic configuration reloading every minute with strategy switching.
Comprehensive testing - Full test coverage for all components including error scenarios and lifecycle management.

The system supports multiple throttling strategies (TabletThrottler, Cinnamon) and can be dynamically reconfigured without restart. The implementation follows Vitess patterns and integrates with the existing throttle client infrastructure.

Tests

Unit tests for all new components (IncomingQueryThrottler, FileBasedConfigLoader, strategies)
Test strategy lifecycle management and switching
Test configuration loading error scenarios
Test concurrent access patterns with mutex protection
Verify integration with existing throttle client

Ref: #18412

Checklist

"Backport to:" labels have been added if this change should be back-ported to release branches
If this change is to be back-ported to previous releases, a justification is included in the PR description
Tests were added or are not required
Did the new or modified tests pass consistently locally and on CI?
Documentation was added or is not required

Deployment Notes

…ry throttler This commit introduces the core interfaces and configuration structures for the incoming query throttler system as outlined in RFC vitessio#18412. Key additions: - ThrottlingStrategyHandler interface for implementing throttling strategies - Config struct with support for multiple throttling strategies: - TabletThrottler (Vitess native tablet throttler) - Cinnamon (Uber's load-shedding system) - ConfigLoader interface for dynamic configuration loading This establishes the architectural foundation for query-level throttling that will be built upon in subsequent commits. Ref: vitessio#18412 Signed-off-by: siddharth16396 <[email protected]>

vitess-bot · 2025-07-15T07:20:02Z

codecov · 2025-07-15T08:04:58Z

Codecov Report

❌ Patch coverage is 90.05525% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.54%. Comparing base (770ae74) to head (13cb472).
⚠️ Report is 11 commits behind head on main.

Files with missing lines	Patch %	Lines
...let/tabletserver/querythrottler/query_throttler.go	88.31%	9 Missing ⚠️
go/vt/vttablet/tabletserver/query_executor.go	33.33%	4 Missing ⚠️
...t/tabletserver/querythrottler/registry/registry.go	92.50%	3 Missing ⚠️
go/vt/vttablet/tabletserver/tabletserver.go	80.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #18449      +/-   ##
==========================================
+ Coverage   67.51%   67.54%   +0.03%     
==========================================
  Files        1607     1613       +6     
  Lines      263357   263742     +385     
==========================================
+ Hits       177815   178155     +340     
- Misses      85542    85587      +45

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Add file-based configuration loader for the incoming query throttler system. This implementation reads throttler configuration from a JSON file at `/config/throttler-config.json` and supports the Config struct with enabled flag and throttling strategy selection. Key features: - Implements ConfigLoader interface for file-based config loading - Reads from fixed path /config/throttler-config.json - Supports JSON unmarshaling of Config struct with enabled/strategy fields - Comprehensive error handling for file read and JSON parsing failures - Extensive test coverage with stubbed dependencies using gostub library The FileBasedConfigLoader provides a foundation for loading throttler configuration from external files, supporting the broader incoming query throttling system architecture. Related to: vitessio#18412 Signed-off-by: siddharth16396 <[email protected]>

Add file-based configuration loader for the incoming query throttler system. This implementation reads throttler configuration from a JSON file at `/config/throttler-config.json` and supports the Config struct with enabled flag and throttling strategy selection. Key features: - Implements ConfigLoader interface for file-based config loading - Reads from fixed path /config/throttler-config.json - Supports JSON unmarshaling of Config struct with enabled/strategy fields - Comprehensive error handling for file read and JSON parsin failures - Extensive test coverage with stubbed dependencies using gostub library The FileBasedConfigLoader provides a foundation for loading throttler configuration from external files, supporting the broader incoming query throttling system architecture. Related to: vitessio#18412 Signed-off-by: siddharth16396 <[email protected]>

Signed-off-by: siddharth16396 <[email protected]>

This commit introduces a new incoming query throttling system for Vitess tablets with the following key components: Core Implementation: * IncomingQueryThrottler: Main throttler with configurable strategies and automatic config refresh * Strategy pattern with ThrottlingStrategyHandler interface for pluggable throttling algorithms * NoOpStrategy: Default no-operation strategy * CinnamonStrategy: Placeholder for future Cinnamon throttling implementation Key Features: * Automatic configuration refresh every minute via background goroutine * Thread-safe strategy switching with proper lifecycle management (start/stop) * Integration with existing Vitess throttle infrastructure via throttle.Client * Graceful shutdown with resource cleanup Signed-off-by: siddharth16396 <[email protected]>

Signed-off-by: siddharth16396 <[email protected]>

siddharth16396 · 2025-07-20T10:31:58Z

go/vt/vttablet/tabletserver/incomingquerythrottler/file_based_config_loader.go

+type FileBasedConfigLoader struct{}
+
+// NewFileBasedConfigLoader creates a new instance of FileBasedConfigLoader with the given file path.
+func NewFileBasedConfigLoader() *FileBasedConfigLoader {


We have a TopoServerConfig loader as well, but that will be a seperate PR by another team mate

siddharth16396 · 2025-07-20T10:32:33Z

go/vt/vttablet/tabletserver/incomingquerythrottler/incoming_query_throttler.go

+	case ThrottlingStrategyCinnamon:
+		return &CinnamonStrategy{}
+	case ThrottlingStrategyTabletThrottler:
+		fallthrough


This will be implemented as a seperate PR by me.

dbussink · 2025-07-20T11:11:41Z

go/vt/vttablet/tabletserver/incomingquerythrottler/file_based_config_loader.go

+
+var (
+	_              ConfigLoader = (*FileBasedConfigLoader)(nil)
+	_osReadFile                 = os.ReadFile


We should not update global state like this. Similar to below, we should pass in configuration like this and also then we don't need to mock / change _osReadFile.

That we add a dependency in go.mod here is a smell I think and also points at these problems.

I've incorporated the changes and used dependency injection for this

Signed-off-by: siddharth16396 <[email protected]>

harshit-gangal · 2025-07-29T13:35:42Z

go/vt/vttablet/tabletserver/incomingquerythrottler/cinnamon_strategy.go

why do we have this? There is no implementation.

Removed it, the implementation was uber specific code that couldn't be open sourced.

harshit-gangal · 2025-07-29T13:37:41Z

go/vt/vttablet/tabletserver/incomingquerythrottler/config.go

+type ThrottlingStrategy string
+
+// Predefined throttling strategies for the IncomingQueryThrottler.
+const (
+	// ThrottlingStrategyTabletThrottler uses Vitess Tablet Throttler to shed load
+	// from incoming queries when the tablet is under pressure.
+	// Reference: https://vitess.io/docs/21.0/reference/features/tablet-throttler/
+	ThrottlingStrategyTabletThrottler ThrottlingStrategy = "TabletThrottler"
+
+	// ThrottlingStrategyCinnamon uses Uber's Cinnamon load-shedding system
+	// to regulate incoming queries under high load conditions.
+	// Reference: https://www.uber.com/en-IN/blog/cinnamon-using-century-old-tech-to-build-a-mean-load-shedder/
+	ThrottlingStrategyCinnamon ThrottlingStrategy = "Cinnamon"
+
+	// ThrottlingStrategyUnknown is used when the strategy is not known.
+	ThrottlingStrategyUnknown ThrottlingStrategy = "Unknown"
+)
+
+// Config defines the runtime configuration for the IncomingQueryThrottler.
+// It specifies whether throttling is enabled and which strategy to use.
+type Config struct {
+	// Enabled indicates whether the throttler should actively apply throttling logic.
+	Enabled bool `json:"enabled"`
+
+	// Strategy selects which throttling strategy should be used.
+	Strategy ThrottlingStrategy `json:"strategy"`
+}


instead of this we can do factory based. where each implementation registers to the factory and then based on the config will load that strategy

This is done, this strategy is just a enum now.

go/vt/vttablet/tabletserver/incomingquerythrottler/file_based_config_loader.go

harshit-gangal · 2025-07-29T13:52:11Z

go/vt/vttablet/tabletserver/incomingquerythrottler/incoming_query_throttler.go

+var (
+	_configRefreshTicket = time.NewTicker(1 * time.Minute)
+)
+
+type IncomingQueryThrottler struct {
+	ctx            context.Context
+	throttleClient *throttle.Client
+	mu             sync.RWMutex
+	// cfg holds the current configuration for the throttler.


We should not use the global ticker here and pass the timer in constructor.
This way we will also remove the usage of gostub in the test.

This is taken care of now.

harshit-gangal · 2025-07-29T13:54:27Z

go/vt/vttablet/tabletserver/incomingquerythrottler/incoming_query_throttler.go

+				newCfg, err := i.cfgLoader.Load(i.ctx)
+				if err != nil {
+					log.Errorf("Error loading config: %v", err)
+				}


If there is an error loading the config, we cannot continue with this. It will panic with nil

Ack, i will be taking care of this

go/vt/vttablet/tabletserver/state_manager.go

harshit-gangal · 2025-09-08T14:43:34Z

"EnforceThrottlingIfNodeOverloaded" is called from
tabletserver.Execute. I will add this to this diff. We can plug this into QueryExecutor as well but we've plugged it into tabletserver.execute for now. I would love to know your thoughts on it

Both places have pros and cons. The way you call it now in execute() means that if the query is throttled, then you're saving all the query executor/plan computation.

On the other hand, the query plan parses the SQL statement, something that you're going to do as well. So there's waste of computation if the query does not get throttled.

I'm thinking the best solution would be to generate the plan, then use TabletPlan.FullStmt which is a sqlparser.Statement, as input to EnforceThrottlingIfNodeOverloaded. @harshit-gangal your opinion?

I think it make sense that we perform query throttling in query_executor. We anyways cache the plan so we would not be planning the queries under load as well.

siddharth16396 · 2025-09-08T15:21:27Z

I think it make sense that we perform query throttling in query_executor. We anyways cache the plan so we would not be planning the queries under load as well.

@harshit-gangal : Just to be clear, are you saying we move the call to EnforceThrottlingIfNodeOverloaded inside of query_executor.go ?

Or can we keep it in tabletserver.go with the TabletPlan.FullStmt change and call it before calling the qre.Execute() ?

harshit-gangal · 2025-09-08T16:03:42Z

stmtRules

Moving this to query_executor.go makes sense, and checking to throttle there is fine.
See how we use TxThrottler over there. Can we just name the method Throttle? The extra descriptive naming doesn’t seem necessary; the throttler implementation should determine when to throttle.

Regarding your Evaluate code: that logic should be done outside the hot path. The hot path call should return from a cached value instead of evaluating on the fly.

Signed-off-by: siddharth16396 <[email protected]>

siddharth16396 · 2025-09-08T17:54:51Z

I checked the Plan struct

// Plan contains the parameters for executing a request.
type Plan struct {
	PlanID PlanType
	// When the query indicates a single table
	Table *schema.Table
	// This indicates all the tables that are accessed in the query.
	AllTables []*schema.Table

	// Permissions stores the permissions for the tables accessed in the query.
	Permissions []Permission

	**// FullQuery will be set for all plans.
	FullQuery *sqlparser.ParsedQuery**

	// NextCount stores the count for "select next".
	NextCount evalengine.Expr

	// WhereClause is set for DMLs. It is used by the hot row protection
	// to serialize e.g. UPDATEs going to the same row.
	WhereClause *sqlparser.ParsedQuery

	**// FullStmt can be used when the query does not operate on tables
	FullStmt sqlparser.Statement**

	// NeedsReservedConn indicates at a reserved connection is needed to execute this plan
	NeedsReservedConn bool
}

This says // FullStmt can be used when the query does not operate on tables

So should we be using FullQuery instead of FullStmt? @shlomi-noach

I'm thinking the best solution would be to generate the plan, then use TabletPlan.FullStmt which is a sqlparser.Statement, as input to EnforceThrottlingIfNodeOverloaded. @harshit-gangal your opinion?

siddharth16396 · 2025-09-08T17:59:19Z

Also, to move the EnforceThrottlingIfNodeOverloaded function to query_executor, i will have to add ctx , transactionID to QueryExecutor struct, which seems safe but still giving a heads-up

Signed-off-by: siddharth16396 <[email protected]>

query_executor.go Signed-off-by: siddharth16396 <[email protected]>

Signed-off-by: siddharth16396 <[email protected]>

siddharth16396 · 2025-09-11T00:04:00Z

Incorporated all the requested changes @harshit-gangal | @shlomi-noach

harshit-gangal · 2025-09-13T10:43:43Z

go/vt/vttablet/tabletserver/query_executor.go

 		return nil, err
 	}

+	if reqThrottledErr := qre.tsv.queryThrottler.Throttle(qre.ctx, qre.targetTabletType, qre.plan.FullQuery, qre.logStats.TransactionID, qre.options); reqThrottledErr != nil {


connID and TransactionID is same

Signed-off-by: siddharth16396 <[email protected]>

shlomi-noach

Minor last styling comment, otherwise good to go!

shlomi-noach · 2025-09-14T08:45:11Z

go/vt/vttablet/tabletserver/querythrottler/query_throttler.go

+	tCfg := qt.cfg
+	tStrategy := qt.strategy


Is there an intentional reason why these two values are copied?

qt.cfg is not a pointer, the entire config is duplicated. But I don't see the need.

qt.strategy is an interface so there's no additional cost, but then why not just decision := qt.strategy.Evaluate(...)?

There was a reason initially because we used to have read locks on this, but that caused performance degradations so we removed the locks and so we can remove the variable assignments. let me incorp that.

Thanks for pointing out.

go/vt/vttablet/tabletserver/querythrottler/query_throttler.go

removing unnecessary variable assignments Co-authored-by: Shlomi Noach <[email protected]> Signed-off-by: siddharth16396 <[email protected]>

…r-first Signed-off-by: siddharth16396 <[email protected]>

…first Signed-off-by: siddharth16396 <[email protected]>

Signed-off-by: siddharth16396 <[email protected]>

siddharth16396 · 2025-09-14T17:30:51Z

@shlomi-noach : bd07b5a added the same comment twice, so had to remove it 😅

siddharth16396 requested review from harshit-gangal, rohit-nayak-ps, shlomi-noach, systay and timvaillancourt as code owners July 15, 2025 07:20

github-actions bot added this to the v23.0.0 milestone Jul 15, 2025

siddharth16396 added 2 commits July 20, 2025 15:36

siddharth16396 requested review from frouioui and mattlord as code owners July 20, 2025 10:10

siddharth16396 added 3 commits July 20, 2025 15:46

IncomingQueryThrottler: Add NOOP strategy

68a4fb2

Signed-off-by: siddharth16396 <[email protected]>

minor lints

50ce921

Signed-off-by: siddharth16396 <[email protected]>

siddharth16396 commented Jul 20, 2025

View reviewed changes

siddharth16396 changed the title ~~[IncomingQueryThrottler] Add foundational interfaces for throttler~~ [IncomingQueryThrottler] Add foundation code for throttling incoming queries Jul 20, 2025

siddharth16396 changed the title ~~[IncomingQueryThrottler] Add foundation code for throttling incoming queries~~ [IncomingQueryThrottler] Implement Incoming Query Throttler with Strategy Pattern Jul 20, 2025

dbussink reviewed Jul 20, 2025

View reviewed changes

Use Dependency Injection instead of stubs in FileBasedConfigLoader

bfa35b6

Signed-off-by: siddharth16396 <[email protected]>

harshit-gangal reviewed Jul 29, 2025

View reviewed changes

go/vt/vttablet/tabletserver/incomingquerythrottler/file_based_config_loader.go Show resolved Hide resolved

harshit-gangal reviewed Jul 29, 2025

View reviewed changes

shlomi-noach reviewed Sep 8, 2025

View reviewed changes

go/vt/vttablet/tabletserver/state_manager.go Outdated Show resolved Hide resolved

Remove all existing references of the word "incoming" from throttler

58b4c96

Signed-off-by: siddharth16396 <[email protected]>

siddharth16396 added 3 commits September 10, 2025 17:21

s/EnforceThrottlingIfNodeOverloaded/Throttle

2577aff

Signed-off-by: siddharth16396 <[email protected]>

Use sqlparser.FullQuery as Throttle input + Move throttle check to

ba86971

query_executor.go Signed-off-by: siddharth16396 <[email protected]>

Add COPYRIGHT comment to all new files

d53b367

Signed-off-by: siddharth16396 <[email protected]>

harshit-gangal reviewed Sep 13, 2025

View reviewed changes

use connID as transaction ID in query_executor

d0b476b

Signed-off-by: siddharth16396 <[email protected]>

harshit-gangal approved these changes Sep 13, 2025

View reviewed changes

shlomi-noach reviewed Sep 14, 2025

View reviewed changes

Update go/vt/vttablet/tabletserver/querythrottler/query_throttler.go

bd07b5a

removing unnecessary variable assignments Co-authored-by: Shlomi Noach <[email protected]> Signed-off-by: siddharth16396 <[email protected]>

shlomi-noach enabled auto-merge (squash) September 14, 2025 17:20

siddharth16396 added 3 commits September 14, 2025 22:53

Merge remote-tracking branch 'upstream/throttler-first' into throttle…

25cbb48

…r-first Signed-off-by: siddharth16396 <[email protected]>

Merge remote-tracking branch 'origin/throttler-first' into throttler-…

87f36d4

…first Signed-off-by: siddharth16396 <[email protected]>

remove duplicate comment

13cb472

Signed-off-by: siddharth16396 <[email protected]>

auto-merge was automatically disabled September 14, 2025 17:29
Head branch was pushed to by a user without write access

siddharth16396 changed the title ~~[QueryThrottler] Implement Query Throttler with Strategy Pattern~~ [QueryThrottler] Implement Query Throttler Part #1 Sep 14, 2025

siddharth16396 changed the title ~~[QueryThrottler] Implement Query Throttler Part #1~~ [QueryThrottler] Implement Query Throttler [Part 1] Sep 14, 2025

siddharth16396 changed the title ~~[QueryThrottler] Implement Query Throttler [Part 1]~~ [QueryThrottler] Implement Query Throttler Sep 14, 2025

shlomi-noach enabled auto-merge (squash) September 15, 2025 05:10

shlomi-noach approved these changes Sep 15, 2025

View reviewed changes

shlomi-noach merged commit 384b553 into vitessio:main Sep 15, 2025
103 of 106 checks passed

siddharth16396 mentioned this pull request Sep 16, 2025

[Query Throttler] Add dry-run mode #18657

Merged

5 tasks

BrewTestBot mentioned this pull request Nov 4, 2025

vitess 23.0.0 Homebrew/homebrew-core#252716

Merged

[QueryThrottler] Implement Query Throttler #18449

[QueryThrottler] Implement Query Throttler #18449

Uh oh!

Conversation

siddharth16396 commented Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Deployment Notes

Uh oh!

vitess-bot bot commented Jul 15, 2025

Review Checklist

General

Tests

Documentation

New flags

If a workflow is added or modified:

Backward compatibility

Uh oh!

codecov bot commented Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

harshit-gangal commented Sep 8, 2025

Uh oh!

siddharth16396 commented Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

harshit-gangal commented Sep 8, 2025

Uh oh!

siddharth16396 commented Sep 8, 2025

Uh oh!

siddharth16396 commented Sep 8, 2025

Uh oh!

siddharth16396 commented Sep 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shlomi-noach left a comment

Choose a reason for hiding this comment

Uh oh!

shlomi-noach Sep 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

siddharth16396 commented Sep 14, 2025

Uh oh!

Uh oh!

Reviewers

siddharth16396 commented Jul 15, 2025 •

edited

Loading

codecov bot commented Jul 15, 2025 •

edited

Loading

siddharth16396 commented Sep 8, 2025 •

edited

Loading

shlomi-noach Sep 14, 2025 •

edited

Loading