Skip to content

[Controller] Add gRPC support for listStores API - #2413

Merged
pthirun merged 4 commits into
linkedin:mainfrom
pthirun:controller-grpc-migration-list-stores
Jan 28, 2026
Merged

[Controller] Add gRPC support for listStores API#2413
pthirun merged 4 commits into
linkedin:mainfrom
pthirun:controller-grpc-migration-list-stores

Conversation

@pthirun

@pthirun pthirun commented Jan 27, 2026

Copy link
Copy Markdown
Contributor

Problem Statement

As part of the controller gRPC migration, the listStores API needs to be available via gRPC while maintaining the existing HTTP endpoint during the migration period. This follows the same pattern established by other controller API migrations, where business logic is abstracted so both HTTP and gRPC can share the same implementation.

Solution

Added gRPC support for the listStores API following the established migration pattern:

  1. Proto definitions (StoreGrpcService.proto): Added listStores RPC method with ListStoresGrpcRequest and ListStoresGrpcResponse messages, including support for filtering (includeSystemStores, storeConfigNameFilter, storeConfigValueFilter)
  2. Request handler (StoreRequestHandler.java): Added listStores() method that validates parameters and calls admin.getAllStores() with filtering support, returning a gRPC response
  3. gRPC service implementation (StoreGrpcServiceImpl.java): Added listStores() override (no ACL check, consistent with HTTP endpoint behavior)
  4. HTTP route update (StoresRoutes.java): Updated getAllStores() to convert HTTP requests to gRPC format and delegate to the shared handler
  5. Route registration (AdminSparkServer.java): Updated to pass the request handler to the route

The implementation reuses the existing business logic in Admin.getAllStores(), ensuring consistency between HTTP and gRPC endpoints.

Code changes

  • Added new code behind a config. If so list the config names and their default values in the PR description.
  • Introduced new log lines.
    • Confirmed if logs need to be rate limited to avoid excessive logging. (Debug level logging only, consistent with other gRPC endpoints)

Concurrency-Specific Checks

Both reviewer and PR author to verify

  • Code has no race conditions or thread safety issues. (Stateless request handling, delegates to existing thread-safe Admin methods)
  • Proper synchronization mechanisms (e.g., synchronized, RWLock) are used where needed. (N/A - no shared mutable state)
  • No blocking calls inside critical sections that could lead to deadlocks or performance degradation. (N/A - no critical sections)
  • Verified thread-safe collections are used (e.g., ConcurrentHashMap, CopyOnWriteArrayList). (N/A - no collections introduced)
  • Validated proper exception handling in multi-threaded code to avoid silent thread termination. (Uses handleRequest() utility for consistent error handling)

How was this PR tested?

  • New unit tests added.
    • testListStoresReturnsSuccessfulResponse - Verifies successful response with stores list
    • testListStoresReturnsErrorResponse - Verifies error handling
    • testListStoresWithBadRequest - Verifies validation of invalid requests
    • testListStoresWithFilters - Verifies filtering functionality
    • testGetAllStores - Verifies HTTP route success case
    • testGetAllStoresReturnsEmptyList - Verifies empty list handling
    • testGetAllStoresReturnsError - Verifies HTTP error handling
    • testGetAllStoresWithFilters - Verifies HTTP filter handling
  • New integration tests added.
    • testListStoresGrpcEndpoint - End-to-end test with real cluster
  • Modified or extended existing tests.
  • Code coverage verified.
    • Generated Jacoco coverage report: ./gradlew :services:venice-controller:test :services:venice-controller:jacocoTestReport
    • Verified 100% instruction and branch coverage for all new methods
  • Verified backward compatibility (if applicable). (HTTP endpoint unchanged, gRPC is additive)

Does this PR introduce any user-facing or breaking changes?

  • No. You can skip the rest of this section.
  • Yes. Clearly explain the behavior change and its impact.

pthirun and others added 3 commits January 27, 2026 13:42
This PR migrates the LIST_STORES endpoint from HTTP to gRPC as part of the
controller gRPC migration effort.

