server: add gRPC rate limit #6834

rleungx · 2023-07-24T10:52:36Z

What problem does this PR solve?

Issue Number: ref #6556. close #5739.

What is changed and how does it work?

This PR adds a gRPC rate limit for some interfaces to prevent wasting lots of resources.

Check List

Tests

Unit test

Release note

None.

ti-chi-bot · 2023-07-24T10:52:37Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

CabinfeverB
nolouch

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

ti-chi-bot · 2023-07-24T10:52:38Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

codecov · 2023-07-25T08:12:03Z

Codecov Report

Merging #6834 (6696c12) into master (ebceb83) will decrease coverage by 0.01%.
Report is 1 commits behind head on master.
The diff coverage is 53.68%.

❗ Current head 6696c12 differs from pull request most recent head 77a9848. Consider uploading reports for the commit 77a9848 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6834      +/-   ##
==========================================
- Coverage   74.23%   74.23%   -0.01%     
==========================================
  Files         433      427       -6     
  Lines       45860    45508     -352     
==========================================
- Hits        34046    33784     -262     
+ Misses       8802     8741      -61     
+ Partials     3012     2983      -29

Flag	Coverage Δ
unittests	`74.23% <53.68%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

errors.toml

nolouch · 2023-07-25T09:44:54Z

server/grpc_service.go

+			defer limiter.Release(fName)
+		} else {
+			return &pdpb.GetStoreResponse{
+				Header: s.wrapErrorToHeader(pdpb.ErrorType_UNKNOWN, errs.ErrReachRateLimit.FastGenByArgs().Error()),


maybe we need to define an error type that can let the client to handle it.

CabinfeverB · 2023-07-31T06:49:26Z

server/grpc_service.go

@@ -848,6 +871,17 @@ func (s *GrpcServer) PutStore(ctx context.Context, request *pdpb.PutStoreRequest

 // GetAllStores implements gRPC PDServer.
 func (s *GrpcServer) GetAllStores(ctx context.Context, request *pdpb.GetAllStoresRequest) (*pdpb.GetAllStoresResponse, error) {


How abou renaming GetStores to GetAllStores in http-api's router.go?

CabinfeverB · 2023-07-31T06:59:11Z

server/server.go

@@ -299,8 +305,8 @@ func CreateServer(ctx context.Context, cfg *config.Config, services []string, le
 		diagnosticspb.RegisterDiagnosticsServer(gs, s)
 		// Register the micro services GRPC service.
 		s.registry.InstallAllGRPCServices(s, gs)
+		s.grpcServer = gs


When exec initGRPCServiceLabels, will it fetch the methods of ETCD gRPC server?

Yes, we only have one gRPC server.

ti-chi-bot · 2023-08-04T06:06:15Z

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

nolouch

lgtm

Signed-off-by: Ryan Leung <rleungx@gmail.com>

nolouch · 2023-08-23T10:10:07Z

/merge

ti-chi-bot · 2023-08-23T10:10:09Z

@nolouch: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

ti-chi-bot · 2023-08-23T10:10:10Z

This pull request has been accepted and is ready to merge.

Commit hash: 9b6440a

ti-chi-bot · 2023-08-23T10:14:16Z

@rleungx: Your PR was out of date, I have automatically updated it for you.

If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

ti-chi-bot bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. labels Jul 24, 2023

ti-chi-bot bot requested review from disksing and HunDunDM July 24, 2023 10:52

ti-chi-bot bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 24, 2023

rleungx marked this pull request as ready for review July 25, 2023 08:00

ti-chi-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 25, 2023

rleungx requested review from nolouch and CabinfeverB and removed request for disksing and HunDunDM July 25, 2023 08:02

nolouch reviewed Jul 25, 2023

View reviewed changes

errors.toml Outdated Show resolved Hide resolved

nolouch reviewed Jul 25, 2023

View reviewed changes

rleungx force-pushed the grpc-rate-limit branch from 58ee339 to d1c879a Compare July 26, 2023 06:28

CabinfeverB reviewed Jul 31, 2023

View reviewed changes

CabinfeverB approved these changes Aug 2, 2023

View reviewed changes

ti-chi-bot bot added status/LGT1 Indicates that a PR has LGTM 1. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Aug 2, 2023

nolouch approved these changes Aug 8, 2023

View reviewed changes

ti-chi-bot bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Aug 8, 2023

rleungx force-pushed the grpc-rate-limit branch from 1feff7f to 6696c12 Compare August 21, 2023 09:26

ti-chi-bot bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Aug 21, 2023

rleungx added 5 commits August 23, 2023 10:09

add gRPC rate limit

0cf1c5e

Signed-off-by: Ryan Leung <rleungx@gmail.com>

add tests for grpc rate limit

88ecfa3

Signed-off-by: Ryan Leung <rleungx@gmail.com>

address the comment

b2e86fb

Signed-off-by: Ryan Leung <rleungx@gmail.com>

address the comment

b2b3864

Signed-off-by: Ryan Leung <rleungx@gmail.com>

resolve the conflicts

9b6440a

Signed-off-by: Ryan Leung <rleungx@gmail.com>

rleungx force-pushed the grpc-rate-limit branch from 6696c12 to 9b6440a Compare August 23, 2023 02:09

nolouch approved these changes Aug 23, 2023

View reviewed changes

nolouch removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 23, 2023

ti-chi-bot bot added the status/can-merge Indicates a PR has been approved by a committer. label Aug 23, 2023

ti-chi-bot bot added 2 commits August 23, 2023 10:10

Merge branch 'master' into grpc-rate-limit

02dbca7

Merge branch 'master' into grpc-rate-limit

77a9848

ti-chi-bot bot merged commit fbd386a into tikv:master Aug 24, 2023
19 checks passed

rleungx deleted the grpc-rate-limit branch August 24, 2023 02:49

CabinfeverB mentioned this pull request Aug 24, 2023

grpc/api: should we limit the concurrency request access to PD/ETCD in PD #6604

Closed

Tema mentioned this pull request Oct 3, 2024

Implement Circuit Breaker Pattern to Protect PD Leader #8678

Open

rleungx mentioned this pull request Oct 11, 2024

Improve usability of the gRPC rate limiter #8691

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

server: add gRPC rate limit #6834

server: add gRPC rate limit #6834

rleungx commented Jul 24, 2023 •

edited

Loading

ti-chi-bot bot commented Jul 24, 2023 •

edited

Loading

ti-chi-bot bot commented Jul 24, 2023

codecov bot commented Jul 25, 2023 •

edited

Loading

nolouch Jul 25, 2023

CabinfeverB Jul 31, 2023

CabinfeverB Jul 31, 2023

rleungx Jul 31, 2023

ti-chi-bot bot commented Aug 4, 2023

nolouch left a comment

nolouch commented Aug 23, 2023

ti-chi-bot bot commented Aug 23, 2023

ti-chi-bot bot commented Aug 23, 2023

ti-chi-bot bot commented Aug 23, 2023

		@@ -848,6 +871,17 @@ func (s GrpcServer) PutStore(ctx context.Context, request pdpb.PutStoreRequest

		// GetAllStores implements gRPC PDServer.
		func (s GrpcServer) GetAllStores(ctx context.Context, request pdpb.GetAllStoresRequest) (*pdpb.GetAllStoresResponse, error) {

server: add gRPC rate limit #6834

server: add gRPC rate limit #6834

Conversation

rleungx commented Jul 24, 2023 • edited Loading

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

ti-chi-bot bot commented Jul 24, 2023 • edited Loading

ti-chi-bot bot commented Jul 24, 2023

codecov bot commented Jul 25, 2023 • edited Loading

Codecov Report

nolouch Jul 25, 2023

Choose a reason for hiding this comment

CabinfeverB Jul 31, 2023

Choose a reason for hiding this comment

CabinfeverB Jul 31, 2023

Choose a reason for hiding this comment

rleungx Jul 31, 2023

Choose a reason for hiding this comment

ti-chi-bot bot commented Aug 4, 2023

nolouch left a comment

Choose a reason for hiding this comment

nolouch commented Aug 23, 2023

ti-chi-bot bot commented Aug 23, 2023

ti-chi-bot bot commented Aug 23, 2023

ti-chi-bot bot commented Aug 23, 2023

rleungx commented Jul 24, 2023 •

edited

Loading

ti-chi-bot bot commented Jul 24, 2023 •

edited

Loading

codecov bot commented Jul 25, 2023 •

edited

Loading