[tinswzy]

Closes #410. Adds seven service-mode data corruption tests using a three-replica cluster and a dedicated CI workflow.

The tests damage or truncate replica data.log files and assert recovery behavior:

• Active, one damaged replica: a fresh writer fences the segment; healthy replicas retain all entries and compaction seals it.
• Active, two or three truncated replicas: a fresh writer fences at the shorter boundary; the reader crosses into the next segment without exposing the discarded tail, and compaction seals the shortened segment.
• Completed, one or two damaged replicas: healthy replicas serve every entry and compaction seals it.
• Completed, all replicas damaged over entries 10..19: compaction fails, the reader stalls, and declaring a skip range lets it read entries 20..99.
• Truncate: the truncation point advances across damaged data, and a new segment remains writable and readable.

A2b is not constructed separately; A3 covers the same stalled-reader and skip-range recovery behavior.

Test infrastructure waits for restarted gRPC servers to initialize and for membership to converge before the next restart. Component CI excludes the dedicated test prefix. No production-code changes.

Validation: local etcd + MinIO, all seven tests with go test -race -v -timeout=15m -run '^TestDataCorruptionService_' ./tests/integration/; existing full-cluster restart durability test also passes with -race.

[github-actions]

CI commands

Comment any of these to rerun checks on this pull request:

Command	Reruns
/rerun	every failed workflow on the latest commit
/rerun-lint	Lint
/rerun-mod-tidy	Go Mod Tidy Check
/rerun-unit-test	Unit Test
/rerun-e2e-local	E2E Local
/rerun-e2e-objectstorage	E2E Object Storage
/rerun-e2e-service	E2E Service
/rerun-component-test	Component Integration Test
/rerun-chaos	Chaos Test
/rerun-service-stability-process	Service Stability (Process)
/rerun-service-stability-docker	Service Stability (Docker)
/rerun-operator	Operator Test
/rerun-upgrade-local	Upgrade Compat Local
/rerun-upgrade-objectstorage	Upgrade Compat Object Storage
/rerun-upgrade-service	Upgrade Compat Service

The ci-passed label is applied automatically once every required check passes.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.01%. Comparing base (d659eb7) to head (e3e90f6).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #411      +/-   ##
==========================================
+ Coverage   83.99%   84.01%   +0.02%     
==========================================
  Files         213      213              
  Lines       30147    30147              
==========================================
+ Hits        25321    25328       +7     
+ Misses       3656     3648       -8     
- Partials     1170     1171       +1     

Components	Coverage Δ
Server	83.71% <ø> (+0.03%)	⬆️
Client	90.86% <ø> (+0.04%)	⬆️
Meta	85.58% <ø> (ø)
Common	88.30% <ø> (ø)
see 8 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[czs007]

Must-fix issues introduced by this PR: 0
Merge recommendation: Mergeable as-is — this review found no must-fix issue introduced by this PR.

Adversarial review found no issues requiring changes.

Verified:

• mini_cluster.go:512 — RestartNode now calls Run() synchronously; Run returns after gRPC init and logstore start while Serve continues in a background goroutine, so the call does not block, and the error path returns before the node is re-inserted into cluster.Servers, so a half-initialized node is never put back into the cluster.
• service_data_corruption_e2e_test.go:296-303 — the restartCorruptionNode convergence gate is sound: GetActiveNodes() is already at full strength when RestartNode returns, and the GetMemberCount() < GetActiveNodes() condition requires every live node to see 3 members, so a not-yet-joined node keeps Eventually waiting rather than producing a false convergence.
• service_data_corruption_e2e_test.go:565-572 — the A3 skip-range read loop cannot nil-dereference firstAfter: if the 90s context expires, the last readErr is a deadline error and require.NoError at :571 calls FailNow before :572 touches firstAfter.Id.
• service_data_corruption_e2e_test.go:587-601 — truncateDataLogAfterEntry cuts at payloadStart + payloadLength, exactly the end of the last good data record; headerStart comes from offsets already validated by dataRecordPayloadOffsets, and require.Less guards out-of-range indexes.
• service_data_corruption_e2e_test.go:698-775 — B2 asserts the intended quorum recovery semantics: with two replicas truncated to entry 49 and one intact, the reader sees only 0..49, so a future implementation change that recovers past the majority point would turn the test red.
• e2e_service_failover_test.go:4177 — the only existing RestartNode caller that continues after an error only logs it; all 34 existing call sites check err and none depend on Run executing asynchronously, so the sync change does not alter their outcomes.

Low (non-blocking)

