Skip to content

Conversation

@birdstorm
Copy link
Collaborator

@birdstorm birdstorm commented Nov 20, 2021

When the Java Client requests a store id that PD does not recognize, e.g., the PD was scaled-in and does not know about a scaled-out TiKV, the PD will respond with a StatusRuntimeException rather than an error wrapped by Response itself.

Thus we should resolve StatusRuntimeException so that the invalid store info can be dealt with correctly by clearing all-region/store cache info.

Signed-off-by: birdstorm <samuelwyf@hotmail.com>
Signed-off-by: birdstorm <samuelwyf@hotmail.com>
Signed-off-by: birdstorm <samuelwyf@hotmail.com>
@Override
public boolean handleRequestError(BackOffer backOffer, Exception e) {
// store id is not found
if (e instanceof StatusRuntimeException && e.getMessage().contains("invalid store ID")) {
Copy link
Member

Choose a reason for hiding this comment

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

can we directly check whether the instance is of InvalidStoreException?

Copy link
Member

Choose a reason for hiding this comment

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

BTW, should the error message be Invalid storeId?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no, the exception tikv returns is a grpc exception, it is a StatusRuntimeException that only contains "invalid store id xx" information in its message.

public TiStore getStoreById(long id, BackOffer backOffer) {
TiStore store = getStoreByIdWithBackOff(id, backOffer);
if (store == null) {
cache.clearAll();
Copy link
Member

Choose a reason for hiding this comment

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

we should log info or warn messages for this critical operation.

Signed-off-by: birdstorm <samuelwyf@hotmail.com>
zz-jason
zz-jason previously approved these changes Nov 21, 2021
Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

LGTM

marsishandsome
marsishandsome previously approved these changes Nov 21, 2021
Copy link
Collaborator

@marsishandsome marsishandsome left a comment

Choose a reason for hiding this comment

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

LGTM

ti-srebot
ti-srebot previously approved these changes Nov 21, 2021
@marsishandsome
Copy link
Collaborator

/merge

@ti-srebot
Copy link
Collaborator

/run-all-tests

@ti-srebot
Copy link
Collaborator

@birdstorm merge failed.

@marsishandsome
Copy link
Collaborator

/merge

@ti-srebot
Copy link
Collaborator

/run-all-tests

@ti-srebot
Copy link
Collaborator

@birdstorm merge failed.

@marsishandsome
Copy link
Collaborator

/run-all-tests

Signed-off-by: marsishandsome <marsishandsome@gmail.com>
@marsishandsome marsishandsome dismissed stale reviews from ti-srebot, zz-jason, and themself via fe9c73f November 21, 2021 15:57
Copy link
Collaborator

@marsishandsome marsishandsome left a comment

Choose a reason for hiding this comment

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

LGTM

@marsishandsome
Copy link
Collaborator

/merge

@ti-srebot
Copy link
Collaborator

Your auto merge job has been accepted, waiting for:

  • 335

@ti-srebot
Copy link
Collaborator

/run-all-tests

@marsishandsome marsishandsome merged commit 5bbe72e into tikv:release-3.1 Nov 21, 2021
marsishandsome pushed a commit to marsishandsome/client-java that referenced this pull request Dec 1, 2021
Signed-off-by: marsishandsome <marsishandsome@gmail.com>
marsishandsome pushed a commit to marsishandsome/client-java that referenced this pull request Dec 1, 2021
Signed-off-by: marsishandsome <marsishandsome@gmail.com>
@birdstorm birdstorm deleted the fix-pd-invalid-store-id branch December 16, 2021 05:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants