Skip to content
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

Add service-middleware config for pd-ctl #19023

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

rleungx
Copy link
Member

@rleungx rleungx commented Nov 7, 2024

First-time contributors' checklist

What is changed, added or deleted? (Required)

Which TiDB version(s) do your changes apply to? (Required)

Tips for choosing the affected version(s):

By default, CHOOSE MASTER ONLY so your changes will be applied to the next TiDB major or minor releases. If your PR involves a product feature behavior change or a compatibility change, CHOOSE THE AFFECTED RELEASE BRANCH(ES) AND MASTER.

For details, see tips for choosing the affected versions (in Chinese).

  • master (the latest development version)
  • v8.5 (TiDB 8.5 versions)
  • v8.4 (TiDB 8.4 versions)
  • v8.3 (TiDB 8.3 versions)
  • v8.2 (TiDB 8.2 versions)
  • v8.1 (TiDB 8.1 versions)
  • v7.5 (TiDB 7.5 versions)
  • v7.1 (TiDB 7.1 versions)
  • v6.5 (TiDB 6.5 versions)
  • v6.1 (TiDB 6.1 versions)
  • v5.4 (TiDB 5.4 versions)
  • v5.3 (TiDB 5.3 versions)

What is the related PR or file link(s)?

  • This PR is translated from:
  • Other reference link(s):

Do your changes match any of the following descriptions?

  • Delete files
  • Change aliases
  • Need modification after applied to another branch
  • Might cause conflicts after applied to another branch

Signed-off-by: Ryan Leung <rleungx@gmail.com>
@ti-chi-bot ti-chi-bot bot added missing-translation-status This PR does not have translation status info. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 7, 2024
@qiancai qiancai self-requested a review November 8, 2024 01:51
@qiancai qiancai self-assigned this Nov 8, 2024
@qiancai qiancai added v8.5 area/scheduling Indicates that the Issue or PR belongs to the area of scheduling. labels Nov 8, 2024
@rleungx
Copy link
Member Author

rleungx commented Nov 8, 2024

@niubell @okJiang PTAL

@qiancai qiancai added translation/doing This PR’s assignee is translating this PR. and removed missing-translation-status This PR does not have translation status info. labels Nov 8, 2024
pd-control.md Show resolved Hide resolved
pd-control.md Outdated Show resolved Hide resolved
Signed-off-by: Ryan Leung <rleungx@gmail.com>
@rleungx rleungx requested a review from okJiang November 12, 2024 07:53
Copy link

ti-chi-bot bot commented Nov 12, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from qiancai, ensuring that each of them provides their approval before proceeding. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Signed-off-by: Ryan Leung <rleungx@gmail.com>
pd-control.md Outdated
Comment on lines 475 to 476
{{< copyable "" >}}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{{< copyable "" >}}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

文档站目前默认会给所有代码加上复制按钮,所以不需要单独加 copyable 标识了

pd-control.md Outdated
Comment on lines 475 to 476
{{< copyable "" >}}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

文档站目前默认会给所有代码加上复制按钮,所以不需要单独加 copyable 标识了

pd-control.md Outdated Show resolved Hide resolved
pd-control.md Outdated
}
```

控制某个调用的 rate limit,以 GetRegion 调用为例:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
控制某个调用的 rate limit,以 GetRegion 调用为例:
控制某个调用的 rate limit,以 `GetRegion` 调用为例:

pd-control.md Outdated
config set service-middleware grpc-rate-limit GetRegion qps 100
```

控制某个调用的并发,以 GetRegion 调用为例:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
控制某个调用的并发,以 GetRegion 调用为例:
控制某个调用的并发度,以 `GetRegion` 调用为例:

pd-control.md Outdated
Comment on lines 507 to 509
查看修改后的配置

{{< copyable "" >}}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
查看修改后的配置
{{< copyable "" >}}
查看修改后的配置:

