Skip to content

[Store] Python store API supports batch operation#511

Closed
xinranwang17 wants to merge 2 commits intokvcache-ai:mainfrom
xinranwang17:python-batch-api
Closed

[Store] Python store API supports batch operation#511
xinranwang17 wants to merge 2 commits intokvcache-ai:mainfrom
xinranwang17:python-batch-api

Conversation

@xinranwang17
Copy link
Copy Markdown
Contributor

@xinranwang17 xinranwang17 commented Jun 17, 2025

This PR introduces batch APIs (put_batch, get_batch, is_batch_exist) in the Python module to accelerate data transfer. These APIs are concurrently utilized in the Mooncake store implementation as the L3 cache in SGLang.
Related PR in sglang

1. expose batch operation via python api
2. add BatchIsExist support
@xinranwang17 xinranwang17 changed the title Python store API supports batch operation [Store] Python store API supports batch operation Jun 17, 2025
@james0zan
Copy link
Copy Markdown
Member

Since the PR in SGLang requires further refactoring, we can proceed with reviewing and merging this PR in Mooncake as a preparatory step.

@zhaoyongke
Copy link
Copy Markdown
Collaborator

Since the PR in SGLang requires further refactoring, we can proceed with reviewing and merging this PR in Mooncake as a preparatory step.

Thx!

* @param keys Key to check
* @return Map of keys to booleans
*/
std::unordered_map<std::string, bool> BatchIsExist(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I’d prefer to return `vector that matches up one-to-one with the keys in the input parameters.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

According to SGLang integration, is it better to return <key, is_exist> pair so that the existed key can be easily appended instead of addressing them by index again.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I was looking through the sglang code and noticed this part:

mooncake_exist_keys = self.mooncake_l3_kv_pool.is_batch_exist(
    fragment_keys
)
non_exist_keys = []
non_exist_value = []
for i in range(len(fragment_keys)):
    if not mooncake_exist_keys[fragment_keys[i]]:

I think we could simplify it to:

mooncake_exist_keys = self.mooncake_l3_kv_pool.is_batch_exist(
    fragment_keys
)
non_exist_keys = []
non_exist_value = []
for i in range(len(fragment_keys)):
    if mooncake_exist_keys[i] == 1:

This way we're using direct indexing instead of querying the hash map, which should be slightly more efficient.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LMCache needs same interface, so I’ve submitted a separate PR for batch exist: #542.

Appreciate it if you could take a look. Thanks!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

great work! Since array is more efficient, let's use array as the return value of batch exist interface.

* @param keys Keys to check
* @return existence map, true if exists, false if not
*/
std::unordered_map<std::string, ErrorCode> BatchIsExist(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

same above

test_client_->BatchIsExist(keys);
end = std::chrono::high_resolution_clock::now();
LOG(INFO) << "Time taken for BatchIsExist: "
<< std::chrono::duration_cast<std::chrono::microseconds>(end -
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could you share a quick performance result here?

@xinranwang17
Copy link
Copy Markdown
Contributor Author

Since we have had an agreement on using array as the return value of batch exist, this PR will be accept. I'd like to continue contribute other implementation, such as batch put/get interface, in a separate PR.

@jeremyzhang866
Copy link
Copy Markdown

hi @xinranwang17 .When can this pr be merge. thanks

@jeremyzhang866
Copy link
Copy Markdown

hi @xiaguan When does lmcache have such a batch interface. thanks

@xinranwang17
Copy link
Copy Markdown
Contributor Author

hi @xinranwang17 .When can this pr be merge. thanks

Part of this PR has been merged here: #542
I have submit another PR(#556) consisting of the rest part.

@xiaguan
Copy link
Copy Markdown
Collaborator

xiaguan commented Jun 30, 2025

hi @xiaguan When does lmcache have such a batch interface. thanks

After this LMCache/LMCache#924 got merged, I will submit mooncake's impl

@jeremyzhang866
Copy link
Copy Markdown

hi @xiaguan When does lmcache have such a batch interface. thanks

After this LMCache/LMCache#924 got merged, I will submit mooncake's impl

could you creat a PR about this so that i can preview. thanks.

@xiaguan
Copy link
Copy Markdown
Collaborator

xiaguan commented Jul 1, 2025

hi @xiaguan When does lmcache have such a batch interface. thanks

After this LMCache/LMCache#924 got merged, I will submit mooncake's impl

could you creat a PR about this so that i can preview. thanks.

LMCache/LMCache#934,

@xiaguan
Copy link
Copy Markdown
Collaborator

xiaguan commented Jul 1, 2025

since #556 is merged, close this pr

@xiaguan xiaguan closed this Jul 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants