Skip to content

[Store] Freeze restored snapshot test eviction worker - #4156

Merged
Icedcoco merged 3 commits into
kvcache-ai:mainfrom
Icedcoco:fix/snapshot-evict-test-race
Sep 16, 2026
Merged

Icedcoco merged 3 commits into
kvcache-ai:mainfrom
Icedcoco:fix/snapshot-evict-test-race

Conversation

@Icedcoco

@Icedcoco Icedcoco commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

Description

The restored MasterService starts an eviction worker before the snapshot test can freeze it. When EvictObject leaves memory above the watermark with expired leases, that worker changes replica metadata before the second snapshot and state comparison. In the failing PR #4142 CI run, the restored worker evicted 1042 replicas and the metadata snapshot shrank from 737277 to 723791 bytes.

Set the restored instance's memory eviction watermark to 1.0 before construction, then stop and join its eviction worker before snapshot comparison, matching the original instance's existing freeze. This prevents watermark eviction during startup as well as subsequent worker mutations during comparison. Uses the upstream MasterServiceTestPeer accessors; only the shared snapshot test fixture changes.

Related: #4136. This addresses the restore-side eviction race observed in run 34979380422, not the separate tenant-quota scenario mentioned in that issue. PR #4143 records the same flaky snapshot test but does not modify it. No N12 OpLog pruning changes are included.

Module

  • Mooncake Store (mooncake-store)
  • CI/CD

Type of Change

  • Bug fix

How Has This Been Tested?

Rebuilt locally after rebasing onto upstream/main f86289c90, with RelWithDebInfo, USE_CUDA=OFF, STORE_USE_ETCD=ON.

Test commands:

cmake --build /tmp/mooncake-n12-build --target master_service_ssd_test_for_snapshot master_service_test_for_snapshot master_service_promotion_test_for_snapshot -j 16
/tmp/mooncake-n12-build/mooncake-store/tests/master_service_ssd_test_for_snapshot --gtest_filter=MasterServiceSSDSnapshotTest.EvictObject --gtest_repeat=10 --gtest_break_on_failure
ctest --test-dir /tmp/mooncake-n12-build --output-on-failure -R '^(master_service_ssd_test_for_snapshot|master_service_test_for_snapshot|master_service_promotion_test_for_snapshot)$'
pre-commit run --files mooncake-store/tests/ha/snapshot/master_service_test_for_snapshot_base.h

Test results:

  • Unit tests pass: EvictObject 10/10 repetitions; all three snapshot test binaries pass.
  • Integration tests pass (not applicable)
  • Manual testing done: inspected CI logs and verified the PR diff contains only the fixture fix.

Checklist

  • Human submitter has reviewed every changed line and can defend the change end-to-end (draft pending review)
  • AI-assisted code review completed
  • Formatting checked through the pre-commit C/C++ formatting hook
  • PR-scoped pre-commit hooks pass
  • Documentation updated (not applicable)
  • New tests added (existing failing regression and the other shared-fixture suites reused)
  • RFC filed for >500 LOC (not applicable; 9 added lines)

AI Assistance Disclosure

  • AI tools were used: Codex analyzed the CI failure, implemented the fixture fix, resolved the upstream rebase, and ran local checks. Human review is pending; this PR is a draft.

@Icedcoco
Icedcoco marked this pull request as ready for review September 16, 2026 07:40
@Icedcoco
Icedcoco merged commit 211db0d into kvcache-ai:main Sep 16, 2026
63 of 67 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants