raftstore: Implement coprocessor observer post_exec_admin(query) by CalvinNeo · Pull Request #12850 · tikv/tikv

CalvinNeo · 2022-06-20T04:01:10Z

What is changed and how it works?

Issue Number: ref #12849

What's Changed:

According to #12849, we need to support some new observers, including post_exec_admin(query) which is implemented in this PR.

Support new observers post_exec_admin(query).

Related changes

PR to update pingcap/docs/pingcap/docs-cn:
Need to cherry-pick to the release branch

Check List

Tests

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

Side effects

Performance regression
- Consumes more CPU
- Consumes more MEM
Breaking backward compatibility

Release note

None

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

ti-chi-bot · 2022-06-20T04:01:12Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

BusyJay
tonyxuqqi

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.

Details

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

CalvinNeo · 2022-06-20T04:05:09Z

/run-all-tests

CalvinNeo · 2022-06-20T04:15:17Z

components/raftstore/src/store/fsm/apply.rs


+        if should_write {
+            info!("persist data and apply state"; "region_id" => self.region_id(), "peer_id" => self.id(), "state" => ?self.apply_state);
+            ctx.commit(self);


Should we move this into an outer function?

Yes, otherwise the response won't be returned for the last command.

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

CalvinNeo · 2022-06-20T08:39:38Z

/cc @BusyJay @tonyxuqqi

components/raftstore/src/store/fsm/apply.rs

tonyxuqqi

Add some test for changes in apply.rs

CalvinNeo · 2022-06-29T02:45:00Z

Add some test for changes in apply.rs

Will do that after pre_exec_query(admin) are merged in test_exec_observer.

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

components/cdc/src/delegate.rs

components/raftstore/src/coprocessor/dispatcher.rs

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

…nto merge-tikv-1

BusyJay

I think we need to bench the performance impact after adding two observers. sysbench should be enough.

BusyJay · 2022-07-14T01:14:32Z

components/raftstore/src/store/fsm/apply.rs


+        if should_write {
+            info!("persist data and apply state"; "region_id" => self.region_id(), "peer_id" => self.id(), "state" => ?self.apply_state);
+            ctx.commit(self);


Yes, otherwise the response won't be returned for the last command.

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

CalvinNeo · 2022-07-14T10:21:30Z

/build

CalvinNeo · 2022-07-14T10:22:32Z

/run-build-arm64 comment=true

sre-bot · 2022-07-14T11:10:48Z

download tikv binary at http://fileserver.pingcap.net/download/builds/pingcap/test/tikv/d8a63233dbe11e4c2d9a92366c1bf5fe90a6fa7a/centos7/tikv-linux-arm64.tar.gz

CalvinNeo · 2022-07-16T03:10:04Z

I think we need to bench the performance impact after adding two observers. sysbench should be enough.
It is tested that the performance is quite close to each other.

breezewish · 2022-07-18T02:19:51Z

/merge

ti-chi-bot · 2022-07-18T02:19:52Z

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

Details

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 · 2022-07-18T02:19:54Z

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

Details

Commit hash: d8a6323

…v#12850) ref tikv#12849 Support new observers post_exec_admin(query). Signed-off-by: CalvinNeo <calvinneo1995@gmail.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>

sre-bot · 2022-08-05T08:22:45Z

download tikv binary(linux arm64) at http://fileserver.pingcap.net/download/builds/pingcap/test/tikv/d8a63233dbe11e4c2d9a92366c1bf5fe90a6fa7a/centos7/tikv-linux-arm64.tar.gz

CalvinNeo added 3 commits June 20, 2022 10:34

add observer

ce20f96

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

fix

c7e553e

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

fmt

ef452ed

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

CalvinNeo mentioned this pull request Jun 20, 2022

Support TiKV using different KvEngines #12849

Closed

CalvinNeo commented Jun 20, 2022

View reviewed changes

CalvinNeo added 3 commits June 20, 2022 13:15

fix test

14b3896

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

fix

df471bd

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

fix

a8adbfe

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

ti-chi-bot requested review from BusyJay and tonyxuqqi June 20, 2022 08:39

Merge remote-tracking branch 'upstream/master' into merge-tikv-1

117bc8a

tonyxuqqi reviewed Jun 28, 2022

View reviewed changes

components/raftstore/src/store/fsm/apply.rs Outdated Show resolved Hide resolved

tonyxuqqi reviewed Jun 28, 2022

View reviewed changes

CalvinNeo added 2 commits June 29, 2022 11:12

address comments

c39fb0b

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

add ut tests

b0d034e

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

CalvinNeo marked this pull request as ready for review June 30, 2022 02:14

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 Jun 30, 2022

CalvinNeo added 2 commits June 30, 2022 10:35

format

a3d1068

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

fix

dc5674e

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

tonyxuqqi approved these changes Jul 6, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 6, 2022

BusyJay reviewed Jul 8, 2022

View reviewed changes

components/cdc/src/delegate.rs Show resolved Hide resolved

components/raftstore/src/coprocessor/dispatcher.rs Show resolved Hide resolved

CalvinNeo requested a review from BusyJay July 12, 2022 03:35

CalvinNeo and others added 6 commits July 12, 2022 13:26

add comments

cd26ead

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

Merge remote-tracking branch 'upstream/master' into merge-tikv-1

454ab66

fix

ea90abf

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

Merge branch 'master' into merge-tikv-1

0f43963

f

1027568

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

Merge branch 'merge-tikv-1' of github.com:CalvinNeo/tidb-engine-ext i…

34769ed

…nto merge-tikv-1

BusyJay reviewed Jul 14, 2022

View reviewed changes

CalvinNeo added 2 commits July 14, 2022 11:39

f

251c354

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

panic

d8a6323

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

CalvinNeo requested a review from BusyJay July 16, 2022 03:10

BusyJay approved these changes Jul 16, 2022

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 Jul 16, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jul 18, 2022

Merge branch 'master' into merge-tikv-1

d25b847

ti-chi-bot merged commit 21f00d2 into tikv:master Jul 18, 2022

ti-chi-bot added this to the Pool milestone Jul 18, 2022

Conversation

CalvinNeo commented Jun 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is changed and how it works?

Related changes

Check List

Release note

Uh oh!

ti-chi-bot commented Jun 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CalvinNeo commented Jun 20, 2022

Uh oh!

CalvinNeo Jun 20, 2022

Choose a reason for hiding this comment

Uh oh!

BusyJay Jul 14, 2022

Choose a reason for hiding this comment

Uh oh!

CalvinNeo commented Jun 20, 2022

Uh oh!

Uh oh!

tonyxuqqi left a comment

Choose a reason for hiding this comment

Uh oh!

CalvinNeo commented Jun 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

BusyJay left a comment

Choose a reason for hiding this comment

Uh oh!

BusyJay Jul 14, 2022

Choose a reason for hiding this comment

Uh oh!

CalvinNeo commented Jul 14, 2022

Uh oh!

CalvinNeo commented Jul 14, 2022

Uh oh!

sre-bot commented Jul 14, 2022

Uh oh!

CalvinNeo commented Jul 16, 2022

Uh oh!

breezewish commented Jul 18, 2022

Uh oh!

ti-chi-bot commented Jul 18, 2022

Uh oh!

ti-chi-bot commented Jul 18, 2022

Uh oh!

sre-bot commented Aug 5, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

CalvinNeo commented Jun 20, 2022 •

edited

Loading

ti-chi-bot commented Jun 20, 2022 •

edited

Loading

CalvinNeo commented Jun 29, 2022 •

edited

Loading