pingcap / pingcap/tidb

HashJoin empty-build skip-probe can drop TiFlash MPP execution summaries in EXPLAIN ANALYZE

Open
#71,117 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)

MPP execution summaries from TiFlash can be missing in `EXPLAIN ANALYZE` / slow-log plan when a root **HashJoin** treats the MPP `TableReader` as the probe side, the build side ends up empty, and HashJoinV2 skips probing after the first probe `Next()`.

**Why it happens (mechanism)**

1. TiFlash attaches executor execution summaries only in a **trailing** MPP packet after all data packets (`finishWrite` → `sendExecutionSummary` → EOF).
2. HashJoinV2 probe fetcher always does **one** `Next(probe)` **before** checking whether the build side is empty and `canSkipProbeIfHashTableIsEmpty` applies.
3. For `LEFT OUTER JOIN` with the left side as build, an empty build correctly skips further probing and closes the probe `TableReader` / MPP stream.
4. `selectResult.Close()` does **not** drain remaining MPP packets for summaries (`CollectUnconsumedCopRuntimeStats` is only implemented for TiKV cop iterator).
5. `ReportMPPTaskStatus` (out-of-band summary reporting) is only enabled today when there is a `LIMIT` above the MPP `TableReader` (`needReportExecutionSummary`). HashJoin empty-build early close is **not** covered, so the trailing summary packet is dropped.

**Unit-test reproduction (already in tree on a local branch)** https://github.com/JaySon-Huang/tidb/commit/4fc5630017f0eac274a18bd879c9b4eb505dde5c

```bash
cd tidb
go test ./pkg/distsql/ -run TestMPPExecutionSummaryLostOnEarlyClose -v
```

This mocks a TiFlash-like stream: data packet first, summary-only packet second.

- `full_drain_records_summary`: consume until EOF → summaries are recorded.
- `early_close_loses_summary`: one `Next()` then `Close()` → trailing summaries are lost (current buggy behavior).

**Integration shape (mock TiFlash / unistore)**

```bash
go test -tags=intest ./pkg/executor/test/tiflashtest/ \
-run TestHashJoinEmptyBuildMPPProbeEarlyCloseShape -v
```

Simplified SQL shape (same class as production):

```sql
-- t_build: TiKV, filter yields 0 rows (index may still hit some keys)
-- t_probe: TiFlash MPP aggregation / full scan

SELECT /*+ HASH_JOIN(b, c) */ b.id, c.cnt
FROM t_build b
LEFT JOIN (
SELECT /*+ read_from_storage(tiflash[t_probe]), mpp_1phase_agg() */
mid, count(*) AS cnt
FROM t_probe
GROUP BY mid
) c ON b.id = c.mid
WHERE b.customer_id = 1 AND b.link_id = 999; -- matches no rows after Selection
```

Typical `EXPLAIN ANALYZE` shape:

```text
HashJoin (LEFT OUTER) @ root
├─ Build: TableReader/IndexLookUp + Selection actRows = 0
└─ Probe: TableReader → ExchangeSender mpp[tiflash]
TableReader actRows > 0 (first Next happened)
MPP operators: empty execution info / actRows = 0
```

Note: unistore does not emit TiFlash-style `ExecutorId` summary trailers, so the integration test asserts plan/runtime shape; the packet-loss mechanism is covered by the unit test above.

**Production symptom (for context)**

- Root `TableReader` shows non-zero `actRows` and long `fetch_resp_duration` (data was received).
- All `mpp[tiflash]` operators under it have blank execution info and `actRows = 0`.
- TiFlash `MPPTaskStatistics` logs still show complete per-executor stats for the same `start_ts` / digests.
- Slow log may show `Unpacked_bytes_*_tiflash = 0` and `tiflash_ru = 0` while `Storage_from_mpp = true`, because those counters also depend on execution summaries.

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

- `EXPLAIN ANALYZE` / slow-log plan should show TiFlash MPP operator execution info (`time`, `loops`, `tiflash_task`, scan details, etc.) and correct `actRows` for MPP operators whenever TiFlash finished (or reported) the task.
- Early close of an MPP stream (empty HashJoin build skip-probe, similar to `LIMIT`) should not silently drop execution summaries.

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