Changes:
- Added ListStoresGrpcRequest and ListStoresGrpcResponse proto messages
- Added listStores RPC to StoreGrpcService
- Implemented listStores method in StoreRequestHandler with filtering support
- Added listStores override in StoreGrpcServiceImpl
- Updated StoresRoutes to use handler for LIST_STORES endpoint
- Added comprehensive unit tests for gRPC service and HTTP routes
- Added integration test for the new gRPC endpoint

The gRPC endpoint supports:
- Filtering by includeSystemStores flag
- Filtering by store config name/value

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Improve test coverage from 0% to 64% instruction coverage and 69% branch coverage.

Tests added:
- Basic store listing with/without system stores
- Empty list handling
- Cluster name validation
- Config filter XOR validation
- Invalid config name detection
- Data replication policy filtering (with/without matches, null policy)

Remaining uncovered paths (36%) involve complex config filters requiring
fully initialized ZKStore objects, which are adequately covered by
integration tests.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…d parameter

Update getAllStores and validateStoreDeleted methods in StoresRoutes to use
the storeRequestHandler field passed to the constructor, instead of accepting
it as a method parameter. This follows the cleaner pattern established in
SchemaRoutes and reduces complexity in route method signatures.

Changes:
- StoresRoutes: Update getAllStores(Admin, StoreRequestHandler) to getAllStores(Admin)
- StoresRoutes: Update validateStoreDeleted(Admin, StoreRequestHandler) to validateStoreDeleted(Admin)
- AdminSparkServer: Remove handler parameter from route method calls
- StoresRoutesTest: Update tests to pass handler to constructor

@sushantmane sushantmane 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.

LGTM. Thanks!

@pthirun
pthirun enabled auto-merge (squash) January 28, 2026 19:24
@pthirun
pthirun merged commit 1225a93 into linkedin:main Jan 28, 2026
50 checks passed
sushantmane pushed a commit to sushantmane/venice that referenced this pull request Jan 30, 2026
Add gRPC support for the listStores API while maintaining
  backward compatibility with the existing HTTP endpoint.

  Changes:
  - Add listStores RPC to StoreGrpcService.proto
  - Add handler method to StoreRequestHandler
  - Add gRPC service implementation (no ACL check - consistent with HTTP)
  - Update HTTP route to delegate to shared handler

  Tests:
  - Unit tests for success, error, invalid input, and filter handling
  - Integration tests for full lifecycle"
misyel pushed a commit to misyel/venice that referenced this pull request Feb 2, 2026
Add gRPC support for the listStores API while maintaining
  backward compatibility with the existing HTTP endpoint.

  Changes:
  - Add listStores RPC to StoreGrpcService.proto
  - Add handler method to StoreRequestHandler
  - Add gRPC service implementation (no ACL check - consistent with HTTP)
  - Update HTTP route to delegate to shared handler

  Tests:
  - Unit tests for success, error, invalid input, and filter handling
  - Integration tests for full lifecycle"
sushantmane pushed a commit to sushantmane/venice that referenced this pull request Feb 8, 2026
Add gRPC support for the listStores API while maintaining
  backward compatibility with the existing HTTP endpoint.

  Changes:
  - Add listStores RPC to StoreGrpcService.proto
  - Add handler method to StoreRequestHandler
  - Add gRPC service implementation (no ACL check - consistent with HTTP)
  - Update HTTP route to delegate to shared handler

  Tests:
  - Unit tests for success, error, invalid input, and filter handling
  - Integration tests for full lifecycle"
misyel pushed a commit to misyel/venice that referenced this pull request Feb 17, 2026
Add gRPC support for the listStores API while maintaining
  backward compatibility with the existing HTTP endpoint.

  Changes:
  - Add listStores RPC to StoreGrpcService.proto
  - Add handler method to StoreRequestHandler
  - Add gRPC service implementation (no ACL check - consistent with HTTP)
  - Update HTTP route to delegate to shared handler

  Tests:
  - Unit tests for success, error, invalid input, and filter handling
  - Integration tests for full lifecycle"
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.

2 participants