Skip to content

Commit d771a15

Browse files
pthirunpthirun
committed
Address review feedback: rename request/response and use repeated message
Changes per review comments: - Rename GetClusterHealthStoresGrpcRequest to GetStoreStatusRequest - Rename GetClusterHealthStoresGrpcResponse to GetStoreStatusResponse - Rename RPC method from getClusterHealthStores to getStoreStatus - Add StoreStatusEntry message type with storeName and status fields - Replace map<string, string> with repeated StoreStatusEntry - Update all implementation and test files to use new types
1 parent d4e7b5b commit d771a15

8 files changed

Lines changed: 137 additions & 126 deletions

File tree

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

Lines changed: 9 additions & 4 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(GetClusterHealthStoresGrpcRequest) returns (GetClusterHealthStoresGrpcResponse);
17+
rpc getStoreStatus(GetStoreStatusRequest) returns (GetStoreStatusResponse);
1818
}
1919

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

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

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

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

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,14 @@
1919
import com.linkedin.venice.protocols.controller.CreateStoreGrpcResponse;
2020
import com.linkedin.venice.protocols.controller.DiscoverClusterGrpcRequest;
2121
import com.linkedin.venice.protocols.controller.DiscoverClusterGrpcResponse;
22-
import com.linkedin.venice.protocols.controller.GetClusterHealthStoresGrpcRequest;
23-
import com.linkedin.venice.protocols.controller.GetClusterHealthStoresGrpcResponse;
22+
import com.linkedin.venice.protocols.controller.GetStoreStatusRequest;
23+
import com.linkedin.venice.protocols.controller.GetStoreStatusResponse;
2424
import com.linkedin.venice.protocols.controller.LeaderControllerGrpcRequest;
2525
import com.linkedin.venice.protocols.controller.LeaderControllerGrpcResponse;
2626
import com.linkedin.venice.protocols.controller.ListStoresGrpcRequest;
2727
import com.linkedin.venice.protocols.controller.ListStoresGrpcResponse;
2828
import com.linkedin.venice.protocols.controller.StoreGrpcServiceGrpc;
29+
import com.linkedin.venice.protocols.controller.StoreStatusEntry;
2930
import com.linkedin.venice.protocols.controller.ValidateStoreDeletedGrpcRequest;
3031
import com.linkedin.venice.protocols.controller.ValidateStoreDeletedGrpcResponse;
3132
import com.linkedin.venice.protocols.controller.VeniceControllerGrpcServiceGrpc;
@@ -40,6 +41,8 @@
4041
import io.grpc.ManagedChannel;
4142
import io.grpc.StatusRuntimeException;
4243
import java.security.cert.X509Certificate;
44+
import java.util.HashMap;
45+
import java.util.Map;
4346
import java.util.Properties;
4447
import java.util.Set;
4548
import java.util.concurrent.ConcurrentHashMap;
@@ -319,7 +322,7 @@ public void testListStoresGrpcEndpoint() {
319322
}
320323