pd-control.md Outdated
}
```

重置上述设置
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
重置上述设置
重置上述设置

pd-control.md Show resolved Hide resolved
rleungx and others added 2 commits November 18, 2024 14:04
Signed-off-by: Ryan Leung <rleungx@gmail.com>
pd-control.md Outdated Show resolved Hide resolved
@ti-chi-bot ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Nov 20, 2024
pd-control.md Outdated Show resolved Hide resolved
pd-control.md Outdated Show resolved Hide resolved
"audit": {
"enable-audit": "true"
},
"rate-limit": {
Copy link
Contributor

@niubell niubell Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rleungx 上面@okJiang 说的有道理,我们在执行 command show 的时候能不能把 audit、rate-limiter 都隐藏掉呢?否则容易误用,既:文档里不暴露,命令行里(或者server 侧直接禁用掉、不返回)也隐藏掉

Signed-off-by: Ryan Leung <rleungx@gmail.com>
@ti-chi-bot ti-chi-bot bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 21, 2024
pd-control.md Outdated
`service-middleware` 是 PD 中的一个配置模块,主要用于管理和控制 PD 服务的中间件功能,如审计和请求速率限制等。从 v8.5.0 起,你可以通过 `service-middleware` 控制以下 gRPC API 请求的速率和并发度:
> **注意:**
>
> 通常不建议用户对请求速率限制和并发限制进行修改,此类配置可能会引起业务报错。
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

业务报错

这是不是太严重了😂

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这一段可以放在后面一点的位置,放在前面,用户还不知道 请求速率限制和并发限制 是什么

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

还有,请求速率限制和并发限制 是 HTTP 的还是 gRPC 的,还是都不建议使用?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

都不建议

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

放在后面感觉不够警示?

Copy link
Member

@okJiang okJiang Nov 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

如果都不建议设置的话,可以放在

  • audit: 审计功能
  • rate-limit: 用于限制 HTTP API 请求的最大速率和最大并发
  • grpc-rate-limit: 用于限制 gRPC API 请求的最大速率和最大并发

后面,就直接说不建议设置 rate-limit/grpc-rate-limit 可以吧。这样就很明确了

pd-control.md Outdated Show resolved Hide resolved
pd-control.md Outdated Show resolved Hide resolved
pd-control.md Outdated
config set service-middleware audit enable-audit true
```

你可以通过 `service-middleware grpc-rate-limit` 控制以下 gRPC API 请求的速率和并发度:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

如果只能控制这三个的话,可以加下

Suggested change
你可以通过 `service-middleware grpc-rate-limit` 控制以下 gRPC API 请求的速率和并发度:
你可以通过 `service-middleware grpc-rate-limit` 控制以下三个 gRPC API 请求的速率和并发度:

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不是,只暴露三个

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里不写“三个”也可以,不影响理解。而且,以后要是新增或减少了支持的 API 请求个数,也避免了忘记修改这里

