ingestctrl sstIter skips reader Close after iterator Close error
- 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)
This is a deterministic fault-injection/unit-level repro for the local ingest SST iterator close path.
1. Create a real in-memory SST reader and iterator for `sstIter` in `pkg/ingestor/ingestctrl`.
2. Attach an iterator close hook that returns a deterministic root error.
3. Wrap the reader's backing readable so that calls to the underlying `Close` can be counted.
4. Call `sstIter.Close`.
5. Observe both the returned error and whether the reader's backing readable was closed.
The current-source RED observed:
```text
returned error: ai-native iterator close failed
backing readable Close: 0
```
A manual `reader.Close` after `sstIter.Close` could still close the reader, which proves `sstIter.Close` skipped the reader close when the iterator close returned an error.
### 2. What did you expect to see? (Required)
`sstIter.Close` should preserve the iterator close root error and still close the owned reader exactly once.
The iterator and reader are both owned by the same `sstIter`; an error from the first terminal action should not skip the second terminal action.
### 3. What did you see instead (Required)
The current implementation returns immediately when `i.iter.Close()` returns an error, so `i.reader.Close()` is skipped.
The user-visible operation already fails with the iterator close error, but the backing SST reader/readable can remain open until process-level cleanup. This is a resource-lifecycle bug on an error path, not a silent data-corruption issue.
A local minimal fix that records the iterator close error, continues to close the reader, and then returns `multierr.Combine(iterErr, readerErr)` made the same repro pass:
```text
returned error: ai-native iterator close failed
backing readable Close: 1
```
### 4. What is your TiDB version? (Required)
```text
Current master source commit: 13282a8bd06bd33324a4dbfd3c1c03685f3cd9aa
```
Likely root cause and fix direction
`sstIter.Close` uses sequential early return for two owned terminal resources. If the iterator close fails, the function returns that error before closing the reader.
The fix direction is to call both terminal close actions, preserve the iterator root error, and combine it with any reader close error, for example by collecting `iterErr`, calling `reader.Close`, and returning `multierr.Combine(iterErr, readerErr)` wrapped consistently with the surrounding code.
Contributor guide
Research direction
Start in pkg/ingestor/ingestctrl at sstIter.Close and reproduce the iterator-close failure with an in-memory SST reader, a failing iterator close hook, and a counted backing readable Close. Done means the iterator error is preserved, the owned reader is closed exactly once even when iterator close fails, and any reader-close error is combined consistently with the surrounding code.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 76/100