Skip to content

[Bugfix] Pass a lease policy from the BatchProbe DSL step - #4742

Closed
kunkunblueberry wants to merge 1 commit into
kvcache-ai:mainfrom
kunkunblueberry:fix-batchprobe-dsl-call
Closed

kunkunblueberry wants to merge 1 commit into
kvcache-ai:mainfrom
kunkunblueberry:fix-batchprobe-dsl-call

Conversation

@kunkunblueberry

Copy link
Copy Markdown
Contributor

Description

main currently does not build its test tree: #4314 changed MasterService::BatchProbeKey to take a GrantLeasePolicy, but the MasterScenario DSL kept calling the two-argument form, so master_service_test_dsl fails to compile and every PR's CTest unit tests job dies while building the tests (verified on two unrelated PRs: #4736 and #4741, both failing at dsl/scenario.cpp:1926 with no matching function for call to BatchProbeKey(std::vector<std::string>&, TenantId)).

This is a one-call-site fix: pass GrantLeasePolicy{}, i.e. lease_mode = None. The implementation maps that to BatchExistKeyImpl(..., grant_lease=false), which is exactly the lease-free existence probe the DSL step relied on before the signature change, so scenario behavior is unchanged.

Verified by building the affected target (master_service_test_dsl) — it links after the fix.

Suggested Merge Priority

  • P0 — Urgent
  • P1 — High: main's test tree does not build, blocking CI on every open PR
  • P2 — Normal
  • P3 — Low

Module

  • Transfer Engine (mooncake-transfer-engine)
  • Mooncake Store (mooncake-store)
  • Mooncake Conductor (mooncake-conductor)
  • Reshard (mooncake-reshard)
  • Mooncake EP (mooncake-ep)
  • Mooncake PG (mooncake-pg)
  • Integration (mooncake-integration)
  • P2P Store (mooncake-p2p-store)
  • Python Wheel (mooncake-wheel)
  • Common (mooncake-common)
  • Mooncake RL (mooncake-rl)
  • CI/CD
  • Docs
  • Other

Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Breaking change
  • Documentation update
  • Performance improvement
  • Other

How Has This Been Tested?

Test commands:

cmake --build build --target master_service_test_dsl

Test results:

  • Unit tests pass (target builds; the DSL is test-only infrastructure)
  • Integration tests pass (if applicable)
  • Manual testing done

Checklist

  • I have performed a self-review of my own code
  • I have formatted my code using ./scripts/code_format.sh
  • I have run pre-commit on the files changed in this PR and all hooks pass
  • I have updated the documentation (if applicable)
  • I have added tests to prove my changes are effective (existing DSL scenarios exercise this step)
  • For changes >500 LOC: I have filed an RFC issue

AI Assistance Disclosure

  • No AI tools were used
  • AI tools were used (specify below)

Found and fixed with AI assistance; the human submitter reviewed the change and the reasoning for GrantLeasePolicy{} preserving prior behavior.

kvcache-ai#4314 changed MasterService::BatchProbeKey to take a GrantLeasePolicy, but
the MasterScenario DSL kept calling the two-argument form, so
master_service_test_dsl no longer compiles and every PR's CTest job fails
while building the tests.

Pass GrantLeasePolicy{} (lease_mode None), which the implementation maps
to the lease-free existence probe the DSL step relied on before, so the
scenario behavior is unchanged.
Copilot AI balanced review requested due to automatic review settings October 17, 2026 12:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

@he-yufeng

Copy link
Copy Markdown
Collaborator

Heads up: this exact fix already landed via #4729 (merged), which changed the same MasterScenario::Then(BatchProbeSpec) call site to BatchProbeKey({}, keys, TenantId(...)), passing an empty GrantLeasePolicy just like this PR's GrantLeasePolicy{}. The remaining diff between the two is only formatting, and this branch now conflicts with main because both touched the same lines.

Could you rebase onto current main and check whether anything remains? If the DSL step builds clean now, this can probably be closed; if you intended the DSL to thread a real (non-default) lease policy from BatchProbeSpec, that part would need to be reworked on top of the merged fix.

@kunkunblueberry

Copy link
Copy Markdown
Contributor Author

Thanks @he-yufeng — confirmed, #4729 landed the same call-site fix ( BatchProbeKey({}, keys, TenantId(...)) ), so this PR is redundant and I am closing it rather than keeping a duplicate. My branch only existed to keep two unrelated PRs buildable while main was broken; I am dropping that commit from both of them and rebasing onto current main, so nothing from this PR remains in flight.

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.

3 participants