Skip to content

Commit b7e4702

Browse files
pthirunclaude
pthirun
and
claude
committed
[Controller] Migrate LIST_CHILD_CLUSTERS endpoint to gRPC
Add gRPC support for the LIST_CHILD_CLUSTERS endpoint, enabling parent controllers to list child cluster controller URLs and D2 mappings via gRPC in addition to the existing HTTP/REST interface. Changes: - Add ListChildClustersGrpcRequest/Response messages to proto file - Add listChildClusters RPC method to VeniceControllerGrpcService - Add handler method in VeniceControllerRequestHandler - Update ControllerRoutes to delegate to request handler - Add comprehensive unit tests and integration test Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 406454c commit b7e4702

8 files changed

Lines changed: 339 additions & 5 deletions

File tree

‎internal/venice-common/src/main/proto/VeniceControllerGrpcService.proto‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ service VeniceControllerGrpcService {
1414

1515
// ControllerRoutes
1616
rpc getLeaderController(LeaderControllerGrpcRequest) returns (LeaderControllerGrpcResponse);
17+
rpc listChildClusters(ListChildClustersGrpcRequest) returns (ListChildClustersGrpcResponse);
1718
}
1819

1920
message DiscoverClusterGrpcRequest {
@@ -40,3 +41,14 @@ message LeaderControllerGrpcResponse {
4041
string grpcUrl = 4; // gRPC URL for leader controller
4142
string secureGrpcUrl = 5; // Secure gRPC URL for leader controller
4243
}
44+
45+
message ListChildClustersGrpcRequest {
46+
string clusterName = 1;
47+
}
48+
49+
message ListChildClustersGrpcResponse {
50+
string clusterName = 1;
51+
map<string, string> childDataCenterControllerUrlMap = 2;
52+
map<string, string> childDataCenterControllerD2Map = 3;
53+
optional string d2ServiceName = 4;
54+
}

‎internal/venice-test-common/src/integrationTest/java/com/linkedin/venice/endToEnd/TestControllerGrpcEndpoints.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
import com.linkedin.venice.protocols.controller.GetValueSchemaGrpcResponse;
2525
import com.linkedin.venice.protocols.controller.LeaderControllerGrpcRequest;
2626
import com.linkedin.venice.protocols.controller.LeaderControllerGrpcResponse;
27+
import com.linkedin.venice.protocols.controller.ListChildClustersGrpcRequest;
28+
import com.linkedin.venice.protocols.controller.ListChildClustersGrpcResponse;
2729
import com.linkedin.venice.protocols.controller.ListStoresGrpcRequest;
2830
import com.linkedin.venice.protocols.controller.ListStoresGrpcResponse;
2931
import com.linkedin.venice.protocols.controller.SchemaGrpcServiceGrpc;
@@ -386,6 +388,25 @@ public void testGetValueSchemaGrpcEndpoint() {
386388
assertEquals(exception.getStatus().getCode(), io.grpc.Status.Code.INVALID_ARGUMENT);
387389
}
388390

391+
@Test(timeOut = TIMEOUT_MS)
392+
public void testListChildClustersGrpcEndpoint() {
393+
String controllerGrpcUrl = veniceCluster.getLeaderVeniceController().getControllerGrpcUrl();
394+
ManagedChannel channel = Grpc.newChannelBuilder(controllerGrpcUrl, InsecureChannelCredentials.create()).build();
395+
VeniceControllerGrpcServiceBlockingStub blockingStub = VeniceControllerGrpcServiceGrpc.newBlockingStub(channel);
396+
397+
// Test listChildClusters - for a non-parent controller, it should return empty maps
398+
ListChildClustersGrpcRequest request =
399+
ListChildClustersGrpcRequest.newBuilder().setClusterName(veniceCluster.getClusterName()).build();
400+
401+
ListChildClustersGrpcResponse response = blockingStub.listChildClusters(request);
402+
assertNotNull(response, "Response should not be null");
403+
assertEquals(response.getClusterName(), veniceCluster.getClusterName(), "Cluster name should match");
404+
// Non-parent controllers return empty maps
405+
assertEquals(response.getChildDataCenterControllerUrlMapCount(), 0, "URL map should be empty for non-parent");
406+
assertEquals(response.getChildDataCenterControllerD2MapCount(), 0, "D2 map should be empty for non-parent");
407+
assertFalse(response.hasD2ServiceName(), "D2 service name should not be set for non-parent");
408+
}
409+
389410
private static class MockDynamicAccessController extends NoOpDynamicAccessController {
390411
private final Set<String> resourcesInAllowList = ConcurrentHashMap.newKeySet();
391412

‎services/venice-controller/src/main/java/com/linkedin/venice/controller/server/ControllerRoutes.java‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
import com.linkedin.venice.exceptions.ErrorType;
2828
import com.linkedin.venice.protocols.controller.LeaderControllerGrpcRequest;
2929
import com.linkedin.venice.protocols.controller.LeaderControllerGrpcResponse;
30+
import com.linkedin.venice.protocols.controller.ListChildClustersGrpcRequest;
31+
import com.linkedin.venice.protocols.controller.ListChildClustersGrpcResponse;
3032
import com.linkedin.venice.pubsub.PubSubTopicConfiguration;
3133
import com.linkedin.venice.pubsub.PubSubTopicRepository;
3234
import com.linkedin.venice.pubsub.api.PubSubTopic;
@@ -105,12 +107,21 @@ public void internalHandle(Request request, ChildAwareResponse veniceResponse) {
105107
AdminSparkServer.validateParams(request, LIST_CHILD_CLUSTERS.getParams(), admin);
106108
String clusterName = request.queryParams(CLUSTER);
107109

108-
veniceResponse.setCluster(clusterName);
110+
ListChildClustersGrpcRequest grpcRequest =
111+
ListChildClustersGrpcRequest.newBuilder().setClusterName(clusterName).build();
112+
ListChildClustersGrpcResponse grpcResponse = requestHandler.listChildClusters(grpcRequest);
109113

110-
if (admin.isParent()) {
111-
veniceResponse.setChildDataCenterControllerUrlMap(admin.getChildDataCenterControllerUrlMap(clusterName));
112-
veniceResponse.setChildDataCenterControllerD2Map(admin.getChildDataCenterControllerD2Map(clusterName));
113-
veniceResponse.setD2ServiceName(admin.getChildControllerD2ServiceName(clusterName));
114+
veniceResponse.setCluster(grpcResponse.getClusterName());
115+
Map<String, String> childUrlMap = grpcResponse.getChildDataCenterControllerUrlMapMap();
116+
if (!childUrlMap.isEmpty()) {
117+
veniceResponse.setChildDataCenterControllerUrlMap(childUrlMap);
118+
}
119+
Map<String, String> childD2Map = grpcResponse.getChildDataCenterControllerD2MapMap();
120+
if (!childD2Map.isEmpty()) {
121+
veniceResponse.setChildDataCenterControllerD2Map(childD2Map);
122+
}
123+
if (grpcResponse.hasD2ServiceName()) {
124+
veniceResponse.setD2ServiceName(grpcResponse.getD2ServiceName());
114125
}
115126
}
116127
};

‎services/venice-controller/src/main/java/com/linkedin/venice/controller/server/VeniceControllerGrpcServiceImpl.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
import com.linkedin.venice.protocols.controller.DiscoverClusterGrpcResponse;
77
import com.linkedin.venice.protocols.controller.LeaderControllerGrpcRequest;
88
import com.linkedin.venice.protocols.controller.LeaderControllerGrpcResponse;
9+
import com.linkedin.venice.protocols.controller.ListChildClustersGrpcRequest;
10+
import com.linkedin.venice.protocols.controller.ListChildClustersGrpcResponse;
911
import com.linkedin.venice.protocols.controller.VeniceControllerGrpcServiceGrpc;
1012
import com.linkedin.venice.protocols.controller.VeniceControllerGrpcServiceGrpc.VeniceControllerGrpcServiceImplBase;
1113
import io.grpc.stub.StreamObserver;
@@ -52,4 +54,21 @@ public void discoverClusterForStore(
5254
null,
5355
grpcRequest.getStoreName());
5456
}
57+
58+
/**
59+
* Lists all child clusters for a parent controller in a multi-cluster setup.
60+
* No ACL check; any user can list child clusters.
61+
*/
62+
@Override
63+
public void listChildClusters(
64+
ListChildClustersGrpcRequest grpcRequest,
65+
StreamObserver<ListChildClustersGrpcResponse> responseObserver) {
66+
LOGGER.debug("Received listChildClusters with args: {}", grpcRequest);
67+
handleRequest(
68+
VeniceControllerGrpcServiceGrpc.getListChildClustersMethod(),
69+
() -> requestHandler.listChildClusters(grpcRequest),
70+
responseObserver,
71+
grpcRequest.getClusterName(),
72+
null);
73+
}
5574
}

