Support resource group watch #5830

HuSharp · 2023-01-05T01:34:04Z

What problem does this PR solve?

Issue Number: Close #5794 Ref tikv/tikv#14022

What is changed and how does it work?

Support resource group watch

Check List

Tests

Unit test
Integration test

Release note

None

ti-chi-bot · 2023-01-05T01:34:06Z

[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-01-05T01:34:13Z

Hi @HuSharp. Thanks for your PR.

I'm waiting for a tikv member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

client/resourcemanager_client.go

codecov · 2023-01-12T11:03:20Z

Codecov Report

Base: 75.42% // Head: 75.46% // Increases project coverage by +0.03% 🎉

Coverage data is based on head (639121e) compared to base (7ca1b9a).
Patch coverage: 66.66% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5830      +/-   ##
==========================================
+ Coverage   75.42%   75.46%   +0.03%     
==========================================
  Files         341      341              
  Lines       34336    34404      +68     
==========================================
+ Hits        25897    25962      +65     
+ Misses       6209     6196      -13     
- Partials     2230     2246      +16

Flag	Coverage Δ
unittests	`75.46% <66.66%> (+0.03%)`	⬆️

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

Impacted Files	Coverage Δ
server/grpc_service.go	`49.25% <53.84%> (+1.69%)`	⬆️
client/resourcemanager_client.go	`65.83% <68.96%> (+0.16%)`	⬆️
client/client.go	`70.89% <72.97%> (+0.34%)`	⬆️
server/server.go	`75.03% <100.00%> (+0.03%)`	⬆️
server/api/diagnostic.go	`76.92% <0.00%> (-23.08%)`	⬇️
pkg/storage/endpoint/rule.go	`83.33% <0.00%> (-11.12%)`	⬇️
...erver/config/service_middleware_persist_options.go	`91.66% <0.00%> (-8.34%)`	⬇️
pkg/dashboard/adapter/manager.go	`74.71% <0.00%> (-6.90%)`	⬇️
server/cluster/diagnostic_manager.go	`72.04% <0.00%> (-4.31%)`	⬇️
pkg/storage/kv/etcd_kv.go	`72.22% <0.00%> (-3.18%)`	⬇️
... and 27 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Signed-off-by: husharp <jinhao.hu@pingcap.com>

HuSharp · 2023-01-18T03:32:09Z

Is lower code coverage a blocker issue ?

This does not block merge

glorv · 2023-01-18T06:16:25Z

client/client.go

-	// TODO: complete this function with new implementation.
-	return nil, nil
+	// register watch components
+	globalConfigWatcherCh := make(chan []GlobalConfigItem, 16)


Do we need to return the max revision of each []GlobalConfigItem so the caller can do incremental retry?

since resource client is just for testing, I will add a retry mechanism after I solve pd client/tidb/CDC compatibility...

server/grpc_service.go

Signed-off-by: husharp <jinhao.hu@pingcap.com>

client/client.go

server/grpc_service.go

client/resourcemanager_client.go

Signed-off-by: husharp <jinhao.hu@pingcap.com>

HuSharp · 2023-01-19T03:55:05Z

@glorv @nolouch @BornChanger @CabinfeverB pls take a look, thx!

Signed-off-by: husharp <jinhao.hu@pingcap.com>

glorv

LGTM

client/client.go

ti-chi-bot · 2023-01-19T04:19:23Z

@glorv: 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:

LGTM

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: husharp <jinhao.hu@pingcap.com>

nolouch · 2023-01-19T07:46:26Z

/merge

ti-chi-bot · 2023-01-19T07:46:28Z

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

/run-all-tests

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-01-19T07:46:30Z

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

Commit hash: 4bd95b5

HuSharp · 2023-01-19T08:08:58Z

/run-all-tests

nolouch · 2023-01-19T08:54:44Z

/rebuild

ti-chi-bot · 2023-01-19T08:56:00Z

@HuSharp: 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.

purelind · 2023-01-19T09:40:32Z

/test build

ti-chi-bot requested review from HunDunDM and rleungx January 5, 2023 01:34

HuSharp force-pushed the support_resource_group_watch branch from 3fc7577 to 4366236 Compare January 5, 2023 01:45

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 6, 2023

HuSharp force-pushed the support_resource_group_watch branch 5 times, most recently from fb9a3db to 23c074a Compare January 10, 2023 10:12

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 10, 2023

HuSharp force-pushed the support_resource_group_watch branch from 003db29 to 51f33da Compare January 11, 2023 02:52

CabinfeverB mentioned this pull request Jan 11, 2023

Tracking issue for resource manager #5851

Closed

7 tasks

HuSharp force-pushed the support_resource_group_watch branch from 51f33da to 4b06244 Compare January 12, 2023 02:59

CabinfeverB self-requested a review January 12, 2023 03:03

CabinfeverB reviewed Jan 12, 2023

View reviewed changes

client/resourcemanager_client.go Show resolved Hide resolved

CabinfeverB requested review from nolouch and removed request for HunDunDM and rleungx January 12, 2023 07:10

HuSharp force-pushed the support_resource_group_watch branch 2 times, most recently from 9cfe759 to 492c62c Compare January 12, 2023 10:44

HuSharp marked this pull request as ready for review January 12, 2023 10:59

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

rleungx mentioned this pull request Jan 13, 2023

*: upgrade kvproto to the latest master #5861

Merged

support etcd and resource group

3d5218d

Signed-off-by: husharp <jinhao.hu@pingcap.com>

glorv reviewed Jan 18, 2023

View reviewed changes

HuSharp added 2 commits January 18, 2023 16:35

Merge branch 'master' into support_resource_group_watch

f27d706

update watch

3d2255d

Signed-off-by: husharp <jinhao.hu@pingcap.com>

glorv reviewed Jan 19, 2023

View reviewed changes

client/client.go Outdated Show resolved Hide resolved

server/grpc_service.go Outdated Show resolved Hide resolved

server/grpc_service.go Outdated Show resolved Hide resolved

client/resourcemanager_client.go Show resolved Hide resolved

client/resourcemanager_client.go Show resolved Hide resolved

update kvproto and address comment

f7c1912

Signed-off-by: husharp <jinhao.hu@pingcap.com>

make format happy

eca0d95

Signed-off-by: husharp <jinhao.hu@pingcap.com>

glorv approved these changes Jan 19, 2023

View reviewed changes

client/client.go Outdated Show resolved Hide resolved

add some comment

4bd95b5

Signed-off-by: husharp <jinhao.hu@pingcap.com>

nolouch approved these changes Jan 19, 2023

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Jan 19, 2023

CabinfeverB approved these changes Jan 19, 2023

View reviewed changes

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

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

Merge branch 'master' into support_resource_group_watch

639121e

ti-chi-bot merged commit c5bf5be into tikv:master Jan 19, 2023

HuSharp deleted the support_resource_group_watch branch January 19, 2023 09:50

HuSharp restored the support_resource_group_watch branch January 19, 2023 09:50

HuSharp deleted the support_resource_group_watch branch January 19, 2023 09:54

overvenus mentioned this pull request Jan 19, 2023

Test fails due to get source id error "strconv.ParseUint: parsing \"\\x00\\x00\\x00\\x00\\x00\\x00\\x03\\xe8\": invalid syntax"" pingcap/tiflow#8110

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support resource group watch #5830

Support resource group watch #5830

HuSharp commented Jan 5, 2023 •

edited

Loading

ti-chi-bot commented Jan 5, 2023 •

edited

Loading

ti-chi-bot commented Jan 5, 2023

codecov bot commented Jan 12, 2023 •

edited

Loading

HuSharp commented Jan 18, 2023

glorv Jan 18, 2023

HuSharp Jan 19, 2023

HuSharp commented Jan 19, 2023

glorv left a comment

ti-chi-bot commented Jan 19, 2023

nolouch commented Jan 19, 2023

ti-chi-bot commented Jan 19, 2023

ti-chi-bot commented Jan 19, 2023

HuSharp commented Jan 19, 2023

nolouch commented Jan 19, 2023

ti-chi-bot commented Jan 19, 2023

purelind commented Jan 19, 2023

Support resource group watch #5830

Support resource group watch #5830

Conversation

HuSharp commented Jan 5, 2023 • edited Loading

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

ti-chi-bot commented Jan 5, 2023 • edited Loading

ti-chi-bot commented Jan 5, 2023

codecov bot commented Jan 12, 2023 • edited Loading

Codecov Report

HuSharp commented Jan 18, 2023

glorv Jan 18, 2023

Choose a reason for hiding this comment

HuSharp Jan 19, 2023

Choose a reason for hiding this comment

HuSharp commented Jan 19, 2023

glorv left a comment

Choose a reason for hiding this comment

ti-chi-bot commented Jan 19, 2023

nolouch commented Jan 19, 2023

ti-chi-bot commented Jan 19, 2023

ti-chi-bot commented Jan 19, 2023

HuSharp commented Jan 19, 2023

nolouch commented Jan 19, 2023

ti-chi-bot commented Jan 19, 2023

purelind commented Jan 19, 2023

HuSharp commented Jan 5, 2023 •

edited

Loading

ti-chi-bot commented Jan 5, 2023 •

edited

Loading

codecov bot commented Jan 12, 2023 •

edited

Loading