[router][thin-client] Add getAllStoreNames capability via /stores endpoint by minhmo1620 · Pull Request #2704 · linkedin/venice

minhmo1620 · 2026-04-07T20:31:04Z

Problem Statement

No way to query all store names across clusters from the thin client without ControllerClient (cluster-specific). HelixReadOnlyStoreConfigRepository already caches this data globally but it wasn't exposed via the router.

Solution

Add getStores(boolean includeSystemStores) to ReadOnlyStoreConfigRepository and implement in HelixReadOnlyStoreConfigRepository with system store filtering (venice_system_store_ prefix).
Add TYPE_STORES("stores") to RouterResourceType and handle in MetaDataHandler.handleStoresLookup(), returning a MultiStoreResponse with regular stores only.
Add StoreMetadataFetcher interface and RouterBasedStoreMetadataFetcher that calls GET /stores and deserializes the response.
Add ClientFactory.createStoreMetadataFetcher(ClientConfig) factory method.

D2 interaction

The fetcher takes a D2Client + d2ServiceName, constructs its own D2TransportClient (owns the lifecycle), and calls GET d2://<d2ServiceName>/stores directly. D2 resolves the service name via ZooKeeper, load-balances across live router instances, and the router returns a MultiStoreResponse.

We use D2TransportClient directly rather than AbstractAvroStoreClient because getRaw() unconditionally calls discoverD2Service(), hitting discover_cluster/<storeName> on the router. With no store name this becomes discover_cluster/null → 404 → VeniceNoStoreException after 10 retries.

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.

Concurrency-Specific Checks

Both reviewer and PR author to verify

Code has no race conditions or thread safety issues.
Proper synchronization mechanisms (e.g., synchronized, RWLock) are used where needed.
No blocking calls inside critical sections that could lead to deadlocks or performance degradation.
Verified thread-safe collections are used (e.g., ConcurrentHashMap, CopyOnWriteArrayList).
Validated proper exception handling in multi-threaded code to avoid silent thread termination.

How was this PR tested?

New unit tests: RouterBasedStoreMetadataFetcherTest (happy path, empty stores, router error, null response, null body, transport failure, close), TestHelixReadOnlyStoreConfigRepository (getStores with/without system stores).
New integration test: TestRouterBasedStoreMetadataFetcher — creates 2 stores via VeniceTwoLayerMultiRegionMultiClusterWrapper over D2, verifies getAllStoreNames() returns both.
Extended existing tests: TestMetaDataHandler updated for /stores endpoint.

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

Yes. New StoreMetadataFetcher API accessible via ClientFactory.createStoreMetadataFetcher().

endpoint - Add TYPE_STORE_NAMES router resource type and handle it in MetaDataHandler using HelixReadOnlyStoreConfigRepository.getAvailableStoreNames() - Add getAvailableStoreNames() to ReadOnlyStoreConfigRepository interface with implementation in HelixReadOnlyStoreConfigRepository - Move MultiStoreResponse from venice-common to venice-client-common so venice-thin-client can use it without a compile-scope dependency on venice-common - Add StoreMetadataFetcher interface and RouterBasedStoreMetadataFetcher impl that calls the /store_names router endpoint - Add ClientFactory.createStoreMetadataFetcher() factory method mirroring createStoreSchemaFetcher() - Add unit tests for RouterBasedStoreMetadataFetcher and TestMetaDataHandler - Add integration test TestRouterBasedStoreMetadataFetcher using VeniceClusterWrapper Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…outer store propagation The router's HelixReadOnlyStoreConfigRepository updates its store set asynchronously via ZK watches, so getAllStoreNames() may not reflect newly created stores immediately. Wrap the assertion in waitForNonDeterministicAssertion to handle the propagation delay. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…efore querying Add empty pushes so stores have a version before asserting router visibility, rename test to testGetAllStoreNames, and use assertCommand for store creation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

TestRouterBasedStoreMetadataFetcher Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

TestRouterBasedStoreMetadataFetcher Set up a D2Client from the cluster's ZK address and use D2-based routing for both the generic avro clients and the StoreMetadataFetcher, matching the pattern used by other integration tests (e.g. TestDaVinciRequestBasedMetaRepository). Also bump the test timeout to 120s to account for two empty pushes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…m stores - Rename TYPE_STORE_NAMES -> TYPE_STORES and endpoint /store_names -> /stores - Replace getAvailableStoreNames() with getStores(boolean includeSystemStores) on ReadOnlyStoreConfigRepository, filtering out system stores by default - Update RouterBasedStoreMetadataFetcher, MetaDataHandler, and all tests Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Cover both includeSystemStores=true and includeSystemStores=false branches. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

RouterStoreAclHandlerTest Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Adds a router-backed mechanism for thin clients to fetch all store names across clusters via a new /stores metadata endpoint.

Changes:

Extend ReadOnlyStoreConfigRepository with getStores(includeSystemStores) and implement system-store filtering in HelixReadOnlyStoreConfigRepository.
Add router /stores endpoint (RouterResourceType.TYPE_STORES) returning a MultiStoreResponse.
Introduce thin-client StoreMetadataFetcher + RouterBasedStoreMetadataFetcher, plus unit/integration test coverage and a ClientFactory constructor.

Reviewed changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
services/venice-router/src/test/java/com/linkedin/venice/router/TestMetaDataHandler.java	Adds unit tests validating `/stores` response and empty-store behavior.
services/venice-router/src/test/java/com/linkedin/venice/router/acl/RouterStoreAclHandlerTest.java	Updates request-type coverage to include `TYPE_STORES`.
services/venice-router/src/main/java/com/linkedin/venice/router/MetaDataHandler.java	Implements `/stores` handling and returns `MultiStoreResponse`.
services/venice-router/src/main/java/com/linkedin/venice/router/api/RouterResourceType.java	Adds `TYPE_STORES("stores")` to router resource types.
internal/venice-test-common/src/integrationTest/java/com/linkedin/venice/client/store/TestRouterBasedStoreMetadataFetcher.java	Adds integration test for fetching store names via router over D2.
internal/venice-common/src/test/java/com/linkedin/venice/helix/TestHelixReadOnlyStoreConfigRepository.java	Adds tests for including/excluding system stores in `getStores`.
internal/venice-common/src/main/java/com/linkedin/venice/meta/ReadOnlyStoreConfigRepository.java	Adds `getStores(boolean includeSystemStores)` to the repository interface.
internal/venice-common/src/main/java/com/linkedin/venice/helix/HelixReadOnlyStoreViewConfigRepositoryAdapter.java	Delegates the new `getStores` API to the underlying repository.
internal/venice-common/src/main/java/com/linkedin/venice/helix/HelixReadOnlyStoreConfigRepository.java	Implements `getStores` and filters system stores by prefix.
internal/venice-client-common/src/main/java/com/linkedin/venice/controllerapi/MultiStoreResponse.java	Adds response model used by router + thin client for `/stores`.
clients/venice-thin-client/src/test/java/com/linkedin/venice/client/store/RouterBasedStoreMetadataFetcherTest.java	Adds unit tests for router-based store-name fetching and error cases.
clients/venice-thin-client/src/main/java/com/linkedin/venice/client/store/StoreMetadataFetcher.java	Introduces public interface for non-store/cluster-scoped metadata fetches.
clients/venice-thin-client/src/main/java/com/linkedin/venice/client/store/RouterBasedStoreMetadataFetcher.java	Implements `getAllStoreNames()` via `GET /stores` and JSON deserialization.
clients/venice-thin-client/src/main/java/com/linkedin/venice/client/store/ClientFactory.java	Adds `createStoreMetadataFetcher(ClientConfig)` factory method.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

xunyin8

Overall lgtm just one concern.

@afterclass

…etcher - Replace injected TransportClient with owned D2TransportClient; make TransportClient constructor package-private (VisibleForTesting) so the fetcher is always responsible for its own client lifecycle - Add null check for response body before JSON deserialization - Add unit test for null response body case - Close parentControllerClient in @afterclass of integration test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…Fetcher Mirror the RouterBasedStoreSchemaFetcher pattern: accept an injected AbstractAvroStoreClient and call getRaw() with retry instead of owning a TransportClient directly. The caller retains ownership of the client lifecycle so close() is a no-op. Update ClientFactory and unit tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

RouterBasedStoreMetadataFetcher AbstractAvroStoreClient.getRaw() always calls discoverD2Service(), which hits discover_cluster/<storeName> on the router. Since this fetcher is cluster-agnostic and has no store name, that discovery request becomes discover_cluster/null, causing a VeniceNoStoreException after 10 retries. Fix: bypass AbstractAvroStoreClient and use D2TransportClient directly. The fetcher constructs its own D2TransportClient from the passed-in D2Client and d2ServiceName, so it owns the lifecycle and closes it properly. The package-private TransportClient constructor is kept for unit tests. Restore full unit test coverage including null body and close. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 14 out of 15 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…ore set - Add 30s timeout to transportClient.get() in RouterBasedStoreMetadataFetcher to prevent indefinite hangs on network stalls; include path in exception message - Add timeout unit test - Cache filtered regular-store set in HelixReadOnlyStoreConfigRepository so getStores(false) is O(1) with no per-call allocation; kept in sync via refresh() and handleChildChange() Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

xunyin8

LGTM thanks!

Restore the design pulled in from linkedin#2704 discussion r3066176915: the public RouterBasedStoreMetadataFetcher constructor takes inputs the fetcher uses to build its own TransportClient (now ClientConfig instead of the original (D2Client, String)), so the fetcher owns the transport and close() is always safe. The (TransportClient) ctor becomes package-private VisibleForTesting again, matching the original intent — callers cannot pass an externally-owned transport that the fetcher would then close.

The earlier "to avoid store-level D2 service discovery" wording was a holdover from linkedin#2704 and no longer matches the design — D2 is now just one of three supported transports. The actual reason for the direct TransportClient is that AbstractAvroStoreClient is store-scoped while /stores is cluster-agnostic. Reword and drop the stale D2 framing.