- MPP subtree execution info is empty and `actRows` stay 0, even though:
- the probe `TableReader` consumed at least one chunk from TiFlash, and/or
- TiFlash local task tracing shows the query finished with full executor statistics.
- Related RU / unpacked TiFlash byte counters in slow log can also stay zero for the same reason.

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

Observed on a cluster build based on TiDB **v26.3.12**

---

## Suggested fix directions

These are proposals for discussion; not mutually exclusive.

### A. Prefer: extend `ReportMPPTaskStatus` coverage (low latency risk)

Today `needReportExecutionSummary` only returns true when a `LIMIT` sits above the MPP `TableReader`. Empty HashJoin build + `canSkipProbeIfHashTableIsEmpty` is the same class of early close.

- Extend the condition so HashJoin (and similar early-close parents) also set `reportExecutionInfo`.
- TiFlash already supports reporting summaries via `ReportMPPTaskStatus` when asked.
- Latency: out-of-band report; does **not** force the data stream to finish sending unused rows (unlike full drain).

### B. Adjust HashJoin probe fetcher order (improves the empty-build case)

Current order in `fetchProbeSideChunks`:

1. `Next(probe)` (may block a long time on first TiFlash response)
2. `wait4BuildSide`
3. if empty build && `canSkip` → return / close

Suggested (only when `canSkipIfBuildEmpty`):

1. `wait4BuildSide` first
2. if skip → return without `Next(probe)`
3. otherwise fetch probe as today

Latency:

- Empty build: **reduces** TiDB wall time (avoids waiting for a useless first probe chunk; production case waited ~probe runtime even though join result was empty).
- Non-empty build: usually neutral or small; `Open` often already dispatched MPP, so delaying the first `Next` may still hit already-buffered data.
- Alone, this mainly fixes wasted wait; if MPP was already opened and then closed without reading the trailer, summaries may still be missing unless combined with (A) or (C).

Stronger variant: when skip is certain, avoid opening / dispatching the probe MPP task at all (larger change, best for both latency and wasted TiFlash work).

### C. Avoid as primary fix: drain trailing packets on MPP `Close`

Naive drain-to-EOF on `Close` can **hurt latency badly** for `LIMIT` / early cancel with large unread results: the protocol is `data… → summary → EOF`, so you cannot skip data and fetch only the summary on the same stream.

A bounded “drain what is already buffered” helper might help occasionally but is racy and incomplete. Prefer (A) for correctness without sacrificing early-close benefits.

### Recommended combination

1. **(A)** so execution summaries remain correct under HashJoin empty-build early close (same path as `LIMIT`).
2. **(B)** so empty-build queries do not block on a pointless first probe `Next()`.

### Related code pointers

- TiDB HashJoin probe fetch order: `pkg/executor/join/hash_join_base.go` (`fetchProbeSideChunks`, `wait4BuildSide`)
- `canSkipProbeIfHashTableIsEmpty`: `pkg/executor/join/hash_join_v2.go`
- Summary recording / TiFlash trailing packet comment: `pkg/distsql/select_result.go`
- `ReportMPPTaskStatus` gating: `pkg/executor/internal/mpp/local_mpp_coordinator.go` (`needReportExecutionSummary`)
- TiFlash send path: `MPPTask::finishWrite` (`sendExecutionSummary` then `finishWrite`)

Contributor guide

Open the contributing guide

Research direction

Start with pkg/executor/join/hash_join_base.go and hash_join_v2.go, then inspect needReportExecutionSummary in pkg/executor/internal/mpp/local_mpp_coordinator.go and summary handling in pkg/distsql/select_result.go. Run TestMPPExecutionSummaryLostOnEarlyClose and the tiflashtest integration shape first. Done means the empty-build early-close path preserves MPP execution summaries without requiring a full unread-stream drain, with regression coverage for the reported behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
databases, distributed-systems, testing-qa
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.