server: Support explicitly getting health feedback information by calling RPC #17008

MyonKeminta · 2024-05-13T09:20:08Z

What is changed and how it works?

Issue Number: Close #16297

What's Changed:

This PR makes TiKV support explicitly getting health feedback information by calling RPC. Both non-batched mode and batched mode (using BatchCommands stream) are supported.
There's some special behavior when used in batched RPC mode: The BatchCommandsResponse that contains the response of getting health feedback will always have feedback information attached (in the same way as how it's attached without being explicitly requested), and the attached information and the information carried in each single GetHealthFeedbackResponse-s is the same.

Related changes

PR to update pingcap/docs/pingcap/docs-cn:
Need to cherry-pick to the release branch
Requires: Add GetHealthFeedback RPC pingcap/kvproto#1239

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Release note

None

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

…request-health-feedback

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

ti-chi-bot · 2024-05-13T09:20:11Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

cfzjywxk
ekexium

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.

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

…request-health-feedback

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

MyonKeminta · 2024-05-14T07:41:18Z

cc @crazycs520 PTAL thanks!

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

crazycs520 · 2024-05-14T10:01:13Z

src/server/service/kv.rs

+                if requested_feedback.is_none() {
+                    requested_feedback = Some(self.gen_health_feedback_pb());
+                }


When will this logic be used?

There is explicit health fetch request in the batch commands I think.

To make it a complete API, GetHealthFeedbackResponse carries the health feedback information. We don't actually use it for now, but it makes further potential uses possible. When it's used in batch_commands, the feedback information will also be attached to the BatchCommandsResponse, so that it can be easily handled by the batch client, and it's transparent to the batch client whether the feedback is periodically sent or explicitly requested.
Here the code is used to check a BatchCommandsResponse, and attach the information if explicitly requested (by checking whether there is one or more GetHealthFeedbackResponse).

cfzjywxk

LGTM

cfzjywxk · 2024-05-14T10:04:58Z

src/server/service/kv.rs

