matrixorigin / matrixorigin/matrixone

taskservice: stale refresh can close active task store and surface as "sql: database is closed"

Open
#24,144 9 comments 0 reactions 1 assignee Claimed by @gouhongshen View on GitHub
ai-easy deferred kind/bug
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.