• .github/workflows/pr-ci-label.yaml:84 — The new E2E Service Data Corruption job is wired into neither the ci-passed gate nor the nightly run, and this PR also removes its 7 test cases from everywhere else. The job name is absent from both the workflow_run.workflows list at pr-ci-label.yaml:20-31 and the REQUIRED_CHECKS array at :84, so its completion never triggers a label recalculation and the recalculation never consults it. nightly.yaml:13-80 has no uses: for integration-test-e2e-service-corruption.yaml, leaving the new file's workflow_call: trigger and nightly- concurrency-group prefix without any caller. Meanwhile integration-test-components.yaml:90 now adds TestDataCorruptionService to -skip, so the suite runs only in this one unregistered workflow. A later PR breaks fencing, the reader skip range, or compaction CRC validation so that TestDataCorruptionService_* fails → the E2E Service Data Corruption check goes red on the PR, but ci-passed is still applied once the other required checks are green, and after merge nightly never exercises the suite, so the regression is never surfaced again. Note that master currently has no branch protection (the only ruleset, protect_master, is disabled), so ci-passed is a conventional label rather than a hard gate, and the analogy to the Service Stability workflows holds only halfway: those are also outside REQUIRED_CHECKS, but nightly.yaml:67-80 does run them. Suggestion: please confirm whether this is intentional. If the suite should participate in gating, add E2E Service Data Corruption to both workflow_run.workflows and REQUIRED_CHECKS (either one alone has no effect), and add a uses: entry in nightly.yaml so the workflow_call trigger and nightly- prefix actually serve a purpose. Otherwise, consider removing the two caller-less config fragments to avoid misleading future readers. (raised by bigsheeper, xiaocai2333)

[czs007]

Must-fix issues introduced by this PR: 0
Merge recommendation: Mergeable as-is — this review found no must-fix issue introduced by this PR.

Re-review of e3e90f6451d9

• Confirmed — .github/workflows/pr-ci-label.yaml:84 — the new E2E Service Data Corruption workflow is still outside every gate: it is absent from both the workflow_run.workflows list (lines 20-31) and the REQUIRED_CHECKS array (line 84), nightly.yaml:13-80 has no uses: pointing at integration-test-e2e-service-corruption.yaml, and the same PR adds TestDataCorruptionService to the -skip list in integration-test-components.yaml while none of the -run patterns in integration-test-e2e-service.yaml:91/105/119 match TestDataCorruptionService_*. A PR that regresses any of the 7 TestDataCorruptionService_* cases while the other required checks pass → ci-passed is still applied and nightly never exercises the cases, so a regression in the corruption-handling path can be merged unnoticed. The workflow_call: trigger and the nightly- concurrency prefix in the new workflow currently have no caller. Suggestion: add E2E Service Data Corruption to both lists in pr-ci-label.yaml and add a uses: ./.github/workflows/integration-test-e2e-service-corruption.yaml job to nightly.yaml; if exclusion is intentional, remove the dead workflow_call entry and nightly- prefix and state the rationale in the PR description. Rated low. (raised by bigsheeper, xiaocai2333)

Adversarial review found no issues requiring changes.

Verified:

• server/service.go:198-208 — Run() returns after init() (waits only for the grpcErrChan ready signal) and start() (log store start plus restoreLifecycleEnforcement); grpcServer.Serve runs in the background startGrpcLoop goroutine, so the now-synchronous RestartNode cannot block.
• tests/utils/mini_cluster.go:517 — cluster.Servers[nodeIndex] is assigned only after Run returns, which closes the previous window where an immediate LeaveNodeWithIndex/Stop could race a Run goroutine that had not finished initializing grpcServer.
• tests/integration/service_data_corruption_e2e_test.go:198-224 (dataRecordPayloadOffsets) — the CRC4+Type1+Len4 header layout matches codec/format.go:22, only DataRecordType records are collected, and both offset+RecordHeaderSize <= len and offset+total > len guards prevent over-read.
• tests/integration/service_data_corruption_e2e_test.go:587-601 (truncateDataLogAfterEntry) — the cutoff lands at the end of the last good record, keeping the file header and records 0..lastGood intact, consistent with the LAC=49 expectation in B2/B3.
• tests/integration/service_data_corruption_e2e_test.go:542-577 (A3 skip-range recovery) — the read loop exits only on a non-deadline result or skipCtx expiry, so a broken skip mechanism surfaces as a require.NoError failure rather than a false pass; the firstAfter==20 then 21..99 index handoff is correct.
• tests/integration/compacted_cleanup_e2e_test.go:202/217 — compactedCleanupQuorumNodeIndexes and requireCompactedCleanupSegmentState referenced by the new tests exist in the same package.

No new findings this round and nothing retracted; the only surviving item is the previously reported low-severity CI gating gap adjudicated above.