*: stale reads compatible with prepare #25156
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -772,3 +772,80 @@ func (s *testStaleTxnSuite) TestSetTransactionInfoSchema(c *C) { | |
| tk.MustExec("commit") | ||
| c.Assert(tk.Se.GetInfoSchema().SchemaMetaVersion(), Equals, schemaVer3) | ||
| } | ||
|
|
||
| func (s *testStaleTxnSuite) TestStaleSelect(c *C) { | ||
| tk := testkit.NewTestKit(c, s.store) | ||
| tk.MustExec("use test") | ||
| tk.MustExec("drop table if exists t") | ||
| defer tk.MustExec("drop table if exists t") | ||
| tk.MustExec("create table t (id int)") | ||
|
|
||
| tolerance := 50 * time.Millisecond | ||
|
|
||
| tk.MustExec("insert into t values (1)") | ||
| time.Sleep(tolerance) | ||
| time1 := time.Now() | ||
|
|
||
| tk.MustExec("insert into t values (2)") | ||
| time.Sleep(tolerance) | ||
| time2 := time.Now() | ||
|
|
||
| tk.MustExec("insert into t values (3)") | ||
| time.Sleep(tolerance) | ||
|
|
||
| staleRows := testkit.Rows("1") | ||
| staleSQL := fmt.Sprintf(`select * from t as of timestamp '%s'`, time1.Format("2006-1-2 15:04:05.000")) | ||
|
There was a problem hiding this comment. What will happen in this situation? maybe we need to store the expression rather than There was a problem hiding this comment. I wanted to prepare the handle of infoschema in #25080, but that is not true anymore. The handle of infoschema is now added. There was a problem hiding this comment. I want to get the result There was a problem hiding this comment. Yeah, the timestamp is fixed once preprocessed now. Your expectation needs a re-preprocessing, which drops all the prepared efforts. It is not a simple reason of my implementation. If timestamp is changed, schema and snapshot is changed, the result from previous prepare statements is meaningless. Its semantic may be intuitive, but it does not make much sense under our current framework of prepare statements. A full support, I guess, will need to store the expr of There was a problem hiding this comment. I could provide a hack solution, if you really want it to be true. There was a problem hiding this comment. No, it must be re-preprocessed. Infoschema may change. Even if it is not changed, a check for stale infoschema is needed, which leads to one more tikv metadata access. Otherwise, the result is inconsistent.
Glad to hear that but it can not be true for now. I can only hack around. The solution will be tricky. There was a problem hiding this comment. Yes, I mean I can accept re-preprocessed, cannot accept the result is fixed on the point in mine example. BTW, Even if we have the infoschema cache, still we need to access TiKV to get the schema version for every request? There was a problem hiding this comment. No for the current solution. Because it is already ensured when FYI, check Lines 103 to 107 in 5c95062 SchemaVersion.
There was a problem hiding this comment. I have checked it, the implementation is different from what I understand. I think the schema version is guaranteed to increase, it could be optimized to reduce the extra request by check the schema version is not changed between the older request and the up-to-date schema version. There was a problem hiding this comment. Otherwise, it is easy to have a single point of bottleneck with the current implementation, because the meta information is always read from a certain TiKV node. |
||
|
|
||
| // test normal stale select | ||
| tk.MustQuery(staleSQL).Check(staleRows) | ||
|
|
||
| // test stale select in txn | ||
| tk.MustExec("begin") | ||
| c.Assert(tk.ExecToErr(staleSQL), NotNil) | ||
| tk.MustExec("commit") | ||
|
|
||
| // test prepared stale select | ||
| tk.MustExec(fmt.Sprintf(`prepare s from "%s"`, staleSQL)) | ||
| tk.MustQuery("execute s") | ||
|
|
||
| // test prepared stale select in txn | ||
| tk.MustExec("begin") | ||
| tk.MustQuery("execute s").Check(staleRows) | ||
| tk.MustExec("commit") | ||
|
|
||
| // test stale select in stale txn | ||
| tk.MustExec(fmt.Sprintf(`start transaction read only as of timestamp '%s'`, time2.Format("2006-1-2 15:04:05.000"))) | ||
| c.Assert(tk.ExecToErr(staleSQL), NotNil) | ||
| tk.MustExec("commit") | ||
|
|
||
| // test prepared stale select in stale txn | ||
| tk.MustExec(fmt.Sprintf(`start transaction read only as of timestamp '%s'`, time2.Format("2006-1-2 15:04:05.000"))) | ||
| tk.MustQuery("execute s").Check(staleRows) | ||
| tk.MustExec("commit") | ||
|
Comment on lines
+817
to
+824
There was a problem hiding this comment. Is There was a problem hiding this comment. Yes, it is expected. And it should be possible to throw an error in There was a problem hiding this comment. IMO this is not expected. We should avoid multiple snapshotTS in a transaction as users may think it's data inconsistency error during transaction. |
||
|
|
||
| // test prepared stale select with schema change | ||
| tk.MustExec("alter table t add column c int") | ||
| tk.MustExec("insert into t values (4, 5)") | ||
| time.Sleep(10 * time.Millisecond) | ||
| tk.MustQuery("execute s").Check(staleRows) | ||
|
|
||
| // test dynamic timestamp stale select | ||
| time3 := time.Now() | ||
| tk.MustExec("alter table t add column d int") | ||
| tk.MustExec("insert into t values (4, 4, 4)") | ||
| time.Sleep(tolerance) | ||
| time4 := time.Now() | ||
| staleRows = testkit.Rows("1 <nil>", "2 <nil>", "3 <nil>", "4 5") | ||
| tk.MustQuery(fmt.Sprintf("select * from t as of timestamp CURRENT_TIMESTAMP(3) - INTERVAL %d MICROSECOND", time4.Sub(time3).Microseconds())).Check(staleRows) | ||
|
|
||
| // test prepared dynamic timestamp stale select | ||
| time5 := time.Now() | ||
| tk.MustExec(fmt.Sprintf(`prepare v from "select * from t as of timestamp CURRENT_TIMESTAMP(3) - INTERVAL %d MICROSECOND"`, time5.Sub(time3).Microseconds())) | ||
| tk.MustQuery("execute v").Check(staleRows) | ||
|
|
||
| // test point get | ||
| time6 := time.Now() | ||
| tk.MustExec("insert into t values (5, 5, 5)") | ||
| time.Sleep(tolerance) | ||
| tk.MustQuery(fmt.Sprintf("select * from t as of timestamp '%s' where c=5", time6.Format("2006-1-2 15:04:05.000"))).Check(testkit.Rows("4 5 <nil>")) | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a.ExplicitStalenessis added as a parameter, found #25206 lost the point get case, just like me.