BatchScanRegions need to return an error to tell client regions exist hole for retry

[okJiang]

Enhancement Task

ref

• region cache: check if the pd returned regions covers the ranges client-go#1377
• https://github.com/pingcap/tidb/blob/1480bb127a9363e31e8b2e9f3808289ea54d9d28/br/pkg/streamhelper/regioniter.go#L103
• https://github.com/pingcap/tidb/blob/1480bb127a9363e31e8b2e9f3808289ea54d9d28/br/pkg/restore/split/split.go#L40

[Tristan1900]

@okJiang Looks like after the proto change, TiKV code pd_client/src/util#check_resp_header breaks because of not handling the added error type. Curious if there is a fix already opened or in progress? Or who should be good to contact to fix it? Thanks!

[okJiang]

@okJiang Looks like after the proto change, TiKV code pd_client/src/util#check_resp_header breaks because of not handling the added error type. Curious if there is a fix already opened or in progress? Or who should be good to contact to fix it? Thanks!

If you don't set the added parameter(contain_all_key_range), it will not return the added error type(REGIONS_NOT_CONTAIN_ALL_KEY_RANGE). So can you explain which problem do you meet?

[Tristan1900]

@okJiang hmm, from my side it's a compiler error that check_resp_header did not exhaustively handle all the enum type. BTW I did not make any changes on pd_client, I'm working on the backup side

maybe I somehow use it wrong? I added a panic to bypass the complier to not blocking me but I don't think that's the correct way to do so asking for help.

[okJiang]

@okJiang hmm, from my side it's a compiler error that check_resp_header did not exhaustively handle all the enum type. BTW I did not make any changes on pd_client, I'm working on the backup side

maybe I somehow use it wrong? I added a panic to bypass the complier to not blocking me but I don't think that's the correct way to do so asking for help.

I am sorry that I am not familiar with rust... @YuJuncen Can you help to reply it?😂

[YuJuncen]

@Tristan1900 You may just append _ => unreachable!() or _ => unimplemented!() to the match block. TBH I guess we'd better add a #[non_exhaustive] to the protocol buffer enums...

[Tristan1900]

@YuJuncen ok, I'm not sure how should we behave on TiKV side in response of this error so didn't add anything on my own. Sounds like this error will never happen on TiKV side? If we are sure about that I can add unreachable thing.
I feel like having complier error is better in case people forgot to add the handling after changing the proto, but yeah it's cubersome.

[3AceShowHand]

I also meet this issue, any update ?