planner: TiCI/FULLTEXT access paths bypass tidb_isolation_read_engines
- Dominant language
- Go
- Stars
- 40.5k
- Forks
- 6.2k
- PR merge metrics
- PR metrics pending
Description
## Bug Report
### 1. Minimal reproduce step (Required)
Bug is on the FTS/TiCI feature branch `release-fts-202602` (`889c355150`); `master` does not contain the TiCI planner code (`keepOnlyTiCIPath` / `pkg/tici` are absent there).
**A. Planner-only repro (mock store, no cluster needed).** Put this in `pkg/planner/core/casetest/tici/` (TiCI DDL mocks require `failpoint-ctl enable pkg/tici`) and run with `-tags=intest`:
```go
func TestTiCIIsolationReadEngineBypass(t *testing.T) {
for _, name := range []string{"MockCreateTiCIIndexSuccess", "MockFinishIndexUpload", "MockCheckAddIndexProgress"} {
require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/pkg/tici/"+name, `return(true)`))
t.Cleanup(func() { _ = failpoint.Disable("github.com/pingcap/tidb/pkg/tici/" + name) })
}
store := testkit.CreateMockStoreWithSchemaLease(t, time.Second, mockstore.WithMockTiFlash(2))
defer ingesttestutil.InjectMockBackendCtx(t, store)()
tk := testkit.NewTestKit(t, store)
tk.MustExec("use test")
tk.MustExec("create table repro_src(id int primary key, a text, b int, fulltext index ft(a), index idx_b(b))")
tk.MustExec("insert into repro_src values (1,'support',10),(2,'other',20)")
testkit.SetTiFlashReplica(t, domain.GetDomain(tk.Session()), "test", "repro_src")
tk.MustExec("set tidb_isolation_read_engines='tikv'") // TiFlash explicitly excluded
tk.MustExec("set sql_mode='STRICT_TRANS_TABLES'")
// controls: isolation is honored
tk.MustQuery("explain format='brief' select b from repro_src where b=1").CheckContain("cop[tikv]")
tk.MustQuery("explain format='brief' select * from repro_src").CheckContain("cop[tikv]")
// bug: isolation is ignored
plan := fmt.Sprint(tk.MustQuery(
"explain format='brief' select id from repro_src where match(a) against('+support' in boolean mode)").Rows())
require.NotContains(t, plan, "mpp[tiflash]") // <-- FAILS on release-fts-202602
}
```
**B. Same on a real TiCI cluster** (TiFlash + TiCI meta/worker + TiCDC S3 sink), SQL only:
```sql
CREATE TABLE repro_src(id INT PRIMARY KEY, a TEXT, b INT,
FULLTEXT INDEX ft(a) WITH PARSER STANDARD, INDEX idx_b(b));
CREATE TABLE repro_dst(id INT);
INSERT INTO repro_src VALUES (1,'support',10),(2,'other',20);
-- make the TiFlash replica of repro_src available (TiCI reads are served by TiFlash MPP)
SET SESSION tidb_isolation_read_engines='tikv'; -- TiFlash excluded
SET SESSION sql_mode='STRICT_TRANS_TABLES';
-- controls: setting is honored
EXPLAIN format='brief' SELECT b FROM repro_src WHERE b=1; -- IndexRangeScan cop[tikv]
EXPLAIN format='brief' SELECT * FROM repro_src; -- TableFullScan cop[tikv]
-- bug: setting is ignored
EXPLAIN format='brief' SELECT id FROM repro_src WHERE MATCH(a) AGAINST('+support' IN BOOLEAN MODE);
EXPLAIN format='brief' INSERT INTO repro_dst SELECT id FROM repro_src WHERE MATCH(a) AGAINST('+support' IN BOOLEAN MODE);
INSERT INTO repro_dst SELECT id FROM repro_src WHERE MATCH(a) AGAINST('+support' IN BOOLEAN MODE); -- executes, writes id=1
```
### 2. What did you expect to see? (Required)
`tidb_isolation_read_engines='tikv'` must exclude TiFlash. TiCI FTS reads run on TiFlash MPP — `pkg/planner/util/misc.go` `isolationReadEngineForPath()` explicitly maps `kv.TiCI -> kv.TiFlash` — so the FTS access path should be filtered out. Expected either `[planner:1815]` "No access path ... valid values can be 'tikv'", or a non-TiCI plan if one exists. Never `mpp[tiflash]`.
### 3. What did you see instead (Required)
Actual plans on `release-fts-202602` (`889c355150`), real cluster:
```
EXPLAIN SELECT b FROM repro_src WHERE b=1;
IndexRangeScan cop[tikv] index:idx_b(b) -- control, honored
EXPLAIN SELECT * FROM repro_src;
TableFullScan cop[tikv] -- control, honored
EXPLAIN SELECT id FROM repro_src WHERE MATCH(a) AGAINST('+support' IN BOOLEAN MODE);
ExchangeSender mpp[tiflash]
IndexRangeScan mpp[tiflash] index:ft(a) search func:fts_match_word("support", ...) -- TiFlash used
EXPLAIN INSERT INTO repro_dst SELECT id FROM repro_src WHERE MATCH(a) AGAINST('+support' IN BOOLEAN MODE);
Insert -> ExchangeSender mpp[tiflash] -> IndexRangeScan mpp[tiflash] ... -- TiFlash used
```
The `INSERT ... SELECT` really executes and writes `repro_dst = {1}`; nothing is raised, and `tidb.log` contains no `1815`. Same with non-strict `sql_mode`. `tidb_opt_enable_alternative_logical_plans` on/off makes no difference. This is the root cause of the strict-mode DML report where `optimize.go` removes TiFlash for non-readonly statements but the FTS path still uses it.
**Root cause — the isolation check runs before the path becomes TiCI:**
1. `pkg/planner/core/planbuilder.go` `getPossibleAccessPaths()` creates the FTS path as `path := &util.AccessPath{Index: index}` without `StoreType`; the zero value of `kv.StoreType` is `kv.TiKV` (`pkg/kv/kv.go:393`, `TiKV StoreType = iota`).
2. `pkg/planner/core/logical_plan_builder.go:4508` calls `util.FilterPathByIsolationRead()`, which sees TiKV and keeps the path when `'tikv'` is allowed.
3. `pkg/planner/core/operator/logicalop/logical_datasource.go:1019` `keepOnlyTiCIPath()` later sets `ticiPath.StoreType = kv.TiCI` — with no re-check.
**Confirmation that the check itself works.** With `SET SESSION tidb_isolation_read_engines='tidb'` (neither TiKV nor TiFlash allowed), both normal and FTS paths are filtered correctly:
```
[planner:1815]Internal : No access path for table 'repro_src' is found with
'tidb_isolation_read_engines' = 'tidb', valid values can be 'tikv, tiflash'.
```
i.e. the hole only appears when the path's pre-promotion `StoreType` (TiKV) happens to be allowed.
**Related.** On the backport branch the guard added in `AnalyzeTiCIIndex` (commit `6442a27189`, PR #71280) re-checks isolation and returns `1815`, so there the statement errors instead of silently using TiFlash. That closes the isolation hole, but it also turns a statement that currently executes on `release-fts-202602` into an error, so the intended semantics of `tidb_isolation_read_engines` w.r.t. TiCI (and whether strict-mode non-readonly DML may use TiCI) still need a decision.
**Suggested fix.** Mark TiCI index paths with `StoreType = kv.TiCI` (or make `FilterPathByIsolationRead` TiCI-aware via `index.IsTiCIIndex()`) at creation time so the check runs on the real engine; alternatively keep an explicit re-check after `keepOnlyTiCIPath`. Please add regression coverage for `tidb_isolation_read_engines` in {`tikv`, `tikv,tiflash,tidb`, `tidb`} x strict/non-strict x {normal SELECT, FTS SELECT, FTS `INSERT ... SELECT`}.
### 4. What is your TiDB version? (Required)
```
Release Version: v8.4.0-this-is-a-placeholder
Edition: Community
Git Commit Hash: None
Git Branch: None
GoVersion: go1.25.6
Store: unistore
```
(binary built from source without ldflags)
Source: branch `release-fts-202602`, commit `889c3551501ba42e97f6fe8402297489876933f9`
Components used in the real-cluster run: TiFlash `v9.0.0-feature.fts` (`317f5f60`), TiCI server `d91aee51f340d99b151198c42357572d1ba8e6fb`, TiCDC `v8.5.6`; TiKV/PD `v9.0.0-beta.2.pre-nightly`.
`SELECT tidb_version()` was not captured before the ephemeral test cluster was torn down; the git commit above is exact.
Contributor guide
Research direction
Start with pkg/planner/core/planbuilder.go:getPossibleAccessPaths, logical_plan_builder.go:FilterPathByIsolationRead, and logical_datasource.go:keepOnlyTiCIPath; then run the provided tagged casetest/tici reproducer on release-fts-202602. Review the existing AnalyzeTiCIIndex guard and decide the intended strict/non-strict DML semantics. Done means regression coverage passes for the isolation-engine, statement-type, and FTS/non-FTS combinations without an invalid TiFlash plan.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, sql
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100