‎services/venice-controller/src/main/java/com/linkedin/venice/controller/server/VeniceControllerRequestHandler.java‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
import com.linkedin.venice.protocols.controller.DiscoverClusterGrpcResponse;
88
import com.linkedin.venice.protocols.controller.LeaderControllerGrpcRequest;
99
import com.linkedin.venice.protocols.controller.LeaderControllerGrpcResponse;
10+
import com.linkedin.venice.protocols.controller.ListChildClustersGrpcRequest;
11+
import com.linkedin.venice.protocols.controller.ListChildClustersGrpcResponse;
12+
import java.util.Map;
1013
import org.apache.commons.lang.StringUtils;
1114
import org.apache.logging.log4j.LogManager;
1215
import org.apache.logging.log4j.Logger;
@@ -119,4 +122,41 @@ public DiscoverClusterGrpcResponse discoverCluster(DiscoverClusterGrpcRequest re
119122
public VeniceControllerAccessManager getControllerAccessManager() {
120123
return accessManager;
121124
}
125+
126+
/**
127+
* Lists all child clusters for a parent controller in a multi-cluster setup.
128+
* Returns empty maps for child controllers.
129+
* @param request the request containing cluster name
130+
* @return response containing child cluster controller URLs and D2 mappings
131+
*/
132+
public ListChildClustersGrpcResponse listChildClusters(ListChildClustersGrpcRequest request) {
133+
String clusterName = request.getClusterName();
134+
if (StringUtils.isBlank(clusterName)) {
135+
throw new IllegalArgumentException("Cluster name is required");
136+
}
137+
138+
LOGGER.info("Listing child clusters for cluster: {}", clusterName);
139+
140+
ListChildClustersGrpcResponse.Builder responseBuilder =
141+
ListChildClustersGrpcResponse.newBuilder().setClusterName(clusterName);
142+
143+
if (admin.isParent()) {
144+
Map<String, String> childUrlMap = admin.getChildDataCenterControllerUrlMap(clusterName);
145+
if (childUrlMap != null) {
146+
responseBuilder.putAllChildDataCenterControllerUrlMap(childUrlMap);
147+
}
148+
149+
Map<String, String> childD2Map = admin.getChildDataCenterControllerD2Map(clusterName);
150+
if (childD2Map != null) {
151+
responseBuilder.putAllChildDataCenterControllerD2Map(childD2Map);
152+
}
153+
154+
String d2ServiceName = admin.getChildControllerD2ServiceName(clusterName);
155+
if (d2ServiceName != null) {
156+
responseBuilder.setD2ServiceName(d2ServiceName);
157+
}
158+
}
159+
160+
return responseBuilder.build();
161+
}
122162
}