@@ -1042,7 +1042,9 @@ impl<E: Engine, L: LockManager, F: KvFormat> Tikv for Service<E, L, F> {
            GRPC_RESP_BATCH_COMMANDS_SIZE.observe(r.request_ids.len() as f64);
            // TODO: per thread load is more reasonable for batching.
            r.set_transport_layer_load(grpc_thread_load.total_load() as u64);
+


Unrelated blank lines?

cfzjywxk · 2024-05-14T10:16:38Z

src/server/service/kv.rs

+                if requested_feedback.is_none() {
+                    requested_feedback = Some(self.gen_health_feedback_pb());
+                }


There is explicit health fetch request in the batch commands I think.

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

ti-chi-bot · 2024-05-15T06:26:34Z

@crazycs520: Thanks for your review. The bot only counts approvals from reviewers and higher roles in list, but you're still welcome to leave your comments.

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 ti-community-infra/tichi repository.

ti-chi-bot · 2024-05-15T08:23:09Z

@crazycs520: Thanks for your review. The bot only counts approvals from reviewers and higher roles in list, but you're still welcome to leave your comments.

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 ti-community-infra/tichi repository.

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

MyonKeminta · 2024-05-16T06:27:33Z

/merge

ti-chi-bot · 2024-05-16T06:27:35Z

@MyonKeminta: 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 · 2024-05-16T06:27:38Z

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

Commit hash: cb9e4f4

MyonKeminta · 2024-05-16T08:05:31Z

/release

MyonKeminta added 4 commits April 9, 2024 18:06

Support requesting health feedback

800f7b9

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

Merge branch 'master' of https://github.com/tikv/tikv into m/support-…

13f0792

…request-health-feedback

reimplement

63ba869

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

Add integration tests

b2a9a30

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

ti-chi-bot bot added do-not-merge/needs-linked-issue release-note-none size/L labels May 13, 2024

MyonKeminta marked this pull request as draft May 13, 2024 09:20

ti-chi-bot bot added do-not-merge/work-in-progress and removed do-not-merge/needs-linked-issue labels May 13, 2024

MyonKeminta added 3 commits May 13, 2024 17:56

Update kvproto

cca6421

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

Merge branch 'master' of https://github.com/tikv/tikv into m/support-…

38b1f9b

…request-health-feedback

update kvproto

bd62b59

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

MyonKeminta marked this pull request as ready for review May 13, 2024 10:00

ti-chi-bot bot removed the do-not-merge/work-in-progress label May 13, 2024

MyonKeminta mentioned this pull request May 13, 2024

Support actively requesting update health feedback information by calling RPC to TiKV tikv/client-go#1287

Merged

make lint happy

cbd40f7

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

MyonKeminta requested review from cfzjywxk and zyguan May 14, 2024 07:41

Renaming

a2d0b84

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

crazycs520 reviewed May 14, 2024

View reviewed changes

cfzjywxk approved these changes May 14, 2024

View reviewed changes

ti-chi-bot bot added the status/LGT1 Status: PR - There is already 1 approval label May 14, 2024

remove uncecessary empty lines; add more comments

ad04294

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

crazycs520 approved these changes May 15, 2024

View reviewed changes

Make clippy happy

cb9e4f4

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

ekexium approved these changes May 16, 2024

View reviewed changes

ti-chi-bot bot added status/LGT2 Status: PR - There are already 2 approvals and removed status/LGT1 Status: PR - There is already 1 approval labels May 16, 2024

ti-chi-bot bot added the status/can-merge Status: Can merge to base branch label May 16, 2024

Merge branch 'master' into m/support-request-health-feedback

3583aeb

ti-chi-bot bot merged commit 2a865bb into tikv:master May 16, 2024
4 checks passed

ti-chi-bot bot added this to the Pool milestone May 16, 2024

MyonKeminta deleted the m/support-request-health-feedback branch May 16, 2024 06:54

MyonKeminta mentioned this pull request May 31, 2024

Performance/health feedback to client and the Unified Health Controller #16297

Closed

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

server: Support explicitly getting health feedback information by calling RPC #17008

server: Support explicitly getting health feedback information by calling RPC #17008

MyonKeminta commented May 13, 2024 •

edited

ti-chi-bot bot commented May 13, 2024 •

edited

MyonKeminta commented May 14, 2024

crazycs520 May 14, 2024

cfzjywxk May 14, 2024

MyonKeminta May 15, 2024

cfzjywxk left a comment

cfzjywxk May 14, 2024

cfzjywxk May 14, 2024

ti-chi-bot bot commented May 15, 2024

ti-chi-bot bot commented May 15, 2024

MyonKeminta commented May 16, 2024

ti-chi-bot bot commented May 16, 2024

ti-chi-bot bot commented May 16, 2024

MyonKeminta commented May 16, 2024

server: Support explicitly getting health feedback information by calling RPC #17008

server: Support explicitly getting health feedback information by calling RPC #17008

Conversation

MyonKeminta commented May 13, 2024 • edited

What is changed and how it works?

Related changes

Check List

Release note

ti-chi-bot bot commented May 13, 2024 • edited

MyonKeminta commented May 14, 2024

crazycs520 May 14, 2024

Choose a reason for hiding this comment

cfzjywxk May 14, 2024

Choose a reason for hiding this comment

MyonKeminta May 15, 2024

Choose a reason for hiding this comment

cfzjywxk left a comment

Choose a reason for hiding this comment

cfzjywxk May 14, 2024

Choose a reason for hiding this comment

cfzjywxk May 14, 2024

Choose a reason for hiding this comment

ti-chi-bot bot commented May 15, 2024

ti-chi-bot bot commented May 15, 2024

MyonKeminta commented May 16, 2024

ti-chi-bot bot commented May 16, 2024

ti-chi-bot bot commented May 16, 2024

MyonKeminta commented May 16, 2024

MyonKeminta commented May 13, 2024 •

edited

ti-chi-bot bot commented May 13, 2024 •

edited