Skip to content

Commit 6714224

Browse files
pthirunclaude
pthirun
and
claude
committed
fix(controller): align proto message names for getClusterHealthStores API
Rename proto message types to follow Venice gRPC naming conventions: - GetStoreStatusRequest -> GetClusterHealthStoresGrpcRequest - GetStoreStatusResponse -> GetClusterHealthStoresGrpcResponse Change response field from repeated StoreStatus list to map<string, string> to match the existing HTTP endpoint response format and test expectations. Update StoreGrpcServiceImpl and StoreGrpcServiceImplTest to use the renamed message types and the map-based response structure. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 56eafff commit 6714224

3 files changed

Lines changed: 27 additions & 40 deletions

File tree

‎internal/venice-common/src/main/proto/controller/StoreGrpcService.proto‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ service StoreGrpcService {
1414
rpc checkResourceCleanupForStoreCreation(ClusterStoreGrpcInfo) returns (ResourceCleanupCheckGrpcResponse) {}
1515
rpc validateStoreDeleted(ValidateStoreDeletedGrpcRequest) returns (ValidateStoreDeletedGrpcResponse);
1616
rpc listStores(ListStoresGrpcRequest) returns (ListStoresGrpcResponse);
17-
rpc getClusterHealthStores(GetStoreStatusRequest) returns (GetStoreStatusResponse);
17+
rpc getClusterHealthStores(GetClusterHealthStoresGrpcRequest) returns (GetClusterHealthStoresGrpcResponse);
1818
}
1919

