pingcap / pingcap/tidb

executor: HashJoin wait4BuildSide ignores query cancel until Close/buildFinished

Open
#71,122 3 comments 0 reactions 0 assignees View on GitHub
severity/moderate sig/execution type/bug
Dominant language
Go
Stars
40.5k
Forks
6.2k
PR merge metrics
PR metrics pending

Description

## Bug Report

Please answer these questions before submitting your issue. Thanks!

### 1. Minimal reproduce step (Required)

HashJoin's probe fetcher waits for the build side in `wait4BuildSide`, which only selects on:

- `closeCh` (closed almost only from `HashJoin.Close()`)
- `buildFinished`

It does **not** observe `ctx.Done()` or `SQLKiller`.

Original probe-fetcher order:

1. First probe `Next()`
2. Then `wait4BuildSide`
3. Only after that, send the probe chunk to join workers

While blocked in `wait4BuildSide`, the first probe chunk has not been delivered yet, so probe workers typically sit on `probeResultCh` / `closeCh` and also do not check `SQLKiller`.

Repro sketch (conceptual; may need failpoints / a slow build child to make the race stable):

```sql
-- Session A
CREATE TABLE t_build (id INT PRIMARY KEY, v INT);
CREATE TABLE t_probe (id INT PRIMARY KEY, v INT);
-- Insert enough rows so build is slow, or inject a delay/hang in the build path.
INSERT INTO t_build SELECT ...);
INSERT INTO t_probe VALUES (...); -- small probe so the first Next() returns quickly

SELECT /*+ HASH_JOIN(t_probe, t_build) */ *
FROM t_probe JOIN t_build ON t_probe.id = t_build.id;
```

```sql
-- Session B, while the join is in "probe first Next done, still waiting for build"
KILL QUERY ;
```

Observed window to hit:

1. Probe side returns the first chunk quickly (small/local probe).
2. Build side is still running or stuck (large build, spill, or blocked downstream).
3. User issues `KILL QUERY` / session cancel **before** `Close()` runs and **before** build finishes.
4. Build path does not promptly check `SQLKiller` and therefore does not signal `buildFinished` soon.

Related code:

- `pkg/executor/join/hash_join_base.go`: `wait4BuildSide`, `fetchProbeSideChunks`
- `pkg/executor/join/hash_join_v1.go` / `hash_join_v2.go`: probe workers wait on `closeCh` + `probeResultCh`; `closeCh` is closed in `Close()`

Note: [a separate change](https://github.com/pingcap/tidb/pull/71120) may move `wait4BuildSide` earlier when empty-build probe skip is allowed. That can make this cancel blind spot appear slightly earlier for skip-capable joins, but the missing `ctx` / `SQLKiller` handling in `wait4BuildSide` is pre-existing.

### 2. What did you expect to see? (Required)

After `KILL QUERY` / query cancellation:

- HashJoin should stop promptly through the same shutdown path used for executor close (unblock `wait4BuildSide`, wake probe workers, return an interrupt error to the client).
- A blocked wait on build or on `joinResultCh` should not require the build side to finish naturally before the kill takes effect.

### 3. What did you see instead (Required)

Query cancellation that only sets `SQLKiller` / cancels context may **not** unblock `wait4BuildSide`.

In that window:

- Main thread can stay blocked in `HashJoin.Next` on `joinResultCh`
- Probe fetcher stays blocked in `wait4BuildSide`
- Probe workers stay blocked waiting for the first probe chunk
- Kill appears ineffective until build eventually finishes or the executor is `Close()`'d and `closeCh` is closed

Root cause: HashJoin shutdown for this wait is effectively `Close → closeCh`, while `SQLKiller` is checked only on some probe/spill hot paths—not on `wait4BuildSide` or on workers waiting for the first probe chunk.

### 4. What is your TiDB version? (Required)

https://github.com/pingcap/tidb/pull/71120#discussion_r4016513866

[master](https://github.com/pingcap/tidb/commit/5dd438d8ee60ab0ad60929c38cc313fb06d9ed07) (reproduced by code inspection of current HashJoin probe-fetcher / `wait4BuildSide` shutdown paths).

### Suggested fix direction (optional)

1. Make `wait4BuildSide` observe cancellation via the same shutdown path as `closeCh` (e.g. pass query `ctx` and `select` on `ctx.Done()`, and/or ensure SQL killer cancellation closes / feeds `closeCh`).
2. Preserve existing `buildFinished` and `closeCh` handling.
3. Ensure a blocked build wait and a blocked `joinResultCh` wait are released when the query is canceled, without leaving probe workers stranded on `probeResultCh`.

Contributor guide

Open the contributing guide

Research direction

Start in pkg/executor/join/hash_join_base.go by tracing wait4BuildSide and fetchProbeSideChunks, then compare the probe-worker waits in hash_join_v1.go and hash_join_v2.go. Reproduce or inspect the cancellation window described in the issue, and verify that query cancellation releases the build wait and probe workers, unblocks result delivery, and returns an interrupt error without relying on Close().

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.