From a966149f1dba1f8b757be5c8ec868d14850b6296 Mon Sep 17 00:00:00 2001 From: Vishal Sathyanarayana Date: Wed, 16 Oct 2024 21:50:34 -0700 Subject: [PATCH 1/8] migrate list stores to grpc: initial step --- .../controller/server/ListStoresRequest.java | 66 ++++++ .../controller/server/StoresRoutes.java | 219 ++++++++++-------- 2 files changed, 186 insertions(+), 99 deletions(-) create mode 100644 services/venice-controller/src/main/java/com/linkedin/venice/controller/server/ListStoresRequest.java diff --git a/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/ListStoresRequest.java b/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/ListStoresRequest.java new file mode 100644 index 0000000000..2f988d1c9b --- /dev/null +++ b/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/ListStoresRequest.java @@ -0,0 +1,66 @@ +package com.linkedin.venice.controller.server; + +import java.util.Optional; + + +public class ListStoresRequest { + String cluster; + String storeName; + String includeSystemStores; + Optional storeConfigNameFilter; + Optional storeConfigValueFilter; + + public ListStoresRequest( + String cluster, + String storeName, + String includeSystemStores, + Optional storeConfigNameFilter, + Optional storeConfigValueFilter) { + this.cluster = cluster; + this.storeName = storeName; + this.includeSystemStores = includeSystemStores; + } + + public ListStoresRequest() { + } + + public String getCluster() { + return cluster; + } + + public String getStoreName() { + return storeName; + } + + public void setCluster(String cluster) { + this.cluster = cluster; + } + + public void setStoreName(String storeName) { + this.storeName = storeName; + } + + public String getIncludeSystemStores() { + return includeSystemStores; + } + + public void setIncludeSystemStores(String includeSystemStores) { + this.includeSystemStores = includeSystemStores; + } + + public Optional getStoreConfigNameFilter() { + return storeConfigNameFilter; + } + + public void setStoreConfigNameFilter(Optional storeConfigNameFilter) { + this.storeConfigNameFilter = storeConfigNameFilter; + } + + public Optional getStoreConfigValueFilter() { + return storeConfigValueFilter; + } + + public void setStoreConfigValueFilter(Optional storeConfigValueFilter) { + this.storeConfigValueFilter = storeConfigValueFilter; + } +} diff --git a/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/StoresRoutes.java b/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/StoresRoutes.java index 6bfa7f51b3..8335e860c6 100644 --- a/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/StoresRoutes.java +++ b/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/StoresRoutes.java @@ -134,119 +134,140 @@ public Route getAllStores(Admin admin) { @Override public void internalHandle(Request request, MultiStoreResponse veniceResponse) { AdminSparkServer.validateParams(request, LIST_STORES.getParams(), admin); - veniceResponse.setCluster(request.queryParams(CLUSTER)); - veniceResponse.setName(request.queryParams(NAME)); - // Potentially filter out the system stores - String includeSystemStores = request.queryParams(INCLUDE_SYSTEM_STORES); - // If the param is not provided, the default is to include them - boolean excludeSystemStores = (includeSystemStores != null && !Boolean.parseBoolean(includeSystemStores)); + ListStoresRequest listStoresRequest = new ListStoresRequest(); + listStoresRequest.setStoreName(request.queryParams(NAME)); + listStoresRequest.setCluster(request.queryParams(CLUSTER)); + listStoresRequest.setIncludeSystemStores(request.queryParams(INCLUDE_SYSTEM_STORES)); + Optional storeConfigNameFilter = Optional.ofNullable(request.queryParamOrDefault(STORE_CONFIG_NAME_FILTER, null)); Optional storeConfigValueFilter = Optional.ofNullable(request.queryParamOrDefault(STORE_CONFIG_VALUE_FILTER, null)); - if (storeConfigNameFilter.isPresent() ^ storeConfigValueFilter.isPresent()) { + + listStoresRequest.setStoreConfigNameFilter(storeConfigNameFilter); + listStoresRequest.setStoreConfigValueFilter(storeConfigValueFilter); + + MultiStoreResponse duplicateVeniceResponse = listStores(listStoresRequest, admin); + + veniceResponse.setCluster(duplicateVeniceResponse.getCluster()); + veniceResponse.setName(duplicateVeniceResponse.getName()); + veniceResponse.setStores(duplicateVeniceResponse.getStores()); + } + }; + } + + private MultiStoreResponse listStores(ListStoresRequest request, Admin admin) { + MultiStoreResponse veniceResponse = new MultiStoreResponse(); + veniceResponse.setCluster(request.getCluster()); + veniceResponse.setName(request.getStoreName()); + + // Potentially filter out the system stores + String includeSystemStores = request.getIncludeSystemStores(); + // If the param is not provided, the default is to include them + boolean excludeSystemStores = (includeSystemStores != null && !Boolean.parseBoolean(includeSystemStores)); + Optional storeConfigNameFilter = request.getStoreConfigNameFilter(); + Optional storeConfigValueFilter = request.getStoreConfigValueFilter(); + if (storeConfigNameFilter.isPresent() ^ storeConfigValueFilter.isPresent()) { + throw new VeniceException( + "Missing parameter: " + + (storeConfigNameFilter.isPresent() ? "store_config_value_filter" : "store_config_name_filter")); + } + boolean isDataReplicationPolicyConfigFilter = false; + Schema.Field configFilterField = null; + if (storeConfigNameFilter.isPresent()) { + configFilterField = StoreProperties.getClassSchema().getField(storeConfigNameFilter.get()); + if (configFilterField == null) { + isDataReplicationPolicyConfigFilter = storeConfigNameFilter.get().equalsIgnoreCase("dataReplicationPolicy"); + if (!isDataReplicationPolicyConfigFilter) { throw new VeniceException( - "Missing parameter: " - + (storeConfigNameFilter.isPresent() ? "store_config_value_filter" : "store_config_name_filter")); + "The config name filter " + storeConfigNameFilter.get() + " is not a valid store config."); } - boolean isDataReplicationPolicyConfigFilter = false; - Schema.Field configFilterField = null; - if (storeConfigNameFilter.isPresent()) { - configFilterField = StoreProperties.getClassSchema().getField(storeConfigNameFilter.get()); - if (configFilterField == null) { - isDataReplicationPolicyConfigFilter = storeConfigNameFilter.get().equalsIgnoreCase("dataReplicationPolicy"); - if (!isDataReplicationPolicyConfigFilter) { - throw new VeniceException( - "The config name filter " + storeConfigNameFilter.get() + " is not a valid store config."); - } - } + } + } + + List storeList = admin.getAllStores(veniceResponse.getCluster()); + List selectedStoreList; + if (excludeSystemStores || storeConfigNameFilter.isPresent()) { + selectedStoreList = new ArrayList<>(); + for (Store store: storeList) { + if (excludeSystemStores && store.isSystemStore()) { + continue; } - - List storeList = admin.getAllStores(veniceResponse.getCluster()); - List selectedStoreList; - if (excludeSystemStores || storeConfigNameFilter.isPresent()) { - selectedStoreList = new ArrayList<>(); - for (Store store: storeList) { - if (excludeSystemStores && store.isSystemStore()) { + if (storeConfigValueFilter.isPresent()) { + boolean configValueMatch = false; + if (isDataReplicationPolicyConfigFilter) { + if (!store.isHybrid() || store.getHybridStoreConfig().getDataReplicationPolicy() == null) { continue; } - if (storeConfigValueFilter.isPresent()) { - boolean configValueMatch = false; - if (isDataReplicationPolicyConfigFilter) { - if (!store.isHybrid() || store.getHybridStoreConfig().getDataReplicationPolicy() == null) { - continue; - } - configValueMatch = store.getHybridStoreConfig() - .getDataReplicationPolicy() - .name() - .equalsIgnoreCase(storeConfigValueFilter.get()); - } else { - ZKStore cloneStore = new ZKStore(store); - Object configValue = cloneStore.dataModel().get(storeConfigNameFilter.get()); - if (configValue == null) { - // If the store doesn't have the config, it fails the match - continue; - } - // Compare based on schema type - Schema fieldSchema = configFilterField.schema(); - switch (fieldSchema.getType()) { - case BOOLEAN: - configValueMatch = Boolean.valueOf(storeConfigValueFilter.get()).equals((Boolean) configValue); - break; - case INT: - configValueMatch = Integer.valueOf(storeConfigValueFilter.get()).equals((Integer) configValue); - break; - case LONG: - configValueMatch = Long.valueOf(storeConfigValueFilter.get()).equals((Long) configValue); - break; - case FLOAT: - configValueMatch = Float.valueOf(storeConfigValueFilter.get()).equals(configValue); - break; - case DOUBLE: - configValueMatch = Double.valueOf(storeConfigValueFilter.get()).equals(configValue); - break; - case STRING: - configValueMatch = storeConfigValueFilter.get().equals(configValue); - break; - case ENUM: - configValueMatch = storeConfigValueFilter.get().equals(configValue.toString()); - break; - case UNION: - /** - * For union field, return match as long as the union field is not null - */ - configValueMatch = (configValue != null); - break; - case ARRAY: - case MAP: - case FIXED: - case BYTES: - case RECORD: - case NULL: - default: - throw new VeniceException( - "Store config filtering for Schema type " + fieldSchema.getType().toString() - + " is not supported"); - } - } - if (!configValueMatch) { - continue; - } + configValueMatch = store.getHybridStoreConfig() + .getDataReplicationPolicy() + .name() + .equalsIgnoreCase(storeConfigValueFilter.get()); + } else { + ZKStore cloneStore = new ZKStore(store); + Object configValue = cloneStore.dataModel().get(storeConfigNameFilter.get()); + if (configValue == null) { + // If the store doesn't have the config, it fails the match + continue; + } + // Compare based on schema type + Schema fieldSchema = configFilterField.schema(); + switch (fieldSchema.getType()) { + case BOOLEAN: + configValueMatch = Boolean.valueOf(storeConfigValueFilter.get()).equals((Boolean) configValue); + break; + case INT: + configValueMatch = Integer.valueOf(storeConfigValueFilter.get()).equals((Integer) configValue); + break; + case LONG: + configValueMatch = Long.valueOf(storeConfigValueFilter.get()).equals((Long) configValue); + break; + case FLOAT: + configValueMatch = Float.valueOf(storeConfigValueFilter.get()).equals(configValue); + break; + case DOUBLE: + configValueMatch = Double.valueOf(storeConfigValueFilter.get()).equals(configValue); + break; + case STRING: + configValueMatch = storeConfigValueFilter.get().equals(configValue); + break; + case ENUM: + configValueMatch = storeConfigValueFilter.get().equals(configValue.toString()); + break; + case UNION: + /** + * For union field, return match as long as the union field is not null + */ + configValueMatch = (configValue != null); + break; + case ARRAY: + case MAP: + case FIXED: + case BYTES: + case RECORD: + case NULL: + default: + throw new VeniceException( + "Store config filtering for Schema type " + fieldSchema.getType().toString() + " is not supported"); } - selectedStoreList.add(store); } - } else { - selectedStoreList = storeList; - } - - String[] storeNameList = new String[selectedStoreList.size()]; - for (int i = 0; i < selectedStoreList.size(); i++) { - storeNameList[i] = selectedStoreList.get(i).getName(); + if (!configValueMatch) { + continue; + } } - veniceResponse.setStores(storeNameList); + selectedStoreList.add(store); } - }; + } else { + selectedStoreList = storeList; + } + + String[] storeNameList = new String[selectedStoreList.size()]; + for (int i = 0; i < selectedStoreList.size(); i++) { + storeNameList[i] = selectedStoreList.get(i).getName(); + } + veniceResponse.setStores(storeNameList); + return veniceResponse; } /** From a2dda3916198af4a33758dde787dee11a35238b4 Mon Sep 17 00:00:00 2001 From: Vishal Sathyanarayana Date: Wed, 16 Oct 2024 22:26:07 -0700 Subject: [PATCH 2/8] extract getAllStoresStatuses the same way --- .../controller/server/StoresRoutes.java | 76 +++++-------------- 1 file changed, 17 insertions(+), 59 deletions(-) diff --git a/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/StoresRoutes.java b/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/StoresRoutes.java index 8335e860c6..e2b9981651 100644 --- a/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/StoresRoutes.java +++ b/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/StoresRoutes.java @@ -1,59 +1,8 @@ package com.linkedin.venice.controller.server; -import static com.linkedin.venice.controller.server.VeniceRouteHandler.ACL_CHECK_FAILURE_WARN_MESSAGE_PREFIX; -import static com.linkedin.venice.controllerapi.ControllerApiConstants.CLUSTER; -import static com.linkedin.venice.controllerapi.ControllerApiConstants.CLUSTER_DEST; -import static com.linkedin.venice.controllerapi.ControllerApiConstants.FABRIC; -import static com.linkedin.venice.controllerapi.ControllerApiConstants.FABRIC_A; -import static com.linkedin.venice.controllerapi.ControllerApiConstants.FABRIC_B; -import static com.linkedin.venice.controllerapi.ControllerApiConstants.HEARTBEAT_TIMESTAMP; -import static com.linkedin.venice.controllerapi.ControllerApiConstants.INCLUDE_SYSTEM_STORES; -import static com.linkedin.venice.controllerapi.ControllerApiConstants.NAME; -import static com.linkedin.venice.controllerapi.ControllerApiConstants.OPERATION; -import static com.linkedin.venice.controllerapi.ControllerApiConstants.OWNER; -import static com.linkedin.venice.controllerapi.ControllerApiConstants.PARTITION_DETAIL_ENABLED; -import static com.linkedin.venice.controllerapi.ControllerApiConstants.READ_OPERATION; -import static com.linkedin.venice.controllerapi.ControllerApiConstants.READ_WRITE_OPERATION; -import static com.linkedin.venice.controllerapi.ControllerApiConstants.REGIONS_FILTER; -import static com.linkedin.venice.controllerapi.ControllerApiConstants.STATUS; -import static com.linkedin.venice.controllerapi.ControllerApiConstants.STORE_CONFIG_NAME_FILTER; -import static com.linkedin.venice.controllerapi.ControllerApiConstants.STORE_CONFIG_VALUE_FILTER; -import static com.linkedin.venice.controllerapi.ControllerApiConstants.STORE_TYPE; -import static com.linkedin.venice.controllerapi.ControllerApiConstants.TOPIC; -import static com.linkedin.venice.controllerapi.ControllerApiConstants.TOPIC_COMPACTION_POLICY; -import static com.linkedin.venice.controllerapi.ControllerApiConstants.VERSION; -import static com.linkedin.venice.controllerapi.ControllerApiConstants.WRITE_OPERATION; -import static com.linkedin.venice.controllerapi.ControllerRoute.ABORT_MIGRATION; -import static com.linkedin.venice.controllerapi.ControllerRoute.BACKUP_VERSION; -import static com.linkedin.venice.controllerapi.ControllerRoute.CLUSTER_HEALTH_STORES; -import static com.linkedin.venice.controllerapi.ControllerRoute.COMPARE_STORE; -import static com.linkedin.venice.controllerapi.ControllerRoute.COMPLETE_MIGRATION; -import static com.linkedin.venice.controllerapi.ControllerRoute.CONFIGURE_ACTIVE_ACTIVE_REPLICATION_FOR_CLUSTER; -import static com.linkedin.venice.controllerapi.ControllerRoute.DELETE_ALL_VERSIONS; -import static com.linkedin.venice.controllerapi.ControllerRoute.DELETE_KAFKA_TOPIC; -import static com.linkedin.venice.controllerapi.ControllerRoute.DELETE_STORE; -import static com.linkedin.venice.controllerapi.ControllerRoute.ENABLE_STORE; -import static com.linkedin.venice.controllerapi.ControllerRoute.FUTURE_VERSION; -import static com.linkedin.venice.controllerapi.ControllerRoute.GET_DELETABLE_STORE_TOPICS; -import static com.linkedin.venice.controllerapi.ControllerRoute.GET_HEARTBEAT_TIMESTAMP_FROM_SYSTEM_STORE; -import static com.linkedin.venice.controllerapi.ControllerRoute.GET_INUSE_SCHEMA_IDS; -import static com.linkedin.venice.controllerapi.ControllerRoute.GET_REGION_PUSH_DETAILS; -import static com.linkedin.venice.controllerapi.ControllerRoute.GET_REPUSH_INFO; -import static com.linkedin.venice.controllerapi.ControllerRoute.GET_STALE_STORES_IN_CLUSTER; -import static com.linkedin.venice.controllerapi.ControllerRoute.GET_STORES_IN_CLUSTER; -import static com.linkedin.venice.controllerapi.ControllerRoute.LIST_STORES; -import static com.linkedin.venice.controllerapi.ControllerRoute.LIST_STORE_PUSH_INFO; -import static com.linkedin.venice.controllerapi.ControllerRoute.MIGRATE_STORE; -import static com.linkedin.venice.controllerapi.ControllerRoute.REMOVE_STORE_FROM_GRAVEYARD; -import static com.linkedin.venice.controllerapi.ControllerRoute.ROLLBACK_TO_BACKUP_VERSION; -import static com.linkedin.venice.controllerapi.ControllerRoute.ROLL_FORWARD_TO_FUTURE_VERSION; -import static com.linkedin.venice.controllerapi.ControllerRoute.SEND_HEARTBEAT_TIMESTAMP_TO_SYSTEM_STORE; -import static com.linkedin.venice.controllerapi.ControllerRoute.SET_OWNER; -import static com.linkedin.venice.controllerapi.ControllerRoute.SET_TOPIC_COMPACTION; -import static com.linkedin.venice.controllerapi.ControllerRoute.SET_VERSION; -import static com.linkedin.venice.controllerapi.ControllerRoute.STORAGE_ENGINE_OVERHEAD_RATIO; -import static com.linkedin.venice.controllerapi.ControllerRoute.STORE; -import static com.linkedin.venice.controllerapi.ControllerRoute.UPDATE_STORE; +import static com.linkedin.venice.controller.server.VeniceRouteHandler.*; +import static com.linkedin.venice.controllerapi.ControllerApiConstants.*; +import static com.linkedin.venice.controllerapi.ControllerRoute.*; import com.linkedin.venice.HttpConstants; import com.linkedin.venice.acl.DynamicAccessController; @@ -279,14 +228,24 @@ public Route getAllStoresStatuses(Admin admin) { @Override public void internalHandle(Request request, MultiStoreStatusResponse veniceResponse) { AdminSparkServer.validateParams(request, CLUSTER_HEALTH_STORES.getParams(), admin); - String clusterName = request.queryParams(CLUSTER); - veniceResponse.setCluster(clusterName); - Map storeStatusMap = admin.getAllStoreStatuses(clusterName); - veniceResponse.setStoreStatusMap(storeStatusMap); + ListStoresRequest listStoresRequest = new ListStoresRequest(); + listStoresRequest.setCluster(request.queryParams(CLUSTER)); + + MultiStoreStatusResponse duplicateVeniceResponse = listAllStoresStatuses(listStoresRequest, admin); + veniceResponse.setCluster(duplicateVeniceResponse.getCluster()); + veniceResponse.setStoreStatusMap(duplicateVeniceResponse.getStoreStatusMap()); } }; } + private MultiStoreStatusResponse listAllStoresStatuses(ListStoresRequest request, Admin admin) { + MultiStoreStatusResponse veniceResponse = new MultiStoreStatusResponse(); + veniceResponse.setCluster(request.getCluster()); + Map storeStatusMap = admin.getAllStoreStatuses(request.getCluster()); + veniceResponse.setStoreStatusMap(storeStatusMap); + return veniceResponse; + } + public Route getInUseSchemaIds(Admin admin) { return new VeniceRouteHandler(SchemaUsageResponse.class) { @Override @@ -297,7 +256,6 @@ public void internalHandle(Request request, SchemaUsageResponse response) { String storeName = request.queryParams(NAME); Set schemaIds = admin.getInUseValueSchemaIds(clusterName, storeName); response.setInUseValueSchemaIds(schemaIds); - } }; } From f1cbca55c40a2b344da71bd623cbb50279136d4e Mon Sep 17 00:00:00 2001 From: Vishal Sathyanarayana Date: Wed, 16 Oct 2024 22:30:54 -0700 Subject: [PATCH 3/8] add back imports --- .../controller/server/StoresRoutes.java | 57 ++++++++++++++++++- 1 file changed, 54 insertions(+), 3 deletions(-) diff --git a/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/StoresRoutes.java b/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/StoresRoutes.java index e2b9981651..3e2baff0be 100644 --- a/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/StoresRoutes.java +++ b/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/StoresRoutes.java @@ -1,8 +1,59 @@ package com.linkedin.venice.controller.server; -import static com.linkedin.venice.controller.server.VeniceRouteHandler.*; -import static com.linkedin.venice.controllerapi.ControllerApiConstants.*; -import static com.linkedin.venice.controllerapi.ControllerRoute.*; +import static com.linkedin.venice.controller.server.VeniceRouteHandler.ACL_CHECK_FAILURE_WARN_MESSAGE_PREFIX; +import static com.linkedin.venice.controllerapi.ControllerApiConstants.CLUSTER; +import static com.linkedin.venice.controllerapi.ControllerApiConstants.CLUSTER_DEST; +import static com.linkedin.venice.controllerapi.ControllerApiConstants.FABRIC; +import static com.linkedin.venice.controllerapi.ControllerApiConstants.FABRIC_A; +import static com.linkedin.venice.controllerapi.ControllerApiConstants.FABRIC_B; +import static com.linkedin.venice.controllerapi.ControllerApiConstants.HEARTBEAT_TIMESTAMP; +import static com.linkedin.venice.controllerapi.ControllerApiConstants.INCLUDE_SYSTEM_STORES; +import static com.linkedin.venice.controllerapi.ControllerApiConstants.NAME; +import static com.linkedin.venice.controllerapi.ControllerApiConstants.OPERATION; +import static com.linkedin.venice.controllerapi.ControllerApiConstants.OWNER; +import static com.linkedin.venice.controllerapi.ControllerApiConstants.PARTITION_DETAIL_ENABLED; +import static com.linkedin.venice.controllerapi.ControllerApiConstants.READ_OPERATION; +import static com.linkedin.venice.controllerapi.ControllerApiConstants.READ_WRITE_OPERATION; +import static com.linkedin.venice.controllerapi.ControllerApiConstants.REGIONS_FILTER; +import static com.linkedin.venice.controllerapi.ControllerApiConstants.STATUS; +import static com.linkedin.venice.controllerapi.ControllerApiConstants.STORE_CONFIG_NAME_FILTER; +import static com.linkedin.venice.controllerapi.ControllerApiConstants.STORE_CONFIG_VALUE_FILTER; +import static com.linkedin.venice.controllerapi.ControllerApiConstants.STORE_TYPE; +import static com.linkedin.venice.controllerapi.ControllerApiConstants.TOPIC; +import static com.linkedin.venice.controllerapi.ControllerApiConstants.TOPIC_COMPACTION_POLICY; +import static com.linkedin.venice.controllerapi.ControllerApiConstants.VERSION; +import static com.linkedin.venice.controllerapi.ControllerApiConstants.WRITE_OPERATION; +import static com.linkedin.venice.controllerapi.ControllerRoute.ABORT_MIGRATION; +import static com.linkedin.venice.controllerapi.ControllerRoute.BACKUP_VERSION; +import static com.linkedin.venice.controllerapi.ControllerRoute.CLUSTER_HEALTH_STORES; +import static com.linkedin.venice.controllerapi.ControllerRoute.COMPARE_STORE; +import static com.linkedin.venice.controllerapi.ControllerRoute.COMPLETE_MIGRATION; +import static com.linkedin.venice.controllerapi.ControllerRoute.CONFIGURE_ACTIVE_ACTIVE_REPLICATION_FOR_CLUSTER; +import static com.linkedin.venice.controllerapi.ControllerRoute.DELETE_ALL_VERSIONS; +import static com.linkedin.venice.controllerapi.ControllerRoute.DELETE_KAFKA_TOPIC; +import static com.linkedin.venice.controllerapi.ControllerRoute.DELETE_STORE; +import static com.linkedin.venice.controllerapi.ControllerRoute.ENABLE_STORE; +import static com.linkedin.venice.controllerapi.ControllerRoute.FUTURE_VERSION; +import static com.linkedin.venice.controllerapi.ControllerRoute.GET_DELETABLE_STORE_TOPICS; +import static com.linkedin.venice.controllerapi.ControllerRoute.GET_HEARTBEAT_TIMESTAMP_FROM_SYSTEM_STORE; +import static com.linkedin.venice.controllerapi.ControllerRoute.GET_INUSE_SCHEMA_IDS; +import static com.linkedin.venice.controllerapi.ControllerRoute.GET_REGION_PUSH_DETAILS; +import static com.linkedin.venice.controllerapi.ControllerRoute.GET_REPUSH_INFO; +import static com.linkedin.venice.controllerapi.ControllerRoute.GET_STALE_STORES_IN_CLUSTER; +import static com.linkedin.venice.controllerapi.ControllerRoute.GET_STORES_IN_CLUSTER; +import static com.linkedin.venice.controllerapi.ControllerRoute.LIST_STORES; +import static com.linkedin.venice.controllerapi.ControllerRoute.LIST_STORE_PUSH_INFO; +import static com.linkedin.venice.controllerapi.ControllerRoute.MIGRATE_STORE; +import static com.linkedin.venice.controllerapi.ControllerRoute.REMOVE_STORE_FROM_GRAVEYARD; +import static com.linkedin.venice.controllerapi.ControllerRoute.ROLLBACK_TO_BACKUP_VERSION; +import static com.linkedin.venice.controllerapi.ControllerRoute.ROLL_FORWARD_TO_FUTURE_VERSION; +import static com.linkedin.venice.controllerapi.ControllerRoute.SEND_HEARTBEAT_TIMESTAMP_TO_SYSTEM_STORE; +import static com.linkedin.venice.controllerapi.ControllerRoute.SET_OWNER; +import static com.linkedin.venice.controllerapi.ControllerRoute.SET_TOPIC_COMPACTION; +import static com.linkedin.venice.controllerapi.ControllerRoute.SET_VERSION; +import static com.linkedin.venice.controllerapi.ControllerRoute.STORAGE_ENGINE_OVERHEAD_RATIO; +import static com.linkedin.venice.controllerapi.ControllerRoute.STORE; +import static com.linkedin.venice.controllerapi.ControllerRoute.UPDATE_STORE; import com.linkedin.venice.HttpConstants; import com.linkedin.venice.acl.DynamicAccessController; From 56765c304573c3e4975f5a3348e9b25f3249ba17 Mon Sep 17 00:00:00 2001 From: Vishal Sathyanarayana Date: Wed, 16 Oct 2024 23:18:14 -0700 Subject: [PATCH 4/8] add unit test: testGetAllStoresStatuses --- .../controller/server/ListStoresRequest.java | 12 +++++---- .../controller/server/StoreRoutesTest.java | 25 +++++++++++++++++++ 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/ListStoresRequest.java b/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/ListStoresRequest.java index 2f988d1c9b..f07100249b 100644 --- a/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/ListStoresRequest.java +++ b/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/ListStoresRequest.java @@ -4,11 +4,11 @@ public class ListStoresRequest { - String cluster; - String storeName; - String includeSystemStores; - Optional storeConfigNameFilter; - Optional storeConfigValueFilter; + private String cluster; + private String storeName; + private String includeSystemStores; + private Optional storeConfigNameFilter; + private Optional storeConfigValueFilter; public ListStoresRequest( String cluster, @@ -19,6 +19,8 @@ public ListStoresRequest( this.cluster = cluster; this.storeName = storeName; this.includeSystemStores = includeSystemStores; + this.storeConfigNameFilter = storeConfigNameFilter; + this.storeConfigValueFilter = storeConfigValueFilter; } public ListStoresRequest() { diff --git a/services/venice-controller/src/test/java/com/linkedin/venice/controller/server/StoreRoutesTest.java b/services/venice-controller/src/test/java/com/linkedin/venice/controller/server/StoreRoutesTest.java index c434110706..cf5458752a 100644 --- a/services/venice-controller/src/test/java/com/linkedin/venice/controller/server/StoreRoutesTest.java +++ b/services/venice-controller/src/test/java/com/linkedin/venice/controller/server/StoreRoutesTest.java @@ -64,6 +64,31 @@ public void testGetFutureVersion() throws Exception { Assert.assertEquals(multiStoreStatusResponse.getStoreStatusMap(), storeStatusMap); } + @Test + public void testGetAllStoresStatuses() throws Exception { + Admin mockAdmin = mock(VeniceParentHelixAdmin.class); + doReturn(true).when(mockAdmin).isLeaderControllerFor(TEST_CLUSTER); + + Store mockStore = mock(Store.class); + doReturn(mockStore).when(mockAdmin).getStore(TEST_CLUSTER, TEST_STORE_NAME); + + Map storeStatusMap = Collections.singletonMap("dc-0", "1"); + doReturn(storeStatusMap).when(mockAdmin).getAllStoreStatuses(TEST_CLUSTER); + + Request request = mock(Request.class); + doReturn(TEST_CLUSTER).when(request).queryParams(eq(ControllerApiConstants.CLUSTER)); + doReturn(TEST_STORE_NAME).when(request).queryParams(eq(ControllerApiConstants.NAME)); + + Route getAllStoresStatusesRoute = + new StoresRoutes(false, Optional.empty(), pubSubTopicRepository).getAllStoresStatuses(mockAdmin); + MultiStoreStatusResponse multiStoreStatusResponse = ObjectMapperFactory.getInstance() + .readValue( + getAllStoresStatusesRoute.handle(request, mock(Response.class)).toString(), + MultiStoreStatusResponse.class); + Assert.assertEquals(multiStoreStatusResponse.getCluster(), TEST_CLUSTER); + Assert.assertEquals(multiStoreStatusResponse.getStoreStatusMap(), storeStatusMap); + } + @Test public void testRollForwardToFutureVersion() throws Exception { Admin mockAdmin = mock(VeniceParentHelixAdmin.class); From 118fa6a92fc7c56e4f3e97d4b271b39ccaca794f Mon Sep 17 00:00:00 2001 From: Vishal Sathyanarayana Date: Wed, 16 Oct 2024 23:23:38 -0700 Subject: [PATCH 5/8] add unit tests for ListStoresRequest --- .../server/ListStoresRequestTest.java | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 services/venice-controller/src/test/java/com/linkedin/venice/controller/server/ListStoresRequestTest.java diff --git a/services/venice-controller/src/test/java/com/linkedin/venice/controller/server/ListStoresRequestTest.java b/services/venice-controller/src/test/java/com/linkedin/venice/controller/server/ListStoresRequestTest.java new file mode 100644 index 0000000000..6f1809dd30 --- /dev/null +++ b/services/venice-controller/src/test/java/com/linkedin/venice/controller/server/ListStoresRequestTest.java @@ -0,0 +1,84 @@ +package com.linkedin.venice.controller.server; + +import static org.testng.Assert.*; + +import java.util.Optional; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + + +public class ListStoresRequestTest { + private ListStoresRequest _mockListStoresRequest; + + private final String TEST_CLUSTER = "testCluster"; + private final String TEST_STORE_NAME = "testStoreName"; + private final String TEST_INCLUDE_SYSTEM_STORES = "testIncludeSystemStores"; + private final String TEST_STORE_CONFIG_NAME_FILTER = "testStoreConfigNameFilter"; + private final String TEST_STORE_CONFIG_VALUE_FILTER = "testStoreConfigValueFilter"; + + @BeforeMethod + public void setUp() { + // Create a mock ListStoresRequestTest with mock data for each of it's private fields: + _mockListStoresRequest = new ListStoresRequest( + "cluster", + "storeName", + "includeSystemStores", + Optional.of("storeConfigNameFilter"), + Optional.of("storeConfigValueFilter")); + } + + @Test + public void testGetCluster() { + assertEquals(_mockListStoresRequest.getCluster(), "cluster"); + } + + @Test + public void testGetStoreName() { + assertEquals(_mockListStoresRequest.getStoreName(), "storeName"); + } + + @Test + public void testSetCluster() { + _mockListStoresRequest.setCluster(TEST_CLUSTER); + assertEquals(_mockListStoresRequest.getCluster(), TEST_CLUSTER); + } + + @Test + public void testSetStoreName() { + _mockListStoresRequest.setStoreName(TEST_STORE_NAME); + assertEquals(_mockListStoresRequest.getStoreName(), TEST_STORE_NAME); + } + + @Test + public void testGetIncludeSystemStores() { + assertEquals(_mockListStoresRequest.getIncludeSystemStores(), "includeSystemStores"); + } + + @Test + public void testSetIncludeSystemStores() { + _mockListStoresRequest.setIncludeSystemStores(TEST_INCLUDE_SYSTEM_STORES); + assertEquals(_mockListStoresRequest.getIncludeSystemStores(), TEST_INCLUDE_SYSTEM_STORES); + } + + @Test + public void testGetStoreConfigNameFilter() { + assertEquals(_mockListStoresRequest.getStoreConfigNameFilter(), Optional.of("storeConfigNameFilter")); + } + + @Test + public void testSetStoreConfigNameFilter() { + _mockListStoresRequest.setStoreConfigNameFilter(Optional.of(TEST_STORE_CONFIG_NAME_FILTER)); + assertEquals(_mockListStoresRequest.getStoreConfigNameFilter(), Optional.of(TEST_STORE_CONFIG_NAME_FILTER)); + } + + @Test + public void testGetStoreConfigValueFilter() { + assertEquals(_mockListStoresRequest.getStoreConfigValueFilter(), Optional.of("storeConfigValueFilter")); + } + + @Test + public void testSetStoreConfigValueFilter() { + _mockListStoresRequest.setStoreConfigValueFilter(Optional.of(TEST_STORE_CONFIG_VALUE_FILTER)); + assertEquals(_mockListStoresRequest.getStoreConfigValueFilter(), Optional.of(TEST_STORE_CONFIG_VALUE_FILTER)); + } +} From 69124eb192aab2909b0108a87a2b55499d464bb6 Mon Sep 17 00:00:00 2001 From: Vishal Sathyanarayana Date: Wed, 16 Oct 2024 23:50:57 -0700 Subject: [PATCH 6/8] revert first thing --- .../controller/server/StoresRoutes.java | 219 ++++++++---------- 1 file changed, 99 insertions(+), 120 deletions(-) diff --git a/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/StoresRoutes.java b/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/StoresRoutes.java index 3e2baff0be..06ec9c8ce3 100644 --- a/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/StoresRoutes.java +++ b/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/StoresRoutes.java @@ -134,140 +134,119 @@ public Route getAllStores(Admin admin) { @Override public void internalHandle(Request request, MultiStoreResponse veniceResponse) { AdminSparkServer.validateParams(request, LIST_STORES.getParams(), admin); + veniceResponse.setCluster(request.queryParams(CLUSTER)); + veniceResponse.setName(request.queryParams(NAME)); - ListStoresRequest listStoresRequest = new ListStoresRequest(); - listStoresRequest.setStoreName(request.queryParams(NAME)); - listStoresRequest.setCluster(request.queryParams(CLUSTER)); - listStoresRequest.setIncludeSystemStores(request.queryParams(INCLUDE_SYSTEM_STORES)); - + // Potentially filter out the system stores + String includeSystemStores = request.queryParams(INCLUDE_SYSTEM_STORES); + // If the param is not provided, the default is to include them + boolean excludeSystemStores = (includeSystemStores != null && !Boolean.parseBoolean(includeSystemStores)); Optional storeConfigNameFilter = Optional.ofNullable(request.queryParamOrDefault(STORE_CONFIG_NAME_FILTER, null)); Optional storeConfigValueFilter = Optional.ofNullable(request.queryParamOrDefault(STORE_CONFIG_VALUE_FILTER, null)); - - listStoresRequest.setStoreConfigNameFilter(storeConfigNameFilter); - listStoresRequest.setStoreConfigValueFilter(storeConfigValueFilter); - - MultiStoreResponse duplicateVeniceResponse = listStores(listStoresRequest, admin); - - veniceResponse.setCluster(duplicateVeniceResponse.getCluster()); - veniceResponse.setName(duplicateVeniceResponse.getName()); - veniceResponse.setStores(duplicateVeniceResponse.getStores()); - } - }; - } - - private MultiStoreResponse listStores(ListStoresRequest request, Admin admin) { - MultiStoreResponse veniceResponse = new MultiStoreResponse(); - veniceResponse.setCluster(request.getCluster()); - veniceResponse.setName(request.getStoreName()); - - // Potentially filter out the system stores - String includeSystemStores = request.getIncludeSystemStores(); - // If the param is not provided, the default is to include them - boolean excludeSystemStores = (includeSystemStores != null && !Boolean.parseBoolean(includeSystemStores)); - Optional storeConfigNameFilter = request.getStoreConfigNameFilter(); - Optional storeConfigValueFilter = request.getStoreConfigValueFilter(); - if (storeConfigNameFilter.isPresent() ^ storeConfigValueFilter.isPresent()) { - throw new VeniceException( - "Missing parameter: " - + (storeConfigNameFilter.isPresent() ? "store_config_value_filter" : "store_config_name_filter")); - } - boolean isDataReplicationPolicyConfigFilter = false; - Schema.Field configFilterField = null; - if (storeConfigNameFilter.isPresent()) { - configFilterField = StoreProperties.getClassSchema().getField(storeConfigNameFilter.get()); - if (configFilterField == null) { - isDataReplicationPolicyConfigFilter = storeConfigNameFilter.get().equalsIgnoreCase("dataReplicationPolicy"); - if (!isDataReplicationPolicyConfigFilter) { + if (storeConfigNameFilter.isPresent() ^ storeConfigValueFilter.isPresent()) { throw new VeniceException( - "The config name filter " + storeConfigNameFilter.get() + " is not a valid store config."); + "Missing parameter: " + + (storeConfigNameFilter.isPresent() ? "store_config_value_filter" : "store_config_name_filter")); } - } - } - - List storeList = admin.getAllStores(veniceResponse.getCluster()); - List selectedStoreList; - if (excludeSystemStores || storeConfigNameFilter.isPresent()) { - selectedStoreList = new ArrayList<>(); - for (Store store: storeList) { - if (excludeSystemStores && store.isSystemStore()) { - continue; - } - if (storeConfigValueFilter.isPresent()) { - boolean configValueMatch = false; - if (isDataReplicationPolicyConfigFilter) { - if (!store.isHybrid() || store.getHybridStoreConfig().getDataReplicationPolicy() == null) { - continue; + boolean isDataReplicationPolicyConfigFilter = false; + Schema.Field configFilterField = null; + if (storeConfigNameFilter.isPresent()) { + configFilterField = StoreProperties.getClassSchema().getField(storeConfigNameFilter.get()); + if (configFilterField == null) { + isDataReplicationPolicyConfigFilter = storeConfigNameFilter.get().equalsIgnoreCase("dataReplicationPolicy"); + if (!isDataReplicationPolicyConfigFilter) { + throw new VeniceException( + "The config name filter " + storeConfigNameFilter.get() + " is not a valid store config."); } - configValueMatch = store.getHybridStoreConfig() - .getDataReplicationPolicy() - .name() - .equalsIgnoreCase(storeConfigValueFilter.get()); - } else { - ZKStore cloneStore = new ZKStore(store); - Object configValue = cloneStore.dataModel().get(storeConfigNameFilter.get()); - if (configValue == null) { - // If the store doesn't have the config, it fails the match + } + } + + List storeList = admin.getAllStores(veniceResponse.getCluster()); + List selectedStoreList; + if (excludeSystemStores || storeConfigNameFilter.isPresent()) { + selectedStoreList = new ArrayList<>(); + for (Store store: storeList) { + if (excludeSystemStores && store.isSystemStore()) { continue; } - // Compare based on schema type - Schema fieldSchema = configFilterField.schema(); - switch (fieldSchema.getType()) { - case BOOLEAN: - configValueMatch = Boolean.valueOf(storeConfigValueFilter.get()).equals((Boolean) configValue); - break; - case INT: - configValueMatch = Integer.valueOf(storeConfigValueFilter.get()).equals((Integer) configValue); - break; - case LONG: - configValueMatch = Long.valueOf(storeConfigValueFilter.get()).equals((Long) configValue); - break; - case FLOAT: - configValueMatch = Float.valueOf(storeConfigValueFilter.get()).equals(configValue); - break; - case DOUBLE: - configValueMatch = Double.valueOf(storeConfigValueFilter.get()).equals(configValue); - break; - case STRING: - configValueMatch = storeConfigValueFilter.get().equals(configValue); - break; - case ENUM: - configValueMatch = storeConfigValueFilter.get().equals(configValue.toString()); - break; - case UNION: - /** - * For union field, return match as long as the union field is not null - */ - configValueMatch = (configValue != null); - break; - case ARRAY: - case MAP: - case FIXED: - case BYTES: - case RECORD: - case NULL: - default: - throw new VeniceException( - "Store config filtering for Schema type " + fieldSchema.getType().toString() + " is not supported"); + if (storeConfigValueFilter.isPresent()) { + boolean configValueMatch = false; + if (isDataReplicationPolicyConfigFilter) { + if (!store.isHybrid() || store.getHybridStoreConfig().getDataReplicationPolicy() == null) { + continue; + } + configValueMatch = store.getHybridStoreConfig() + .getDataReplicationPolicy() + .name() + .equalsIgnoreCase(storeConfigValueFilter.get()); + } else { + ZKStore cloneStore = new ZKStore(store); + Object configValue = cloneStore.dataModel().get(storeConfigNameFilter.get()); + if (configValue == null) { + // If the store doesn't have the config, it fails the match + continue; + } + // Compare based on schema type + Schema fieldSchema = configFilterField.schema(); + switch (fieldSchema.getType()) { + case BOOLEAN: + configValueMatch = Boolean.valueOf(storeConfigValueFilter.get()).equals((Boolean) configValue); + break; + case INT: + configValueMatch = Integer.valueOf(storeConfigValueFilter.get()).equals((Integer) configValue); + break; + case LONG: + configValueMatch = Long.valueOf(storeConfigValueFilter.get()).equals((Long) configValue); + break; + case FLOAT: + configValueMatch = Float.valueOf(storeConfigValueFilter.get()).equals(configValue); + break; + case DOUBLE: + configValueMatch = Double.valueOf(storeConfigValueFilter.get()).equals(configValue); + break; + case STRING: + configValueMatch = storeConfigValueFilter.get().equals(configValue); + break; + case ENUM: + configValueMatch = storeConfigValueFilter.get().equals(configValue.toString()); + break; + case UNION: + /** + * For union field, return match as long as the union field is not null + */ + configValueMatch = (configValue != null); + break; + case ARRAY: + case MAP: + case FIXED: + case BYTES: + case RECORD: + case NULL: + default: + throw new VeniceException( + "Store config filtering for Schema type " + fieldSchema.getType().toString() + + " is not supported"); + } + } + if (!configValueMatch) { + continue; + } } + selectedStoreList.add(store); } - if (!configValueMatch) { - continue; - } + } else { + selectedStoreList = storeList; } - selectedStoreList.add(store); + + String[] storeNameList = new String[selectedStoreList.size()]; + for (int i = 0; i < selectedStoreList.size(); i++) { + storeNameList[i] = selectedStoreList.get(i).getName(); + } + veniceResponse.setStores(storeNameList); } - } else { - selectedStoreList = storeList; - } - - String[] storeNameList = new String[selectedStoreList.size()]; - for (int i = 0; i < selectedStoreList.size(); i++) { - storeNameList[i] = selectedStoreList.get(i).getName(); - } - veniceResponse.setStores(storeNameList); - return veniceResponse; + }; } /** From a6c298e153d18e8ff32386a8a19629950bab4cf1 Mon Sep 17 00:00:00 2001 From: Vishal Sathyanarayana Date: Wed, 16 Oct 2024 23:59:06 -0700 Subject: [PATCH 7/8] name change --- .../com/linkedin/venice/controller/server/StoresRoutes.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/StoresRoutes.java b/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/StoresRoutes.java index 06ec9c8ce3..ed471ccd54 100644 --- a/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/StoresRoutes.java +++ b/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/StoresRoutes.java @@ -261,9 +261,9 @@ public void internalHandle(Request request, MultiStoreStatusResponse veniceRespo ListStoresRequest listStoresRequest = new ListStoresRequest(); listStoresRequest.setCluster(request.queryParams(CLUSTER)); - MultiStoreStatusResponse duplicateVeniceResponse = listAllStoresStatuses(listStoresRequest, admin); - veniceResponse.setCluster(duplicateVeniceResponse.getCluster()); - veniceResponse.setStoreStatusMap(duplicateVeniceResponse.getStoreStatusMap()); + MultiStoreStatusResponse multiStoreStatusResponse = listAllStoresStatuses(listStoresRequest, admin); + veniceResponse.setCluster(multiStoreStatusResponse.getCluster()); + veniceResponse.setStoreStatusMap(multiStoreStatusResponse.getStoreStatusMap()); } }; } From a1feb3b0ea150d6f08346643f25205295abc30e1 Mon Sep 17 00:00:00 2001 From: Vishal Sathyanarayana Date: Thu, 17 Oct 2024 12:32:05 -0700 Subject: [PATCH 8/8] changes --- ...java => ListAllStoresStatusesRequest.java} | 6 +- .../controller/server/StoresRoutes.java | 8 +- .../ListAllStoresStatusesRequestTest.java | 88 +++++++++++++++++++ .../server/ListStoresRequestTest.java | 84 ------------------ 4 files changed, 95 insertions(+), 91 deletions(-) rename services/venice-controller/src/main/java/com/linkedin/venice/controller/server/{ListStoresRequest.java => ListAllStoresStatusesRequest.java} (92%) create mode 100644 services/venice-controller/src/test/java/com/linkedin/venice/controller/server/ListAllStoresStatusesRequestTest.java delete mode 100644 services/venice-controller/src/test/java/com/linkedin/venice/controller/server/ListStoresRequestTest.java diff --git a/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/ListStoresRequest.java b/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/ListAllStoresStatusesRequest.java similarity index 92% rename from services/venice-controller/src/main/java/com/linkedin/venice/controller/server/ListStoresRequest.java rename to services/venice-controller/src/main/java/com/linkedin/venice/controller/server/ListAllStoresStatusesRequest.java index f07100249b..5d9a0e5fa9 100644 --- a/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/ListStoresRequest.java +++ b/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/ListAllStoresStatusesRequest.java @@ -3,14 +3,14 @@ import java.util.Optional; -public class ListStoresRequest { +public class ListAllStoresStatusesRequest { private String cluster; private String storeName; private String includeSystemStores; private Optional storeConfigNameFilter; private Optional storeConfigValueFilter; - public ListStoresRequest( + public ListAllStoresStatusesRequest( String cluster, String storeName, String includeSystemStores, @@ -23,7 +23,7 @@ public ListStoresRequest( this.storeConfigValueFilter = storeConfigValueFilter; } - public ListStoresRequest() { + public ListAllStoresStatusesRequest() { } public String getCluster() { diff --git a/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/StoresRoutes.java b/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/StoresRoutes.java index ed471ccd54..a1ed48816a 100644 --- a/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/StoresRoutes.java +++ b/services/venice-controller/src/main/java/com/linkedin/venice/controller/server/StoresRoutes.java @@ -258,17 +258,17 @@ public Route getAllStoresStatuses(Admin admin) { @Override public void internalHandle(Request request, MultiStoreStatusResponse veniceResponse) { AdminSparkServer.validateParams(request, CLUSTER_HEALTH_STORES.getParams(), admin); - ListStoresRequest listStoresRequest = new ListStoresRequest(); - listStoresRequest.setCluster(request.queryParams(CLUSTER)); + ListAllStoresStatusesRequest listAllStoresStatusesRequest = new ListAllStoresStatusesRequest(); + listAllStoresStatusesRequest.setCluster(request.queryParams(CLUSTER)); - MultiStoreStatusResponse multiStoreStatusResponse = listAllStoresStatuses(listStoresRequest, admin); + MultiStoreStatusResponse multiStoreStatusResponse = listAllStoresStatuses(listAllStoresStatusesRequest, admin); veniceResponse.setCluster(multiStoreStatusResponse.getCluster()); veniceResponse.setStoreStatusMap(multiStoreStatusResponse.getStoreStatusMap()); } }; } - private MultiStoreStatusResponse listAllStoresStatuses(ListStoresRequest request, Admin admin) { + private MultiStoreStatusResponse listAllStoresStatuses(ListAllStoresStatusesRequest request, Admin admin) { MultiStoreStatusResponse veniceResponse = new MultiStoreStatusResponse(); veniceResponse.setCluster(request.getCluster()); Map storeStatusMap = admin.getAllStoreStatuses(request.getCluster()); diff --git a/services/venice-controller/src/test/java/com/linkedin/venice/controller/server/ListAllStoresStatusesRequestTest.java b/services/venice-controller/src/test/java/com/linkedin/venice/controller/server/ListAllStoresStatusesRequestTest.java new file mode 100644 index 0000000000..4439cfa0ad --- /dev/null +++ b/services/venice-controller/src/test/java/com/linkedin/venice/controller/server/ListAllStoresStatusesRequestTest.java @@ -0,0 +1,88 @@ +package com.linkedin.venice.controller.server; + +import static org.testng.Assert.*; + +import java.util.Optional; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + + +public class ListAllStoresStatusesRequestTest { + private ListAllStoresStatusesRequest mockListAllStoresStatusesRequest; + + private final String TEST_CLUSTER = "testCluster"; + private final String TEST_STORE_NAME = "testStoreName"; + private final String TEST_INCLUDE_SYSTEM_STORES = "testIncludeSystemStores"; + private final String TEST_STORE_CONFIG_NAME_FILTER = "testStoreConfigNameFilter"; + private final String TEST_STORE_CONFIG_VALUE_FILTER = "testStoreConfigValueFilter"; + + @BeforeMethod + public void setUp() { + // Create a mock ListStoresRequestTest with mock data for each of it's private fields: + mockListAllStoresStatusesRequest = new ListAllStoresStatusesRequest( + "cluster", + "storeName", + "includeSystemStores", + Optional.of("storeConfigNameFilter"), + Optional.of("storeConfigValueFilter")); + } + + @Test + public void testGetCluster() { + assertEquals(mockListAllStoresStatusesRequest.getCluster(), "cluster"); + } + + @Test + public void testGetStoreName() { + assertEquals(mockListAllStoresStatusesRequest.getStoreName(), "storeName"); + } + + @Test + public void testSetCluster() { + mockListAllStoresStatusesRequest.setCluster(TEST_CLUSTER); + assertEquals(mockListAllStoresStatusesRequest.getCluster(), TEST_CLUSTER); + } + + @Test + public void testSetStoreName() { + mockListAllStoresStatusesRequest.setStoreName(TEST_STORE_NAME); + assertEquals(mockListAllStoresStatusesRequest.getStoreName(), TEST_STORE_NAME); + } + + @Test + public void testGetIncludeSystemStores() { + assertEquals(mockListAllStoresStatusesRequest.getIncludeSystemStores(), "includeSystemStores"); + } + + @Test + public void testSetIncludeSystemStores() { + mockListAllStoresStatusesRequest.setIncludeSystemStores(TEST_INCLUDE_SYSTEM_STORES); + assertEquals(mockListAllStoresStatusesRequest.getIncludeSystemStores(), TEST_INCLUDE_SYSTEM_STORES); + } + + @Test + public void testGetStoreConfigNameFilter() { + assertEquals(mockListAllStoresStatusesRequest.getStoreConfigNameFilter(), Optional.of("storeConfigNameFilter")); + } + + @Test + public void testSetStoreConfigNameFilter() { + mockListAllStoresStatusesRequest.setStoreConfigNameFilter(Optional.of(TEST_STORE_CONFIG_NAME_FILTER)); + assertEquals( + mockListAllStoresStatusesRequest.getStoreConfigNameFilter(), + Optional.of(TEST_STORE_CONFIG_NAME_FILTER)); + } + + @Test + public void testGetStoreConfigValueFilter() { + assertEquals(mockListAllStoresStatusesRequest.getStoreConfigValueFilter(), Optional.of("storeConfigValueFilter")); + } + + @Test + public void testSetStoreConfigValueFilter() { + mockListAllStoresStatusesRequest.setStoreConfigValueFilter(Optional.of(TEST_STORE_CONFIG_VALUE_FILTER)); + assertEquals( + mockListAllStoresStatusesRequest.getStoreConfigValueFilter(), + Optional.of(TEST_STORE_CONFIG_VALUE_FILTER)); + } +} diff --git a/services/venice-controller/src/test/java/com/linkedin/venice/controller/server/ListStoresRequestTest.java b/services/venice-controller/src/test/java/com/linkedin/venice/controller/server/ListStoresRequestTest.java deleted file mode 100644 index 6f1809dd30..0000000000 --- a/services/venice-controller/src/test/java/com/linkedin/venice/controller/server/ListStoresRequestTest.java +++ /dev/null @@ -1,84 +0,0 @@ -package com.linkedin.venice.controller.server; - -import static org.testng.Assert.*; - -import java.util.Optional; -import org.testng.annotations.BeforeMethod; -import org.testng.annotations.Test; - - -public class ListStoresRequestTest { - private ListStoresRequest _mockListStoresRequest; - - private final String TEST_CLUSTER = "testCluster"; - private final String TEST_STORE_NAME = "testStoreName"; - private final String TEST_INCLUDE_SYSTEM_STORES = "testIncludeSystemStores"; - private final String TEST_STORE_CONFIG_NAME_FILTER = "testStoreConfigNameFilter"; - private final String TEST_STORE_CONFIG_VALUE_FILTER = "testStoreConfigValueFilter"; - - @BeforeMethod - public void setUp() { - // Create a mock ListStoresRequestTest with mock data for each of it's private fields: - _mockListStoresRequest = new ListStoresRequest( - "cluster", - "storeName", - "includeSystemStores", - Optional.of("storeConfigNameFilter"), - Optional.of("storeConfigValueFilter")); - } - - @Test - public void testGetCluster() { - assertEquals(_mockListStoresRequest.getCluster(), "cluster"); - } - - @Test - public void testGetStoreName() { - assertEquals(_mockListStoresRequest.getStoreName(), "storeName"); - } - - @Test - public void testSetCluster() { - _mockListStoresRequest.setCluster(TEST_CLUSTER); - assertEquals(_mockListStoresRequest.getCluster(), TEST_CLUSTER); - } - - @Test - public void testSetStoreName() { - _mockListStoresRequest.setStoreName(TEST_STORE_NAME); - assertEquals(_mockListStoresRequest.getStoreName(), TEST_STORE_NAME); - } - - @Test - public void testGetIncludeSystemStores() { - assertEquals(_mockListStoresRequest.getIncludeSystemStores(), "includeSystemStores"); - } - - @Test - public void testSetIncludeSystemStores() { - _mockListStoresRequest.setIncludeSystemStores(TEST_INCLUDE_SYSTEM_STORES); - assertEquals(_mockListStoresRequest.getIncludeSystemStores(), TEST_INCLUDE_SYSTEM_STORES); - } - - @Test - public void testGetStoreConfigNameFilter() { - assertEquals(_mockListStoresRequest.getStoreConfigNameFilter(), Optional.of("storeConfigNameFilter")); - } - - @Test - public void testSetStoreConfigNameFilter() { - _mockListStoresRequest.setStoreConfigNameFilter(Optional.of(TEST_STORE_CONFIG_NAME_FILTER)); - assertEquals(_mockListStoresRequest.getStoreConfigNameFilter(), Optional.of(TEST_STORE_CONFIG_NAME_FILTER)); - } - - @Test - public void testGetStoreConfigValueFilter() { - assertEquals(_mockListStoresRequest.getStoreConfigValueFilter(), Optional.of("storeConfigValueFilter")); - } - - @Test - public void testSetStoreConfigValueFilter() { - _mockListStoresRequest.setStoreConfigValueFilter(Optional.of(TEST_STORE_CONFIG_VALUE_FILTER)); - assertEquals(_mockListStoresRequest.getStoreConfigValueFilter(), Optional.of(TEST_STORE_CONFIG_VALUE_FILTER)); - } -}