2020
message CreateStoreGrpcRequest {
@@ -85,16 +85,11 @@ message ListStoresGrpcResponse {
8585
repeated string storeNames = 2;
8686
}
8787

88-
message GetStoreStatusRequest {
88+
message GetClusterHealthStoresGrpcRequest {
8989
string clusterName = 1;
9090
}
9191

92-
message StoreStatus {
93-
string storeName = 1;
94-
string status = 2;
95-
}
96-
97-
message GetStoreStatusResponse {
92+
message GetClusterHealthStoresGrpcResponse {
9893
string clusterName = 1;
99-
repeated StoreStatus storeStatuses = 2;
94+
map<string, string> storeStatusMap = 2;
10095
}

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

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,13 @@
1515
import com.linkedin.venice.protocols.controller.DeleteAclForStoreGrpcResponse;
1616
import com.linkedin.venice.protocols.controller.GetAclForStoreGrpcRequest;
1717
import com.linkedin.venice.protocols.controller.GetAclForStoreGrpcResponse;
18-
import com.linkedin.venice.protocols.controller.GetStoreStatusRequest;
19-
import com.linkedin.venice.protocols.controller.GetStoreStatusResponse;
18+
import com.linkedin.venice.protocols.controller.GetClusterHealthStoresGrpcRequest;
19+
import com.linkedin.venice.protocols.controller.GetClusterHealthStoresGrpcResponse;
2020
import com.linkedin.venice.protocols.controller.ListStoresGrpcRequest;
2121
import com.linkedin.venice.protocols.controller.ListStoresGrpcResponse;
2222
import com.linkedin.venice.protocols.controller.ResourceCleanupCheckGrpcResponse;
2323
import com.linkedin.venice.protocols.controller.StoreGrpcServiceGrpc;
2424
import com.linkedin.venice.protocols.controller.StoreGrpcServiceGrpc.StoreGrpcServiceImplBase;
25-
import com.linkedin.venice.protocols.controller.StoreStatus;
2625
import com.linkedin.venice.protocols.controller.UpdateAclForStoreGrpcRequest;
2726
import com.linkedin.venice.protocols.controller.UpdateAclForStoreGrpcResponse;
2827
import com.linkedin.venice.protocols.controller.ValidateStoreDeletedGrpcRequest;
@@ -158,21 +157,18 @@ public void listStores(ListStoresGrpcRequest grpcRequest, StreamObserver<ListSto
158157
*/
159158
@Override
160159
public void getClusterHealthStores(
161-
GetStoreStatusRequest grpcRequest,
162-
StreamObserver<GetStoreStatusResponse> responseObserver) {
160+
GetClusterHealthStoresGrpcRequest grpcRequest,
161+
StreamObserver<GetClusterHealthStoresGrpcResponse> responseObserver) {
163162
LOGGER.debug("Received getClusterHealthStores with args: {}", grpcRequest);
164163
String clusterName = grpcRequest.getClusterName();
165164
handleRequest(StoreGrpcServiceGrpc.getGetClusterHealthStoresMethod(), () -> {
166165
MultiStoreStatusResponse response = storeRequestHandler.getClusterHealthStores(clusterName);
167166

168-
// Convert map to repeated StoreStatus entries
169-
GetStoreStatusResponse.Builder responseBuilder =
170-
GetStoreStatusResponse.newBuilder().setClusterName(response.getCluster());
167+
GetClusterHealthStoresGrpcResponse.Builder responseBuilder =
168+
GetClusterHealthStoresGrpcResponse.newBuilder().setClusterName(response.getCluster());
171169

172170
if (response.getStoreStatusMap() != null) {
173-
response.getStoreStatusMap().forEach((storeName, status) -> {
174-
responseBuilder.addStoreStatuses(StoreStatus.newBuilder().setStoreName(storeName).setStatus(status).build());
175-
});
171+
responseBuilder.putAllStoreStatusMap(response.getStoreStatusMap());
176172
}
177173

178174
return responseBuilder.build();

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

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,13 @@
2626
import com.linkedin.venice.protocols.controller.DeleteAclForStoreGrpcResponse;
2727
import com.linkedin.venice.protocols.controller.GetAclForStoreGrpcRequest;
2828
import com.linkedin.venice.protocols.controller.GetAclForStoreGrpcResponse;
29-
import com.linkedin.venice.protocols.controller.GetStoreStatusRequest;
30-
import com.linkedin.venice.protocols.controller.GetStoreStatusResponse;
29+
import com.linkedin.venice.protocols.controller.GetClusterHealthStoresGrpcRequest;
30+
import com.linkedin.venice.protocols.controller.GetClusterHealthStoresGrpcResponse;
3131
import com.linkedin.venice.protocols.controller.ListStoresGrpcRequest;
3232
import com.linkedin.venice.protocols.controller.ListStoresGrpcResponse;
3333
import com.linkedin.venice.protocols.controller.ResourceCleanupCheckGrpcResponse;
3434
import com.linkedin.venice.protocols.controller.StoreGrpcServiceGrpc;
3535
import com.linkedin.venice.protocols.controller.StoreGrpcServiceGrpc.StoreGrpcServiceBlockingStub;
36-
import com.linkedin.venice.protocols.controller.StoreStatus;
3736
import com.linkedin.venice.protocols.controller.UpdateAclForStoreGrpcRequest;
3837
import com.linkedin.venice.protocols.controller.UpdateAclForStoreGrpcResponse;
3938
import com.linkedin.venice.protocols.controller.ValidateStoreDeletedGrpcRequest;
@@ -454,7 +453,8 @@ public void testListStoresWithFilters() {
454453

455454
@Test
456455
public void testGetClusterHealthStoresReturnsSuccessfulResponse() {
457-
GetStoreStatusRequest request = GetStoreStatusRequest.newBuilder().setClusterName(TEST_CLUSTER).build();
456+
GetClusterHealthStoresGrpcRequest request =
457+
GetClusterHealthStoresGrpcRequest.newBuilder().setClusterName(TEST_CLUSTER).build();
458458
Map<String, String> storeStatusMap = new HashMap<>();
459459
storeStatusMap.put("store1", "ONLINE");
460460
storeStatusMap.put("store2", "DEGRADED");
@@ -464,40 +464,36 @@ public void testGetClusterHealthStoresReturnsSuccessfulResponse() {
464464
handlerResponse.setStoreStatusMap(storeStatusMap);
465465
when(storeRequestHandler.getClusterHealthStores(TEST_CLUSTER)).thenReturn(handlerResponse);
466466

467-
GetStoreStatusResponse actualResponse = blockingStub.getClusterHealthStores(request);
467+
GetClusterHealthStoresGrpcResponse actualResponse = blockingStub.getClusterHealthStores(request);
468468

469469
assertNotNull(actualResponse, "Response should not be null");
470470
assertEquals(actualResponse.getClusterName(), TEST_CLUSTER, "Cluster name should match");
471-
assertEquals(actualResponse.getStoreStatusesCount(), 3, "Should have 3 stores");
472-
473-
// Verify each store status
474-
Map<String, String> responseMap = new HashMap<>();
475-
for (StoreStatus status: actualResponse.getStoreStatusesList()) {
476-
responseMap.put(status.getStoreName(), status.getStatus());
477-
}
478-
assertEquals(responseMap.get("store1"), "ONLINE", "store1 should be ONLINE");
479-
assertEquals(responseMap.get("store2"), "DEGRADED", "store2 should be DEGRADED");
480-
assertEquals(responseMap.get("store3"), "UNAVAILABLE", "store3 should be UNAVAILABLE");
471+
assertEquals(actualResponse.getStoreStatusMapMap().size(), 3, "Should have 3 stores");
472+
assertEquals(actualResponse.getStoreStatusMapMap().get("store1"), "ONLINE", "store1 should be ONLINE");
473+
assertEquals(actualResponse.getStoreStatusMapMap().get("store2"), "DEGRADED", "store2 should be DEGRADED");
474+
assertEquals(actualResponse.getStoreStatusMapMap().get("store3"), "UNAVAILABLE", "store3 should be UNAVAILABLE");
481475
}
482476

483477
@Test
484478
public void testGetClusterHealthStoresReturnsEmptyMapWhenNoStores() {
485-
GetStoreStatusRequest request = GetStoreStatusRequest.newBuilder().setClusterName(TEST_CLUSTER).build();
479+
GetClusterHealthStoresGrpcRequest request =
480+
GetClusterHealthStoresGrpcRequest.newBuilder().setClusterName(TEST_CLUSTER).build();
486481
MultiStoreStatusResponse handlerResponse = new MultiStoreStatusResponse();
487482
handlerResponse.setCluster(TEST_CLUSTER);
488483
handlerResponse.setStoreStatusMap(new HashMap<>());
489484
when(storeRequestHandler.getClusterHealthStores(TEST_CLUSTER)).thenReturn(handlerResponse);
490485

491-
GetStoreStatusResponse actualResponse = blockingStub.getClusterHealthStores(request);
486+
GetClusterHealthStoresGrpcResponse actualResponse = blockingStub.getClusterHealthStores(request);
492487

493488
assertNotNull(actualResponse, "Response should not be null");
494489
assertEquals(actualResponse.getClusterName(), TEST_CLUSTER, "Cluster name should match");
495-
assertEquals(actualResponse.getStoreStatusesCount(), 0, "Should have 0 stores");
490+
assertEquals(actualResponse.getStoreStatusMapMap().size(), 0, "Should have 0 stores");
496491
}
497492

498493
@Test
499494
public void testGetClusterHealthStoresReturnsErrorResponse() {
500-
GetStoreStatusRequest request = GetStoreStatusRequest.newBuilder().setClusterName(TEST_CLUSTER).build();
495+
GetClusterHealthStoresGrpcRequest request =
496+
GetClusterHealthStoresGrpcRequest.newBuilder().setClusterName(TEST_CLUSTER).build();
501497
when(storeRequestHandler.getClusterHealthStores(TEST_CLUSTER))
502498
.thenThrow(new VeniceException("Failed to get cluster health stores"));
503499

@@ -514,7 +510,7 @@ public void testGetClusterHealthStoresReturnsErrorResponse() {
514510

515511
@Test
516512
public void testGetClusterHealthStoresReturnsBadRequestForMissingClusterName() {
517-
GetStoreStatusRequest request = GetStoreStatusRequest.newBuilder().build();
513+
GetClusterHealthStoresGrpcRequest request = GetClusterHealthStoresGrpcRequest.newBuilder().build();
518514
when(storeRequestHandler.getClusterHealthStores(""))
519515
.thenThrow(new IllegalArgumentException("Cluster name is required"));
520516

0 commit comments

Comments
 (0)