pd-control.md Outdated Show resolved Hide resolved
pd-control.md Outdated
"grpc-limiter-config": {
"GetRegion": {
"QPS": 100,
"QPSBurst": 100,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个是作什么用的呢?不能设置?

可以加个注释

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不能设置

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个好像也不太好解释

Copy link
Member

@okJiang okJiang Nov 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

那感觉就在注释里加上,仅作展示就行了

Signed-off-by: Ryan Leung <rleungx@gmail.com>
Signed-off-by: Ryan Leung <rleungx@gmail.com>
pd-control.md Outdated
"grpc-limiter-config": {
"GetRegion": {
"QPS": 100,
"QPSBurst": 100, // 根据 QPS 设置自动调整
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"QPSBurst": 100, // 根据 QPS 设置自动调整
"QPSBurst": 100, // 根据 QPS 设置自动调整,仅作展示

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/cc @qiancai 这个 ok 不

Copy link
Member

@okJiang okJiang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rest lgtm

Signed-off-by: Ryan Leung <rleungx@gmail.com>
pd-control.md Outdated

> **注意:**
>
> 通常不建议用户对 service-middleware 中的配置进行修改。
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
> 通常不建议用户对 service-middleware 中的配置进行修改。
> 为了避免请求速率限制和并发限制对 PD 性能的影响,通常不建议用户对 `service-middleware` 中的配置进行修改。

pd-control.md Outdated



`service-middleware` 是 PD 中的一个配置模块,主要用于管理和控制 PD 服务的中间件功能,如审计,请求速率和并发限制等。从 v8.5.0 起,支持通过 pd-ctl 修改 `service-middleware` 配置, `option` 支持如下几种类型:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
`service-middleware` 是 PD 中的一个配置模块,主要用于管理和控制 PD 服务的中间件功能,如审计,请求速率和并发限制等。从 v8.5.0 起,支持通过 pd-ctl 修改 `service-middleware` 配置, `option` 支持如下几种类型
`service-middleware` 是 PD 中的一个配置模块,主要用于管理和控制 PD 服务的中间件功能,如审计、请求速率限制和并发限制等。从 v8.5.0 起,PD 支持通过 `pd-ctl` 修改 `service-middleware` 的以下配置

pd-control.md Outdated
Comment on lines 479 to 481
- audit: 控制是否开启 HTTP 请求的审计日志。开启后,会在 PD 日志中记录 HTTP 请求的相关信息,默认开启
- rate-limit: 用于限制 HTTP API 请求的最大速率和最大并发
- grpc-rate-limit: 用于限制 gRPC API 请求的最大速率和最大并发
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- audit: 控制是否开启 HTTP 请求的审计日志。开启后,会在 PD 日志中记录 HTTP 请求的相关信息,默认开启
- rate-limit: 用于限制 HTTP API 请求的最大速率和最大并发
- grpc-rate-limit: 用于限制 gRPC API 请求的最大速率和最大并发
- `audit`控制是否开启 PD 处理 HTTP 请求的审计日志(默认开启)。开启时,`service-middleware` 会在 PD 日志中记录 HTTP 请求的相关信息
- `rate-limit`用于限制 PD 处理 HTTP API 请求的最大速率和最大并发
- `grpc-rate-limit`用于限制 PD 处理 gRPC API 请求的最大速率和最大并发

pd-control.md Outdated
Comment on lines 509 to 511
你可以通过 `service-middleware audit` 控制审计功能:

关闭审计功能
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
你可以通过 `service-middleware audit` 控制审计功能:
关闭审计功能
`service-middleware audit` 用于开启或关闭 HTTP 请求的日志审计功能。以关闭该功能为例:

pd-control.md Outdated
config set service-middleware audit enable-audit false
```

你可以通过 `service-middleware grpc-rate-limit` 控制以下 gRPC API 请求的速率和并发度:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
你可以通过 `service-middleware grpc-rate-limit` 控制以下 gRPC API 请求的速率和并发度
`service-middleware grpc-rate-limit` 用于控制以下 gRPC API 请求的最大速率和并发度

pd-control.md Outdated
config set service-middleware grpc-rate-limit GetRegion concurrency 0
```

同理,你可以通过 `service-middleware rate-limit` 控制以下 HTTP API 请求的速率和并发度:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
同理,你可以通过 `service-middleware rate-limit` 控制以下 HTTP API 请求的速率和并发度
`service-middleware rate-limit` 用于控制以下 HTTP API 请求的最大速率和并发度

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

ti-chi-bot bot commented Nov 22, 2024

@okJiang: adding LGTM is restricted to approvers and reviewers in OWNERS files.

In response to this:

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-sigs/prow repository.

@Oreoxmt Oreoxmt self-requested a review November 22, 2024 08:19
pd-control.md Outdated

> **注意:**
>
> 为了避免请求速率限制和并发限制对 PD 性能的影响,通常不建议用户对 `service-middleware` 中的配置进行修改。
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
> 为了避免请求速率限制和并发限制对 PD 性能的影响,通常不建议用户对 `service-middleware` 中的配置进行修改
> 为了避免请求速率限制和并发限制对 PD 性能的影响,不建议修改 `service-middleware` 中的配置

Copy link
Collaborator

@Oreoxmt Oreoxmt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rest LGTM

@ti-chi-bot ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Nov 22, 2024
Copy link

ti-chi-bot bot commented Nov 22, 2024

[LGTM Timeline notifier]

Timeline:

  • 2024-11-20 08:33:39.644309829 +0000 UTC m=+20607.263964344: ☑️ agreed by qiancai.
  • 2024-11-22 08:55:13.243505734 +0000 UTC m=+194700.863160250: ☑️ agreed by Oreoxmt.

Signed-off-by: Ryan Leung <rleungx@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/scheduling Indicates that the Issue or PR belongs to the area of scheduling. lgtm size/L Denotes a PR that changes 100-499 lines, ignoring generated files. translation/doing This PR’s assignee is translating this PR. v8.5
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants