matrixorigin / matrixorigin/matrixone
taskservice: stale refresh can close active task store and surface as "sql: database is closed"
- Dominant language
- Go
- Stars
- 1.9k
- Forks
- 311
- Avg merge
- 1d 3h
- Merged PRs (30d)
- 768
Description
### Is there an existing issue for the same bug?
- [x] I have checked the existing issues.
### Branch Name
3.0-dev
### Commit ID
9b4ab50a1baa71a44d00535cc4e1c75dafaca884
### Other Environment Information
Triggered from PR CI failure: https://github.com/matrixorigin/matrixone/pull/24141
Workflow run: https://github.com/matrixorigin/matrixone/actions/runs/24450572669
Artifact: https://github.com/matrixorigin/matrixone/actions/runs/24450572669/artifacts/6449509838
### Actual Behavior
`test/distributed/cases/task/sql_task.sql` failed in compose pessimistic CI with:
- `execute task sql_task_overlap;` -> `sql: database is closed`
- `select count(*) from overlap_sink;` returned `1` instead of `2`
- `select status from mo_task.sql_task_run ... limit 2;` returned only one `SUCCESS`
### Expected Behavior
The task-service storage refresh path should never close the current healthy store for a stale refresh request or a failed refresh attempt. Later SQL task executions should not observe `sql: database is closed` from a previously valid task store.
### Steps to Reproduce
The original failure was observed in the workflow above. The root cause is visible in `pkg/taskservice/task_service_holder.go`:
`refreshableTaskStorage.refresh()` closes `s.mu.store` before it:
1. rejects a stale `lastAddress` refresh request, and
2. confirms that a replacement store was created successfully.
That allows a stale refresh or a failed `storeFactory.Create()` to leave the holder pointing at a closed task store, which later surfaces as `sql: database is closed` from SQL task execution paths.
### Additional information
This appears unrelated to PR #24141 itself. That PR only changes `pkg/vm/engine/disttae/logtailreplay/*`, while the failing CI case is in the task-service/SQL-task path.
A local fix and regression coverage were prepared around:
- `TestRefreshTaskStorageSkipStaleRefreshKeepsCurrentStore`
- `TestRefreshTaskStorageCreateFailureKeepsCurrentStore`
Contributor guide
Assessment
This issue has not been assessed yet.