Implement reverse iterator

[arthurkiller]

What problem does this PR solve?

Request IterReverse for long.

See also #7429

What is changed and how it works?

IIRC we asking for reverse iterator since last summer, and the job was block on the dependency last year (#7429).

Finally I implemented reverseIterator for tikv SDK by add a new member for tikv.Scanner.

• getRegion on reverse will return the key from start to end in descend order
• on region border, set nextStartKey to the StartKey.Next(), cause the region will not contains StartKey.
• Once StartKey is blank, we met the very first region stop the iterator.

Check List

Tests

• Manual test (add detailed scripts or steps below)

1. write keys into tikv
2. split the region
3. find any region start key, and reverseIter it
4. check if things going right

Side effects
rely on these functions already implemented

• regionCache.LocateEndKey
• pb.ScanRequest.Reverse = true

and changed this internal function

func newScanner(snapshot *tikvSnapshot, startKey []byte, endKey []byte, batchSize int)
func newScanner(snapshot *tikvSnapshot, startKey []byte, endKey []byte, batchSize int, reverse bool)

Related changes

• Need to update the documentation
• Need to be included in the release note

[shenli]

@arthurkiller Manual test is not enough. Could you add some unit tests?

[arthurkiller]

@shenli thx for reply and I will fix the unit test then. But I'm not sure if tikv mock is easy to implement.

[shenli]

@arthurkiller Thanks for your contribution! If you need any help, please let us know. :)

[arthurkiller]

API did not add end key limitation yet. Should I add this? Have an awesome day ✉️✉️✉️✉️✉️✉️✉️✉️✉️✉️✉️✉️ Arthur lee Sent from my iPhone

…

On 6 Mar 2019, at 10:08 AM, disksing ***@***.***> wrote: @disksing commented on this pull request. In store/tikv/scan.go: > @@ -172,6 +176,11 @@ func (s *Scanner) getData(bo *Backoffer) error { NotFillCache: s.snapshot.notFillCache, }, } + // Reverse seek will not limit endKey. Reverse scan stops at end_key too. See the description here: https://github.com/pingcap/kvproto/blob/ab7debc182d9c536775b3adde7e35716f10bc0b6/proto/kvrpcpb.proto#L106-L107 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[disksing]

@arthurkiller I think it's ok to update kv.IterReverse later. But as TiKV already supports it, it is better to support in the scanner.

[arthurkiller]

@shenli Hey, I have added unit test for scanner scan reverse.
@disksing and endKey support for scan reverse now.

[shenli]

@arthurkiller Cool!

[arthurkiller]

----------------------------------------------------------------------
FAIL: scan_mock_test.go:29: testScanMockSuite.TestScanMultipleRegions

scan_mock_test.go:56:
    c.Assert([]byte{ch}, BytesEquals, []byte(scanner.Key()))
... bytes_one []uint8 = []byte{0x7a}
... bytes_two []uint8 = []byte(nil)

PASS: scan_test.go:66: testScanSuite.TestScan   0.064s

----------------------------------------------------------------------
FAIL: scan_test.go:141: testScanSuite.TestScanReverse

scan_test.go:187:
    check(c, scan, rowNum, false)
scan_test.go:145:
    c.Assert([]byte(k), BytesEquals, encodeKey(s.prefix, s08d("key", i)))
... bytes_one []uint8 = []byte{0x73, 0x65, 0x65, 0x6b, 0x5f, 0x31, 0x35, 0x35, 0xff, 0x31, 0x38, 0x37, 0x34, 0x35, 0x35, 0x31, 0x5f, 0xff, 0x6b, 0x65, 0x79, 0x30, 0x30, 0x30, 0x30, 0x30, 0xff, 0x30, 0x30, 0x30, 0x0, 0x0, 0x0, 0x0, 0x0, 0xfa}
... bytes_two []uint8 = []byte{0x73, 0x65, 0x65, 0x6b, 0x5f, 0x31, 0x35, 0x35, 0xff, 0x31, 0x38, 0x37, 0x34, 0x35, 0x35, 0x31, 0x5f, 0xff, 0x6b, 0x65, 0x79, 0x30, 0x30, 0x30, 0x30, 0x30, 0xff, 0x30, 0x30, 0x31, 0x0, 0x0, 0x0, 0x0, 0x0, 0xfa}

seems SDK did not mock the reverse yet

[arthurkiller]

anyway I have added the test case in this pr

@shenli have you got any plan to add this mock?

[disksing]

Seems the tests are failing. @arthurkiller

[arthurkiller]

----------------------------------------------------------------------
FAIL: scan_mock_test.go:29: testScanMockSuite.TestScanMultipleRegions

scan_mock_test.go:56:
    c.Assert([]byte{ch}, BytesEquals, []byte(scanner.Key()))
... bytes_one []uint8 = []byte{0x7a}
... bytes_two []uint8 = []byte(nil)

PASS: scan_test.go:66: testScanSuite.TestScan   0.064s

----------------------------------------------------------------------
FAIL: scan_test.go:141: testScanSuite.TestScanReverse

scan_test.go:187:
    check(c, scan, rowNum, false)
scan_test.go:145:
    c.Assert([]byte(k), BytesEquals, encodeKey(s.prefix, s08d("key", i)))
... bytes_one []uint8 = []byte{0x73, 0x65, 0x65, 0x6b, 0x5f, 0x31, 0x35, 0x35, 0xff, 0x31, 0x38, 0x37, 0x34, 0x35, 0x35, 0x31, 0x5f, 0xff, 0x6b, 0x65, 0x79, 0x30, 0x30, 0x30, 0x30, 0x30, 0xff, 0x30, 0x30, 0x30, 0x0, 0x0, 0x0, 0x0, 0x0, 0xfa}
... bytes_two []uint8 = []byte{0x73, 0x65, 0x65, 0x6b, 0x5f, 0x31, 0x35, 0x35, 0xff, 0x31, 0x38, 0x37, 0x34, 0x35, 0x35, 0x31, 0x5f, 0xff, 0x6b, 0x65, 0x79, 0x30, 0x30, 0x30, 0x30, 0x30, 0xff, 0x30, 0x30, 0x31, 0x0, 0x0, 0x0, 0x0, 0x0, 0xfa}

seems SDK did not mock the reverse yet @disksing

[zz-jason]

@arthurkiller Could update the master branch and resolve conflicts?

[sre-bot]

Hi contributor, thanks for your PR.

This patch needs to be approved by someone of admins. They should reply with "/ok-to-test" to accept this PR for running test automatically.

[zz-jason]

please follow CONTRIBUTING.md to refine the PR title.

[arthurkiller]

@zz-jason okay, I will finish this later this week

[arthurkiller]

@zz-jason @disksing

Do you guys have already implemented reverse seek? I found the code has been added at 2019-05-31 23:40:09 +0800

crazycs           2019-05-31 23:40:09 +0800)|  216         if !s.reverse {

[arthurkiller]

@crazycs520 Seems you have added the IterReverse already, is that right?

[arthurkiller]

@disksing @shenli @zz-jason ping

[crazycs520]

@arthurkiller, Sorry for that, I don't know this pr before, I write PR #10152 to speed up admin show ddl jobs and implement the reverse scan.

[arthurkiller]

nvm, so we can use IterReverse now, and this pr can be closed

CC @shafreeck

[crazycs520]

@arthurkiller, Thanks for the contribution.