Skip to content
This repository has been archived by the owner on Dec 8, 2021. It is now read-only.

backend/local: add encode for retry scan region keys #531

Merged
merged 1 commit into from
Dec 23, 2020
Merged

Conversation

glorv
Copy link
Contributor

@glorv glorv commented Dec 23, 2020

What problem does this PR solve?

Fix the bug that the retry split key is not encoded, which causes scanned regions are incorrect.

What is changed and how it works?

  • Fix the retry split scan keys bug
  • Add more error checks and logs
  • Add some unit tests

Check List

Tests

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

Side effects

Related changes

Release Note

  • Fix the bug that the retry scan region key is not encoded, which causes the retryed region is not split.

@glorv glorv added the type/bug-fix Bug fix label Dec 23, 2020
@glorv glorv force-pushed the fix-retry-split branch 3 times, most recently from 9920914 to dff08f3 Compare December 23, 2020 03:50
regionMap := make(map[uint64]*split.RegionInfo)
for _, region := range regions {
regionMap[region.Region.GetId()] = region
}

var splitKeyMap map[uint64][][]byte
if len(retryKeys) > 0 {
firstKeyEnc := codec.EncodeBytes([]byte{}, retryKeys[0])
lastKeyEnc := codec.EncodeBytes([]byte{}, retryKeys[len(retryKeys)-1])
if bytes.Compare(firstKeyEnc, regions[0].Region.StartKey) < 0 || !beforeEnd(lastKeyEnc, regions[len(regions)-1].Region.EndKey) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why compare firstKey and regions[0]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fistKey is the smallest key, so it should be included in region[0]; and lastKey in region[-1]

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems that region.StartKey without encode can not compare to other encoded key directly?

Copy link
Contributor

Choose a reason for hiding this comment

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

@glorv
Copy link
Contributor Author

glorv commented Dec 23, 2020

/run-all-tests

Signed-off-by: glorv <glorvs@163.com>
@glorv
Copy link
Contributor Author

glorv commented Dec 23, 2020

/run-all-tests

Copy link
Contributor

@Little-Wallace Little-Wallace 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
Copy link
Contributor

@Little-Wallace, Thanks for your review. The bot only counts LGTMs from Reviewers and higher roles, but you're still welcome to leave your comments. See the corresponding SIG page for more information. Related SIG: migrate(slack).

Copy link
Contributor

@3pointer 3pointer 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 added the status/LGT1 One reviewer already commented LGTM (LGTM1) label Dec 23, 2020
@glorv glorv merged commit c3fc041 into master Dec 23, 2020
@glorv glorv deleted the fix-retry-split branch December 23, 2020 06:27
glorv added a commit that referenced this pull request Jan 22, 2021
Signed-off-by: glorv <glorvs@163.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status/LGT1 One reviewer already commented LGTM (LGTM1) type/bug-fix Bug fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants