May receive multiple rows in streaming mode

[mapleFU]

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. What did you do?
   If possible, provide a recipe for reproducing the error.

When tidb open streaming mode in pr10949 , we may receive multiple rows in TiDB.

When we prepare enviroment:

CREATE TABLE t1 (i INT);
INSERT INTO t1 VALUES (10),(11),(12),(13),(14),(15),(16),(17),(18),(19),
                      (20),(21),(22),(23),(24),(25),(26),(27),(28),(29),
                      (30),(31),(32),(33),(34),(35);

And run the SQL below for multiple times:

CREATE TABLE t2 (a CHAR(2), b CHAR(2), c CHAR(2), INDEX (a, b));
INSERT INTO t2 (a, b) SELECT i, i FROM t1; # 插入一把记录
INSERT INTO t2 (a, b) SELECT t1.i, t1.i FROM t1, t1 x1, t1 x2;

UPDATE t2 SET c = 10 ORDER BY a DESC, b DESC LIMIT 5;
# SHOW SESSION STATUS LIKE 'Sort%';
# SHOW STATUS LIKE 'Handler_read_%';
SELECT * FROM t2 WHERE c = 10 ORDER BY a DESC, b DESC;

DROP TABLE t2;

SELECT * FROM t2 WHERE c = 10 ORDER BY a DESC, b DESC; May return the response below:

mysql> SELECT * FROM t2 WHERE c = 10 ORDER BY a DESC, b DESC;
+------+------+------+
| a    | b    | c    |
+------+------+------+
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
+------+------+------+
20 rows in set (2.37 sec)

Here it returns multiple times of rows(expect 5, may get 10, 15 or more).

2. What did you expect to see?

mysql> SELECT * FROM t2 WHERE c = 10 ORDER BY a DESC, b DESC;
+------+------+------+
| a    | b    | c    |
+------+------+------+
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
+------+------+------+
5 rows in set (1.67 sec)

3. What did you see instead?

mysql> SELECT * FROM t2 WHERE c = 10 ORDER BY a DESC, b DESC;
+------+------+------+
| a    | b    | c    |
+------+------+------+
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
| 35   | 35   | 10   |
+------+------+------+
20 rows in set (2.37 sec)

4. What version of TiDB are you using (tidb-server -V or run select tidb_version(); on TiDB)?

In pr 11949. @SunRunAway

[SunRunAway]

It reproduced in https://internal.pingcap.net/idc-jenkins/blue/organizations/jenkins/stability_integration_common_test/detail/stability_integration_common_test/10/pipeline/1371/

[SunRunAway]

After some investigation, if a streaming response returning by TiKV meeting a lock error, it does not contain the range entry, so that TiDB creates a new request with a range from original.

We have two options to fix,

1. TiKV returns a response containing the range entry when meeting a lock error (and maybe other errors like region error).
2. When a lock error happens, TiDB uses the last range to build a new request. (but I think it's just a workaround.)

@mapleFU @breeswish PTAL.

[mapleFU]

@SunRunAway We Intend to choose method1 as the solving method. Method 2 needs to change the streaming request logic in TiDB. And we think method 1 is better.

[SunRunAway]

Great, and I'll add a strict check to make sure each streaming response does contain a range entry.

[breezewish]

@SunRunAway Note that the last success streaming response may not contain a range (since it is useless)

[SunRunAway]

Could this be possible to have one?

[mapleFU]

@SunRunAway We can attach one, but I wonder why it's necessary. And it may reduce the performance.

[mapleFU]

@breeswish In executor/runner:

        if record_cnt > 0 {
            let range = self.executor.take_scanned_range();
            return self
                .make_stream_response(chunk)
                .map(|r| (Some((r, range)), finished));
        }
        Ok((None, true))

Seems that if there is any data, we will return the range here. If there is no data left, we just return EOF.

[breezewish]

@mapleFU You can have a try. I may have an incorrect memory.

[mapleFU]

In TiKV, not all requests can be attached with range. If no range returned. Please use last received range and timestamp to send a request. If thats the first request, please send with the time and range again. @SunRunAway @breeswish

[mapleFU]

Bravo