matrixorigin / matrixorigin/matrixone
[Tech Debt]: tunnel.go over-complex pipe state machine (989 lines, 7 implicit state flags)
- Dominant language
- Go
- Stars
- 1.9k
- Forks
- 311
- Avg merge
- 1d 3h
- Merged PRs (30d)
- 768
Description
## Summary
`tunnel.go` has grown to 989 lines with 54 exported symbols. The pipe state machine uses 7 implicit state flags (`started`, `paused`, `transferred`, `transferIntent`, `inPreRecv`, `unsafeToTransfer`, `shutdown`) coordinated through 3 synchronization primitives (`sync.Mutex`, `sync.Cond`, `sync.WaitGroup`) with no explicit state transition validation.
## Problem
- **Maintenance risk**: no one (including the original author) can safely modify this file after 6 months without high probability of introducing bugs
- **State explosion**: 7 boolean flags = 128 theoretical states, transition validity is implicit in code logic
- **Testability**: hard to test specific state transitions in isolation
- **Reviewability**: PRs touching tunnel.go require extremely high reviewer context
## Specific Issues
### Complex pipe state coordination (L669-832)
The `pipe.kickoff()` function handles: message rx/tx, error handling, pause/resume, transfer start, and deliver submission - all in a single goroutine entry point.
### Mixed synchronization primitives
Three different primitives (Mutex, Cond, WaitGroup) used for coordination where channel-based communication would be simpler and less error-prone.
### No explicit state machine
Current approach uses boolean flags checked in ad-hoc order. An explicit FSM would:
- Make all valid transitions visible
- Allow mechanical verification of transition correctness
- Simplify testing
## Suggested Approach
1. Extract pipe into `sender` + `receiver` as independent goroutines
2. Use channels (not Cond) for coordination between them
3. Introduce explicit FSM (e.g., `github.com/looplab/fsm`) for transfer state management
4. Split the 989-line file into `tunnel.go` + `pipe.go` + `transfer.go`
Contributor guide
Assessment
This issue has not been assessed yet.