‎services/venice-controller/src/test/java/com/linkedin/venice/controller/server/ControllerRoutesTest.java‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import com.linkedin.venice.controller.VeniceParentHelixAdmin;
1717
import com.linkedin.venice.controllerapi.AdminOperationProtocolVersionControllerResponse;
1818
import com.linkedin.venice.controllerapi.AggregatedHealthStatusRequest;
19+
import com.linkedin.venice.controllerapi.ChildAwareResponse;
1920
import com.linkedin.venice.controllerapi.ControllerApiConstants;
2021
import com.linkedin.venice.controllerapi.LeaderControllerResponse;
2122
import com.linkedin.venice.controllerapi.StoppableNodeStatusResponse;
@@ -258,4 +259,61 @@ public void testGetAdminOperationVersionFromControllers() throws Exception {
258259
assertEquals(response.getLocalControllerName(), leaderControllerHost);
259260
assertEquals(response.getCluster(), TEST_CLUSTER);
260261
}
262+
263+
@Test
264+
public void testGetChildControllersForParentController() throws Exception {
265+
doReturn(true).when(mockAdmin).isLeaderControllerFor(anyString());
266+
doReturn(true).when(mockAdmin).isParent();
267+
268+
Map<String, String> childUrlMap = new HashMap<>();
269+
childUrlMap.put("dc1", "http://dc1-controller:8080");
270+
childUrlMap.put("dc2", "http://dc2-controller:8080");
271+
272+
Map<String, String> childD2Map = new HashMap<>();
273+
childD2Map.put("dc1", "d2://dc1-controller");
274+
childD2Map.put("dc2", "d2://dc2-controller");
275+
276+
doReturn(childUrlMap).when(mockAdmin).getChildDataCenterControllerUrlMap(TEST_CLUSTER);
277+
doReturn(childD2Map).when(mockAdmin).getChildDataCenterControllerD2Map(TEST_CLUSTER);
278+
doReturn("VeniceController").when(mockAdmin).getChildControllerD2ServiceName(TEST_CLUSTER);
279+
280+
Request request = mock(Request.class);
281+
doReturn(TEST_CLUSTER).when(request).queryParams(eq(ControllerApiConstants.CLUSTER));
282+
283+
Route childControllersRoute = new ControllerRoutes(false, Optional.empty(), pubSubTopicRepository, requestHandler)
284+
.getChildControllers(mockAdmin);
285+
ChildAwareResponse response = OBJECT_MAPPER
286+
.readValue(childControllersRoute.handle(request, mock(Response.class)).toString(), ChildAwareResponse.class);
287+
288+
assertEquals(response.getCluster(), TEST_CLUSTER);
289+
assertEquals(response.getChildDataCenterControllerUrlMap().size(), 2);
290+
assertEquals(response.getChildDataCenterControllerUrlMap().get("dc1"), "http://dc1-controller:8080");
291+
assertEquals(response.getChildDataCenterControllerUrlMap().get("dc2"), "http://dc2-controller:8080");
292+
assertEquals(response.getChildDataCenterControllerD2Map().size(), 2);
293+
assertEquals(response.getChildDataCenterControllerD2Map().get("dc1"), "d2://dc1-controller");
294+
assertEquals(response.getChildDataCenterControllerD2Map().get("dc2"), "d2://dc2-controller");
295+
assertEquals(response.getD2ServiceName(), "VeniceController");
296+
}
297+
298+
@Test
299+
public void testGetChildControllersForChildController() throws Exception {
300+
doReturn(true).when(mockAdmin).isLeaderControllerFor(anyString());
301+
doReturn(false).when(mockAdmin).isParent();
302+
303+
Request request = mock(Request.class);
304+
doReturn(TEST_CLUSTER).when(request).queryParams(eq(ControllerApiConstants.CLUSTER));
305+
306+
Route childControllersRoute = new ControllerRoutes(false, Optional.empty(), pubSubTopicRepository, requestHandler)
307+
.getChildControllers(mockAdmin);
308+
ChildAwareResponse response = OBJECT_MAPPER
309+
.readValue(childControllersRoute.handle(request, mock(Response.class)).toString(), ChildAwareResponse.class);
310+
311+
assertEquals(response.getCluster(), TEST_CLUSTER);
312+
assertTrue(
313+
response.getChildDataCenterControllerUrlMap() == null
314+
|| response.getChildDataCenterControllerUrlMap().isEmpty());
315+
assertTrue(
316+
response.getChildDataCenterControllerD2Map() == null || response.getChildDataCenterControllerD2Map().isEmpty());
317+
assertTrue(response.getD2ServiceName() == null);
318+
}
261319
}