Restore the original public constructor signature from linkedin#2704 as a thin delegate that constructs and owns its own D2TransportClient. This keeps the API binary- and source-compatible for any caller that wired up RouterBasedStoreMetadataFetcher directly with a D2 client, while the new (ClientConfig) overload remains the preferred entry point for new code (it supports HTTP/HTTPS routing too).

Minh Nguyen and others added 10 commits April 7, 2026 13:17

[test] Remove redundant duplicate getNewStore call for storeName2

2072c81

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

[test] Use ClientFactory.createStoreMetadataFetcher in

a524b49

TestRouterBasedStoreMetadataFetcher Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Revert accidental blobDbEnabled comment-out in integration tests

140721a

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

[router] Fix testStoreNamesLookup to use /stores endpoint

fac94bd

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

[test] Add unit tests for HelixReadOnlyStoreConfigRepository.getStores()

f48e9ca

Cover both includeSystemStores=true and includeSystemStores=false branches. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

minhmo1620 changed the title ~~[router][thin-client] Add getAllStoreNames capability via /store_names~~ [router][thin-client] Add getAllStoreNames capability via /stores endpoint Apr 8, 2026

Minh Nguyen and others added 2 commits April 8, 2026 10:47

[router][test] Handle TYPE_STORES in RouterStoreAclHandlerTest

aec5425

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

[router][test] Fix SpotBugs duplicate switch clause for TYPE_STORES in

9b04f5b

RouterStoreAclHandlerTest Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

minhmo1620 marked this pull request as ready for review April 8, 2026 18:16

minhmo1620 requested review from Copilot and xunyin8 April 9, 2026 17:28

Copilot started reviewing on behalf of minhmo1620 April 10, 2026 17:47 View session

Copilot AI reviewed Apr 10, 2026

View reviewed changes

Comment thread ...n-client/src/main/java/com/linkedin/venice/client/store/RouterBasedStoreMetadataFetcher.java

Comment thread ...tegrationTest/java/com/linkedin/venice/client/store/TestRouterBasedStoreMetadataFetcher.java

xunyin8 reviewed Apr 10, 2026

View reviewed changes

Comment thread ...n-client/src/main/java/com/linkedin/venice/client/store/RouterBasedStoreMetadataFetcher.java

Minh Nguyen and others added 2 commits April 10, 2026 12:09

Copilot AI review requested due to automatic review settings April 10, 2026 19:56

Copilot started reviewing on behalf of minhmo1620 April 10, 2026 19:57 View session

Copilot AI reviewed Apr 10, 2026

View reviewed changes

Comment thread clients/venice-thin-client/src/main/java/com/linkedin/venice/client/store/ClientFactory.java

Comment thread ...n-client/src/main/java/com/linkedin/venice/client/store/RouterBasedStoreMetadataFetcher.java Outdated

Minh Nguyen and others added 2 commits April 10, 2026 13:13

[docs] Document /stores endpoint in router-api.md

adf5354

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings April 10, 2026 20:52

Copilot started reviewing on behalf of minhmo1620 April 10, 2026 20:52 View session

Copilot AI reviewed Apr 10, 2026

View reviewed changes

Comment thread ...n-client/src/main/java/com/linkedin/venice/client/store/RouterBasedStoreMetadataFetcher.java

Comment thread ...enice-common/src/main/java/com/linkedin/venice/helix/HelixReadOnlyStoreConfigRepository.java Outdated

Minh Nguyen and others added 2 commits April 10, 2026 14:30

trigger CI rerun

2eaf6eb

[docs] Revert /stores endpoint documentation to fix CI

a7b97bf

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings April 10, 2026 21:54

Copilot started reviewing on behalf of minhmo1620 April 10, 2026 21:55 View session

Copilot AI reviewed Apr 10, 2026

View reviewed changes

trigger CI rerun

cde1863

xunyin8 approved these changes Apr 11, 2026

View reviewed changes

minhmo1620 merged commit 413d87d into linkedin:main Apr 11, 2026
106 checks passed

minhmo1620 mentioned this pull request May 11, 2026

[thin-client] Support HTTP/HTTPS routing in createStoreMetadataFetcher #2788

Merged

14 tasks

Navigation Menu

Uh oh!

[router][thin-client] Add getAllStoreNames capability via /stores endpoint - #2704

[router][thin-client] Add getAllStoreNames capability via /stores endpoint#2704
minhmo1620 merged 20 commits into
linkedin:mainfrom
minhmo1620:minnguye/store-metadata-fetcher

minhmo1620 commented Apr 7, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

xunyin8 left a comment •

edited

Loading

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xunyin8 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

minhmo1620 commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem Statement

Solution

D2 interaction

Code changes

Concurrency-Specific Checks

How was this PR tested?

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

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

xunyin8 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xunyin8 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

minhmo1620 commented Apr 7, 2026 •

edited

Loading

xunyin8 left a comment •

edited

Loading