321324
@Test(timeOut = TIMEOUT_MS)
322-
public void testGetClusterHealthStoresGrpcEndpoint() {
325+
public void testGetStoreStatusGrpcEndpoint() {
323326
String storeName1 = Utils.getUniqueString("test_health_stores_1");
324327
String storeName2 = Utils.getUniqueString("test_health_stores_2");
325328
String controllerGrpcUrl = veniceCluster.getLeaderVeniceController().getControllerGrpcUrl();
@@ -353,29 +356,27 @@ public void testGetClusterHealthStoresGrpcEndpoint() {
353356
CreateStoreGrpcResponse createResponse2 = storeBlockingStub.createStore(createStoreRequest2);
354357
assertNotNull(createResponse2, "Response should not be null");
355358

356-
// Step 2: Get cluster health stores
357-
GetClusterHealthStoresGrpcRequest healthRequest =
358-
GetClusterHealthStoresGrpcRequest.newBuilder().setClusterName(veniceCluster.getClusterName()).build();
359+
// Step 2: Get store status
360+
GetStoreStatusRequest statusRequest =
361+
GetStoreStatusRequest.newBuilder().setClusterName(veniceCluster.getClusterName()).build();
359362

360-
GetClusterHealthStoresGrpcResponse healthResponse = storeBlockingStub.getClusterHealthStores(healthRequest);
361-
assertNotNull(healthResponse, "Response should not be null");
362-
assertEquals(healthResponse.getClusterName(), veniceCluster.getClusterName());
363+
GetStoreStatusResponse statusResponse = storeBlockingStub.getStoreStatus(statusRequest);
364+
assertNotNull(statusResponse, "Response should not be null");
365+
assertEquals(statusResponse.getClusterName(), veniceCluster.getClusterName());
363366

364-
// Verify the stores we created are in the status map
365-
assertTrue(
366-
healthResponse.getStoreStatusMapMap().containsKey(storeName1),
367-
"Store status map should contain " + storeName1);
368-
assertTrue(
369-
healthResponse.getStoreStatusMapMap().containsKey(storeName2),
370-
"Store status map should contain " + storeName2);
367+
// Convert to map for easier assertion
368+
Map<String, String> storeStatusMap = new HashMap<>();
369+
for (StoreStatusEntry entry: statusResponse.getStoreStatusesList()) {
370+
storeStatusMap.put(entry.getStoreName(), entry.getStatus());
371+
}
372+
373+
// Verify the stores we created are in the status list
374+
assertTrue(storeStatusMap.containsKey(storeName1), "Store status should contain " + storeName1);
375+
assertTrue(storeStatusMap.containsKey(storeName2), "Store status should contain " + storeName2);
371376

372377
// Verify the statuses are not null/empty
373-
assertNotNull(
374-
healthResponse.getStoreStatusMapMap().get(storeName1),
375-
"Status for " + storeName1 + " should not be null");
376-
assertNotNull(
377-
healthResponse.getStoreStatusMapMap().get(storeName2),
378-
"Status for " + storeName2 + " should not be null");
378+
assertNotNull(storeStatusMap.get(storeName1), "Status for " + storeName1 + " should not be null");
379+
assertNotNull(storeStatusMap.get(storeName2), "Status for " + storeName2 + " should not be null");
379380
}
380381

381382
private static class MockDynamicAccessController extends NoOpDynamicAccessController {

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@
1414
import com.linkedin.venice.protocols.controller.DeleteAclForStoreGrpcResponse;
1515
import com.linkedin.venice.protocols.controller.GetAclForStoreGrpcRequest;
1616
import com.linkedin.venice.protocols.controller.GetAclForStoreGrpcResponse;
17-
import com.linkedin.venice.protocols.controller.GetClusterHealthStoresGrpcRequest;
18-
import com.linkedin.venice.protocols.controller.GetClusterHealthStoresGrpcResponse;
17+
import com.linkedin.venice.protocols.controller.GetStoreStatusRequest;
18+
import com.linkedin.venice.protocols.controller.GetStoreStatusResponse;
1919
import com.linkedin.venice.protocols.controller.ListStoresGrpcRequest;
2020
import com.linkedin.venice.protocols.controller.ListStoresGrpcResponse;
2121
import com.linkedin.venice.protocols.controller.ResourceCleanupCheckGrpcResponse;
@@ -155,14 +155,14 @@ public void listStores(ListStoresGrpcRequest grpcRequest, StreamObserver<ListSto
155155
* No ACL check; any user can query store health statuses.
156156
*/
157157
@Override
158-
public void getClusterHealthStores(
159-
GetClusterHealthStoresGrpcRequest grpcRequest,
160-
StreamObserver<GetClusterHealthStoresGrpcResponse> responseObserver) {
161-
LOGGER.debug("Received getClusterHealthStores with args: {}", grpcRequest);
158+
public void getStoreStatus(
159+
GetStoreStatusRequest grpcRequest,
160+
StreamObserver<GetStoreStatusResponse> responseObserver) {
161+
LOGGER.debug("Received getStoreStatus with args: {}", grpcRequest);
162162
String clusterName = grpcRequest.getClusterName();
163163
handleRequest(
164-
StoreGrpcServiceGrpc.getGetClusterHealthStoresMethod(),
165-
() -> storeRequestHandler.getClusterHealthStores(grpcRequest),
164+
StoreGrpcServiceGrpc.getGetStoreStatusMethod(),
165+
() -> storeRequestHandler.getStoreStatus(grpcRequest),
166166
responseObserver,
167167
clusterName,
168168
null);

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

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,11 @@
1313
import com.linkedin.venice.protocols.controller.DeleteAclForStoreGrpcResponse;
1414
import com.linkedin.venice.protocols.controller.GetAclForStoreGrpcRequest;
1515
import com.linkedin.venice.protocols.controller.GetAclForStoreGrpcResponse;
16-
import com.linkedin.venice.protocols.controller.GetClusterHealthStoresGrpcRequest;
17-
import com.linkedin.venice.protocols.controller.GetClusterHealthStoresGrpcResponse;
16+
import com.linkedin.venice.protocols.controller.GetStoreStatusRequest;
17+
import com.linkedin.venice.protocols.controller.GetStoreStatusResponse;
1818
import com.linkedin.venice.protocols.controller.ListStoresGrpcRequest;
1919
import com.linkedin.venice.protocols.controller.ListStoresGrpcResponse;
20+
import com.linkedin.venice.protocols.controller.StoreStatusEntry;
2021
import com.linkedin.venice.protocols.controller.UpdateAclForStoreGrpcRequest;
2122
import com.linkedin.venice.protocols.controller.UpdateAclForStoreGrpcResponse;
2223
import com.linkedin.venice.protocols.controller.ValidateStoreDeletedGrpcRequest;
@@ -262,9 +263,9 @@ public ListStoresGrpcResponse listStores(ListStoresGrpcRequest request) {
262263
/**
263264
* Gets the health status of all stores in the specified cluster.
264265
* @param request the request containing cluster name
265-
* @return response containing the map of store names to their statuses
266+
* @return response containing the list of store status entries
266267
*/
267-
public GetClusterHealthStoresGrpcResponse getClusterHealthStores(GetClusterHealthStoresGrpcRequest request) {
268+
public GetStoreStatusResponse getStoreStatus(GetStoreStatusRequest request) {
268269
String clusterName = request.getClusterName();
269270
if (StringUtils.isBlank(clusterName)) {
270271
throw new IllegalArgumentException("Cluster name is required");
@@ -274,9 +275,13 @@ public GetClusterHealthStoresGrpcResponse getClusterHealthStores(GetClusterHealt
274275
Map<String, String> storeStatusMap = admin.getAllStoreStatuses(clusterName);
275276
LOGGER.info("Found {} stores with health status in cluster: {}", storeStatusMap.size(), clusterName);
276277

277-
return GetClusterHealthStoresGrpcResponse.newBuilder()
278-
.setClusterName(clusterName)
279-
.putAllStoreStatusMap(storeStatusMap)
280-
.build();
278+
GetStoreStatusResponse.Builder responseBuilder = GetStoreStatusResponse.newBuilder().setClusterName(clusterName);
279+
280+
for (Map.Entry<String, String> entry: storeStatusMap.entrySet()) {
281+
responseBuilder.addStoreStatuses(
282+
StoreStatusEntry.newBuilder().setStoreName(entry.getKey()).setStatus(entry.getValue()).build());
283+
}
284+
285+
return responseBuilder.build();
281286
}
282287
}

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -109,10 +109,11 @@
109109
import com.linkedin.venice.meta.StoreInfo;
110110
import com.linkedin.venice.meta.Version;
111111
import com.linkedin.venice.protocols.controller.ClusterStoreGrpcInfo;
112-
import com.linkedin.venice.protocols.controller.GetClusterHealthStoresGrpcRequest;
113-
import com.linkedin.venice.protocols.controller.GetClusterHealthStoresGrpcResponse;
112+
import com.linkedin.venice.protocols.controller.GetStoreStatusRequest;
113+
import com.linkedin.venice.protocols.controller.GetStoreStatusResponse;
114114
import com.linkedin.venice.protocols.controller.ListStoresGrpcRequest;
115115
import com.linkedin.venice.protocols.controller.ListStoresGrpcResponse;
116+
import com.linkedin.venice.protocols.controller.StoreStatusEntry;
116117
import com.linkedin.venice.protocols.controller.ValidateStoreDeletedGrpcRequest;
117118
import com.linkedin.venice.protocols.controller.ValidateStoreDeletedGrpcResponse;
118119
import com.linkedin.venice.pubsub.PubSubTopicRepository;
@@ -231,12 +232,16 @@ public void internalHandle(Request request, MultiStoreStatusResponse veniceRespo
231232
AdminSparkServer.validateParams(request, CLUSTER_HEALTH_STORES.getParams(), admin);
232233
String clusterName = request.queryParams(CLUSTER);
233234

234-
GetClusterHealthStoresGrpcRequest grpcRequest =
235-
GetClusterHealthStoresGrpcRequest.newBuilder().setClusterName(clusterName).build();
236-
GetClusterHealthStoresGrpcResponse grpcResponse = storeRequestHandler.getClusterHealthStores(grpcRequest);
235+
GetStoreStatusRequest grpcRequest = GetStoreStatusRequest.newBuilder().setClusterName(clusterName).build();
236+
GetStoreStatusResponse grpcResponse = storeRequestHandler.getStoreStatus(grpcRequest);
237237

238238
veniceResponse.setCluster(grpcResponse.getClusterName());
239-
veniceResponse.setStoreStatusMap(grpcResponse.getStoreStatusMapMap());
239+
// Convert repeated StoreStatusEntry to Map for HTTP response
240+
Map<String, String> storeStatusMap = new HashMap<>();
241+
for (StoreStatusEntry entry: grpcResponse.getStoreStatusesList()) {
242+
storeStatusMap.put(entry.getStoreName(), entry.getStatus());
243+
}
244+
veniceResponse.setStoreStatusMap(storeStatusMap);
240245
}
241246
};
242247
}

0 commit comments

Comments
 (0)