‎services/venice-controller/src/test/java/com/linkedin/venice/controller/server/VeniceControllerGrpcServiceImplTest.java‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
import com.linkedin.venice.protocols.controller.DiscoverClusterGrpcResponse;
1818
import com.linkedin.venice.protocols.controller.LeaderControllerGrpcRequest;
1919
import com.linkedin.venice.protocols.controller.LeaderControllerGrpcResponse;
20+
import com.linkedin.venice.protocols.controller.ListChildClustersGrpcRequest;
21+
import com.linkedin.venice.protocols.controller.ListChildClustersGrpcResponse;
2022
import com.linkedin.venice.protocols.controller.VeniceControllerGrpcErrorInfo;
2123
import com.linkedin.venice.protocols.controller.VeniceControllerGrpcServiceGrpc;
2224
import com.linkedin.venice.protocols.controller.VeniceControllerGrpcServiceGrpc.VeniceControllerGrpcServiceBlockingStub;
@@ -26,6 +28,8 @@
2628
import io.grpc.StatusRuntimeException;
2729
import io.grpc.inprocess.InProcessChannelBuilder;
2830
import io.grpc.inprocess.InProcessServerBuilder;
31+
import java.util.HashMap;
32+
import java.util.Map;
2933
import org.testng.annotations.AfterMethod;
3034
import org.testng.annotations.BeforeMethod;
3135
import org.testng.annotations.Test;
@@ -176,4 +180,89 @@ public void testDiscoverClusterForStore() {
176180
assertEquals(errorInfo2.getErrorType(), ControllerGrpcErrorType.GENERAL_ERROR);
177181
assertTrue(errorInfo2.getErrorMessage().contains("Failed to discover cluster"));
178182
}
183+
184+
@Test
185+
public void testListChildClustersReturnsSuccessfulResponse() {
186+
Map<String, String> childUrlMap = new HashMap<>();
187+
childUrlMap.put("dc1", "http://dc1-controller:8080");
188+
childUrlMap.put("dc2", "http://dc2-controller:8080");
189+
Map<String, String> childD2Map = new HashMap<>();
190+
childD2Map.put("dc1", "d2://dc1-controller");
191+
childD2Map.put("dc2", "d2://dc2-controller");
192+
193+
ListChildClustersGrpcResponse response = ListChildClustersGrpcResponse.newBuilder()
194+
.setClusterName(TEST_CLUSTER)
195+
.putAllChildDataCenterControllerUrlMap(childUrlMap)
196+
.putAllChildDataCenterControllerD2Map(childD2Map)
197+
.setD2ServiceName("VeniceController")
198+
.build();
199+
doReturn(response).when(requestHandler).listChildClusters(any(ListChildClustersGrpcRequest.class));
200+
201+
ListChildClustersGrpcRequest request =
202+
ListChildClustersGrpcRequest.newBuilder().setClusterName(TEST_CLUSTER).build();
203+
ListChildClustersGrpcResponse actualResponse = blockingStub.listChildClusters(request);
204+
205+
assertNotNull(actualResponse, "Response should not be null");
206+
assertEquals(actualResponse.getClusterName(), TEST_CLUSTER, "Cluster name should match");
207+
assertEquals(actualResponse.getChildDataCenterControllerUrlMapCount(), 2, "Should have 2 URL mappings");
208+
assertEquals(
209+
actualResponse.getChildDataCenterControllerUrlMapMap().get("dc1"),
210+
"http://dc1-controller:8080",
211+
"DC1 URL should match");
212+
assertEquals(actualResponse.getChildDataCenterControllerD2MapCount(), 2, "Should have 2 D2 mappings");
213+
assertEquals(actualResponse.getD2ServiceName(), "VeniceController", "D2 service name should match");
214+
}
215+
216+
@Test
217+
public void testListChildClustersReturnsEmptyForChildController() {
218+
// Child controller returns empty maps
219+
ListChildClustersGrpcResponse response =
220+
ListChildClustersGrpcResponse.newBuilder().setClusterName(TEST_CLUSTER).build();
221+
doReturn(response).when(requestHandler).listChildClusters(any(ListChildClustersGrpcRequest.class));
222+
223+
ListChildClustersGrpcRequest request =
224+
ListChildClustersGrpcRequest.newBuilder().setClusterName(TEST_CLUSTER).build();
225+
ListChildClustersGrpcResponse actualResponse = blockingStub.listChildClusters(request);
226+
227+
assertNotNull(actualResponse, "Response should not be null");
228+
assertEquals(actualResponse.getClusterName(), TEST_CLUSTER, "Cluster name should match");
229+
assertEquals(actualResponse.getChildDataCenterControllerUrlMapCount(), 0, "Should have no URL mappings");
230+
assertEquals(actualResponse.getChildDataCenterControllerD2MapCount(), 0, "Should have no D2 mappings");
231+
assertFalse(actualResponse.hasD2ServiceName(), "Should have no D2 service name");
232+
}
233+
234+
@Test
235+
public void testListChildClustersReturnsErrorResponse() {
236+
doThrow(new VeniceException("Failed to list child clusters")).when(requestHandler)
237+
.listChildClusters(any(ListChildClustersGrpcRequest.class));
238+
239+
ListChildClustersGrpcRequest request =
240+
ListChildClustersGrpcRequest.newBuilder().setClusterName(TEST_CLUSTER).build();
241+
StatusRuntimeException e =
242+
expectThrows(StatusRuntimeException.class, () -> blockingStub.listChildClusters(request));
243+
244+
assertNotNull(e.getStatus(), "Status should not be null");
245+
assertEquals(e.getStatus().getCode(), Status.INTERNAL.getCode());
246+
VeniceControllerGrpcErrorInfo errorInfo = GrpcRequestResponseConverter.parseControllerGrpcError(e);
247+
assertNotNull(errorInfo, "Error info should not be null");
248+
assertEquals(errorInfo.getErrorType(), ControllerGrpcErrorType.GENERAL_ERROR);
249+
assertTrue(errorInfo.getErrorMessage().contains("Failed to list child clusters"));
250+
}
251+
252+
@Test
253+
public void testListChildClustersReturnsBadRequestForMissingClusterName() {
254+
doThrow(new IllegalArgumentException("Cluster name is required")).when(requestHandler)
255+
.listChildClusters(any(ListChildClustersGrpcRequest.class));
256+
257+
ListChildClustersGrpcRequest request = ListChildClustersGrpcRequest.newBuilder().build();
258+
StatusRuntimeException e =
259+
expectThrows(StatusRuntimeException.class, () -> blockingStub.listChildClusters(request));
260+
261+
assertNotNull(e.getStatus(), "Status should not be null");
262+
assertEquals(e.getStatus().getCode(), Status.INVALID_ARGUMENT.getCode());
263+
VeniceControllerGrpcErrorInfo errorInfo = GrpcRequestResponseConverter.parseControllerGrpcError(e);
264+
assertNotNull(errorInfo, "Error info should not be null");
265+
assertEquals(errorInfo.getErrorType(), ControllerGrpcErrorType.BAD_REQUEST);
266+
assertTrue(errorInfo.getErrorMessage().contains("Cluster name is required"));
267+
}
179268
}

0 commit comments

Comments
 (0)