dataroaring commented on PR #61199:
URL: https://github.com/apache/doris/pull/61199#issuecomment-4181395008
## Test Coverage Analysis
The core TSO logic (TSOTimestamp + TSOService) has solid unit test coverage.
However, the integration seams — where TSO meets the transaction commit path,
DDL operations, BE publish pipeline, and edit log replay — are under-tested.
### What's Well Covered
- **TSOTimestamp** — 11 test methods: constructor, compose/extract,
bit-width masking, negative validation, serialization round-trip, compareTo,
MAX_LOGICAL_COUNTER.
- **TSOService** — ~20 tests: getTSO error paths, runAfterCatalogReady,
calibrateTimestamp (persist failure, clock backward, journal disabled, fatal
flag reset), updateTimestamp, generateTSO, writeTimestampToBDBJE, save/load
round-trip.
- **Commit TSO in transactions** — TSO set when `enableTso=true`, remains
`-1` when `enableTso=false`.
- **Regression tests** — REST API validation (monotonicity, composition) and
end-to-end `COMMIT_TSO` in `information_schema.rowsets`.
### Gaps
**High Priority:**
1. **No concurrent `getTSO()` test** — TSOService uses `ReentrantLock` +
`AtomicLong`, but no test verifies correctness under concurrent access. For a
globally-monotonic TSO, this is critical.
2. **`getCommitTSO()` error paths untested** —
`DatabaseTransactionMgr.getCommitTSO()` has 5 branches but only 2 tested.
Missing:
- `Config.enable_tso_feature = false` (global disable)
- `TSOService` throws exception during commit →
`TransactionCommitFailedException`
- `TSOService` returns `<= 0`
- Mixed tables (some TSO-enabled, some not)
3. **No `ALTER TABLE ... SET ("enable_tso" = "true/false")` test** —
`ModifyTablePropertiesOp` (+16 lines) and `PropertyAnalyzer` (+23 lines) have
zero test coverage. Regression tests only cover CREATE TABLE.
**Medium Priority:**
4. **TSOAction.java has no unit test** — Error paths (FE not ready,
non-master FE, exception) untested. Only happy path via regression.
5. **BE C++ plumbing largely untested** — Changes span 10+ files propagating
`commit_tso`. Specific gaps:
- `pb_convert.cpp` — 4 new `has_commit_tso()` blocks, no test
- `schema_rowsets_scanner.cpp` — COMMIT_TSO column fill, no BE unit test
- `DiscontinuousVersionTablet` struct, no test
- `engine_publish_version_task.cpp` — significant refactoring, no direct
test
6. **Edit log replay path** — `replayWindowEndTSO` is tested, but the full
path (deserialize from journal → dispatch via `OP_TSO_TIMESTAMP_WINDOW_END` →
replay) is not.
7. **Metrics** — 4 new counters (`tso_clock_drift_detected`,
`tso_clock_backward_detected`, `tso_clock_calculated`, `tso_clock_updated`)
never asserted in any test.
**Low Priority:**
8. No regression test for TSO-disabled tables (verifying `commit_tso` stays
`-1`).
9. `SchemaChangeHandler` / `CloudSchemaChangeHandler` `enable_tso`
validation — no test.
10. No integration test for TSO state surviving FE restart/checkpoint
recovery.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
