Skip to content

Conversation

Jeffwan
Copy link
Collaborator

@Jeffwan Jeffwan commented May 1, 2025

Pull Request Description

  1. Remove centralized and distributed controller. Use KVBackendReconciler interface instead, each backend should implement their own reconciler
  2. Build an abstract DistributedReconciler for both HPKV and Infinistore, they share most of the similar logic.
  3. Build another KVBackend interface, they just provide objects and main reconcilation workflow is done by DistributedReconciler
  4. Add enough use test coverage
  5. Refactor some common helpers, constants to common libs.

Related Issues

Resolves: part of #948

Important: Before submitting, please complete the description above and review the checklist below.


Contribution Guidelines (Expand for Details)

We appreciate your contribution to aibrix! To ensure a smooth review process and maintain high code quality, please adhere to the following guidelines:

Pull Request Title Format

Your PR title should start with one of these prefixes to indicate the nature of the change:

  • [Bug]: Corrections to existing functionality
  • [CI]: Changes to build process or CI pipeline
  • [Docs]: Updates or additions to documentation
  • [API]: Modifications to aibrix's API or interface
  • [CLI]: Changes or additions to the Command Line Interface
  • [Misc]: For changes not covered above (use sparingly)

Note: For changes spanning multiple categories, use multiple prefixes in order of importance.

Submission Checklist

  • PR title includes appropriate prefix(es)
  • Changes are clearly explained in the PR description
  • New and existing tests pass successfully
  • Code adheres to project style and best practices
  • Documentation updated to reflect changes (if applicable)
  • Thorough testing completed, no regressions introduced

By submitting this PR, you confirm that you've read these guidelines and your changes align with the project's contribution standards.

Jeffwan added 5 commits April 30, 2025 14:07
Signed-off-by: Jiaxin Shan <seedjeffwan@gmail.com>
Signed-off-by: Jiaxin Shan <seedjeffwan@gmail.com>
Signed-off-by: Jiaxin Shan <seedjeffwan@gmail.com>
Signed-off-by: Jiaxin Shan <seedjeffwan@gmail.com>
Signed-off-by: Jiaxin Shan <seedjeffwan@gmail.com>
@Jeffwan Jeffwan force-pushed the jiaxin/kvcache-infinistore-support branch 2 times, most recently from 2dfaaad to c47eaf5 Compare May 1, 2025 17:13
@Jeffwan Jeffwan requested review from Copilot and varungup90 May 2, 2025 17:33
@Jeffwan Jeffwan assigned DwyaneShi and unassigned DwyaneShi May 2, 2025
@Jeffwan Jeffwan requested a review from DwyaneShi May 2, 2025 17:33
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the kvcache backend to support infinistore by replacing the centralized/distributed controller with a KVBackendReconciler interface, building a common abstract DistributedReconciler for HPKV and Infinistore, and adding comprehensive test coverage and refactored common helpers.

  • Removed centralized and distributed controller logic and replaced it with a per-backend reconciler design.
  • Introduced a new DistributedReconciler, updated interfaces, and refactored helper functions and constants.
  • Added extensive test cases for annotation processing, backend validation, and resource construction for each supported backend.

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pkg/utils/annotations.go & annotations_test.go Added helper functions to extract annotations and tests verifying their correctness.
pkg/controller/kvcache/kvcache_controller.go Refactored the controller logic to derive the backend dynamically and delegate reconciliation to backend-specific handlers.
pkg/controller/kvcache/backends/* Introduced separate backend modules for Vineyard, HPKV, and Infinistore with their corresponding tests.
pkg/constants/kvcache.go Updated constants to support the new backend names and refactored labeling conventions.
Other test files (e.g. infinistore_test.go, hpkv_test.go, distributed_test.go, common_test.go) Added new test cases for validating the construction and reconciliation of backend-specific resources.

@Jeffwan Jeffwan force-pushed the jiaxin/kvcache-infinistore-support branch from c47eaf5 to e9fbd4b Compare May 2, 2025 23:44
Signed-off-by: Jiaxin Shan <seedjeffwan@gmail.com>
@Jeffwan Jeffwan force-pushed the jiaxin/kvcache-infinistore-support branch from e9fbd4b to fcda7cf Compare May 3, 2025 00:35
Jeffwan added 3 commits May 4, 2025 11:49
Signed-off-by: Jiaxin Shan <seedjeffwan@gmail.com>
Signed-off-by: Jiaxin Shan <seedjeffwan@gmail.com>
Signed-off-by: Jiaxin Shan <seedjeffwan@gmail.com>
@Jeffwan Jeffwan force-pushed the jiaxin/kvcache-infinistore-support branch from 8a2b32e to 930deba Compare May 5, 2025 01:22
@Jeffwan
Copy link
Collaborator Author

Jeffwan commented May 6, 2025

I will merge this change to unblock another one. If someone has more feedback, please left comment here and I will address them in next PR

@Jeffwan Jeffwan merged commit c603ae0 into vllm-project:main May 6, 2025
13 checks passed
@Jeffwan Jeffwan deleted the jiaxin/kvcache-infinistore-support branch May 6, 2025 16:59
Yaegaki1Erika pushed a commit to Yaegaki1Erika/aibrix that referenced this pull request Jul 23, 2025
* Remove predicates and prepare for deletion event handling
* Add kvcache backend package
* Refactor orchestration logic to kvcache backends
* Further abstract the distributed reconciler
* Add KVBackend Concept for extensibility
* Support kv cache backend in watcher
* Fix unit tests from last change
* Fix service port mapping issues
---------

Signed-off-by: Jiaxin Shan <seedjeffwan@gmail.com>
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.

3 participants