matrixorigin / matrixorigin/matrixone
fix: complete WAL waiters before append fail-stop
- 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. Parent issue #27945 exposed the shutdown wait chain; this issue tracks the distinct accepted-WAL-entry terminal invariant.
### Branch Name
main
### Commit ID
`c46d897e9645b80178568ef0783dd8e99e527222`
### Other Environment Information
- Hardware parameters: mo-55 bare-metal validation host; resource thresholds remained above 12 GiB MemAvailable and 100 GiB root free space.
- OS type: Linux amd64.
- Deployment: single-process standalone containing TN and LogService.
- Workload: retained 2,303,643-row FULLTEXT2 runtime with two accepted TN commits waiting for WAL completion during `SIGTERM`.
- Parent lifecycle issue: #27945.
- Implementation PR: #28014.
### Actual Behavior
An accepted TAE WAL entry normally waits for `groupCommitter` and the commit wait loop to publish its terminal state. Several failure paths can bypass that publication:
- serialization or pre-callback failure;
- LogService client acquisition failure;
- worker submission failure;
- LogService append failure before the normal success wait loop;
- close racing with pending committers.
In those paths, an accepted entry can remain blocked in `WaitDone()`. The driver close path can also consume separate worker and waiter timeout windows and then enter an additional unbounded `committer.Wait()`. During standalone shutdown this can keep TN alive while LogService is also being closed, producing a circular lifecycle wait.
Completing waiters from multiple cleanup paths is not safe either: without one explicit completion owner, reset/close/error handling can notify an entry twice or advance committed DSN for a failed committer.
### Expected Behavior
Every WAL entry accepted into a `groupCommitter` must receive exactly one terminal result:
```text
success
or
explicit error before fail-stop escalation
```
`groupCommitter` must be the sole completion owner. The first terminal publication must clear its owned-entry list so wait-loop, reset, and close paths cannot notify twice. Successful committers must preserve existing ordered notification and committed-DSN semantics; failed committers must not advance the committed watermark.
Driver close must use one shared bounded deadline for stopping intake, draining accepted workers, and completing waiter notification. On timeout it must return an error without entering another unbounded wait or closing clients/pools still in use. Append failure must retain the existing fail-stop and unknown-commit semantics after accepted waiters receive their error terminal.
### Steps to Reproduce
1. Create a LogService-backed TAE WAL driver and submit one or more entries accepted into a group committer.
2. Inject one of the pre-append or append failures listed above, or race worker submission with driver close.
3. Wait on each accepted entry with `WaitDone()`.
4. Observe that a failure before the normal commit wait loop can leave the waiter without a terminal result, or that close can enter a second/unbounded wait after its earlier timeout budget.
5. In a standalone process, trigger graceful shutdown while TN commits are blocked on those entries and observe the TN-to-LogService dependency wait.
Regression tests should cover single- and multi-entry success, every failure terminal, failed-DSN suppression, worker-submission/close races, repeated reset/close, and an injectable short shared deadline.
### Additional information
- Scope: TAE LogService WAL waiter completion and close deadline only.
- Non-goals: no SQL, DDL, public API, WAL format, transaction wire protocol, persisted format, fail-stop policy, or unknown-commit contract change.
- Independence: PR #28014 is independent of PRs #28015 and #28016 and can be reviewed separately.
- BVT: N/A. SQL BVT cannot deterministically inject serialization, client acquisition, worker submission, or LogService append failures and assert waiter completion before fail-stop. Production commit/wait-loop package and race tests are the equivalent deterministic terminal coverage.
- QA required: yes. Lifecycle integration should include this exact head in the A+B+C validation branch before final issue closure.
Contributor guide
Assessment
This issue has not been assessed yet.