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

raftstore/error: add StoreNotMatch details #3885

Merged
merged 4 commits into from Dec 6, 2018

Conversation

Projects
None yet
4 participants
@BusyJay
Copy link
Contributor

commented Dec 5, 2018

It helps debugging implementation errors.

Should not merge this before pingcap/kvproto#333.

raftstore/error: add StoreNotMatch details
Signed-off-by: Jay Lee <busyjaylee@gmail.com>

@BusyJay BusyJay added the C: Raft label Dec 5, 2018

@BusyJay BusyJay requested a review from overvenus Dec 5, 2018

@BusyJay BusyJay added the S: DNM label Dec 5, 2018

@BusyJay BusyJay requested a review from disksing Dec 5, 2018

Error::StoreNotMatch(to_store_id, my_store_id) => {
errorpb
.mut_store_not_match()
.set_expect_store_id(to_store_id);

This comment has been minimized.

Copy link
@disksing

disksing Dec 5, 2018

Collaborator

Should expect_store_id be my_store_id?

This comment has been minimized.

Copy link
@BusyJay

BusyJay Dec 5, 2018

Author Contributor

How about requested_store_id and actual_store_id?

This comment has been minimized.

Copy link
@BusyJay

BusyJay Dec 5, 2018

Author Contributor

Or expect_store_id and actual_store_id?

This comment has been minimized.

Copy link
@ngaut

ngaut Dec 5, 2018

Member

expect_store_id and actual_store_id +1

This comment has been minimized.

Copy link
@disksing

disksing Dec 6, 2018

Collaborator

I think request_store_id is better. By expect, I cannot know it is the client or the server that is expecting.

address comment
Signed-off-by: Jay Lee <busyjaylee@gmail.com>
@overvenus
Copy link
Member

left a comment

LGTM

update kvproto
Signed-off-by: Jay Lee <busyjaylee@gmail.com>

@BusyJay BusyJay dismissed stale reviews from disksing and overvenus via 4db23fb Dec 6, 2018

@BusyJay

This comment has been minimized.

Copy link
Contributor Author

commented Dec 6, 2018

/run-integration-tests

@BusyJay BusyJay removed the S: DNM label Dec 6, 2018

@BusyJay BusyJay merged commit d6a1def into tikv:master Dec 6, 2018

6 checks passed

DCO All commits are signed off!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
jenkins-ci-tikv/build Jenkins job succeeded.
Details
jenkins-ci-tikv/integration-common-test Jenkins job succeeded.
Details
jenkins-ci-tikv/integration-compatibility-test Jenkins job succeeded.
Details
jenkins-ci-tikv/integration-ddl-test Jenkins job succeeded.
Details

@BusyJay BusyJay deleted the BusyJay:fill-store-not-matched branch Dec 6, 2018

BusyJay added a commit to BusyJay/tikv that referenced this pull request Feb 27, 2019

raftstore/error: add StoreNotMatch details (tikv#3885)
Signed-off-by: Jay Lee <busyjaylee@gmail.com>

overvenus added a commit to overvenus/tikv that referenced this pull request Feb 27, 2019

raftstore/error: add StoreNotMatch details (tikv#3885)
Signed-off-by: Jay Lee <busyjaylee@gmail.com>

overvenus added a commit to overvenus/tikv that referenced this pull request Feb 27, 2019

raftstore/error: add StoreNotMatch details (tikv#3885)
Signed-off-by: Jay Lee <busyjay@users.noreply.github.com>

overvenus added a commit to overvenus/tikv that referenced this pull request Feb 27, 2019

raftstore/error: add StoreNotMatch details (tikv#3885)
Signed-off-by: Jay <busyjay@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.