From 2c94dcb1f55097d2adf807389896922da51b5620 Mon Sep 17 00:00:00 2001 From: Sophie Guo Date: Wed, 15 Jan 2025 16:15:59 -0800 Subject: [PATCH] Fixing test --- .../github/ambry/config/ClusterMapConfig.java | 6 ++--- .../com/github/ambry/config/RouterConfig.java | 15 +++++++++++ .../FrontendRestRequestServiceTest.java | 3 +++ .../ambry/frontend/TtlUpdateHandlerTest.java | 5 +++- .../ambry/router/NonBlockingRouter.java | 25 ++++++++++++++++++- .../github/ambry/quota/QuotaTestUtils.java | 4 +++ .../github/ambry/router/InMemoryRouter.java | 14 +++++++++-- 7 files changed, 65 insertions(+), 7 deletions(-) diff --git a/ambry-api/src/main/java/com/github/ambry/config/ClusterMapConfig.java b/ambry-api/src/main/java/com/github/ambry/config/ClusterMapConfig.java index f5fdcead8a..960a7acd4a 100644 --- a/ambry-api/src/main/java/com/github/ambry/config/ClusterMapConfig.java +++ b/ambry-api/src/main/java/com/github/ambry/config/ClusterMapConfig.java @@ -398,9 +398,9 @@ public ClusterMapConfig(VerifiableProperties verifiableProperties) { verifiableProperties.getEnum(CLUSTERMAP_DATA_NODE_CONFIG_SOURCE_TYPE, DataNodeConfigSourceType.class, DataNodeConfigSourceType.INSTANCE_CONFIG); clusterMapDcsZkConnectStrings = verifiableProperties.getString("clustermap.dcs.zk.connect.strings", ""); - clusterMapClusterName = verifiableProperties.getString(CLUSTERMAP_CLUSTER_NAME); - clusterMapDatacenterName = verifiableProperties.getString(CLUSTERMAP_DATACENTER_NAME); - clusterMapHostName = verifiableProperties.getString(CLUSTERMAP_HOST_NAME); + clusterMapClusterName = verifiableProperties.getString(CLUSTERMAP_CLUSTER_NAME, ""); + clusterMapDatacenterName = verifiableProperties.getString(CLUSTERMAP_DATACENTER_NAME, ""); + clusterMapHostName = verifiableProperties.getString(CLUSTERMAP_HOST_NAME, ""); clusterMapPort = verifiableProperties.getInteger(CLUSTERMAP_PORT, null); clusterMapResolveHostnames = verifiableProperties.getBoolean("clustermap.resolve.hostnames", true); clusterMapDefaultPartitionClass = diff --git a/ambry-api/src/main/java/com/github/ambry/config/RouterConfig.java b/ambry-api/src/main/java/com/github/ambry/config/RouterConfig.java index e38c5b557f..4e3274351d 100644 --- a/ambry-api/src/main/java/com/github/ambry/config/RouterConfig.java +++ b/ambry-api/src/main/java/com/github/ambry/config/RouterConfig.java @@ -149,6 +149,7 @@ public class RouterConfig { public static final String ROUTER_OPERATION_TRACKER_CHECK_ALL_ORIGINATING_REPLICAS_FOR_NOT_FOUND = "router.operation.tracker.check.all.originating.replicas.for.not.found"; public static final String RESERVED_METADATA_ENABLED = "router.reserved.metadata.enabled"; + public static final String CLUSTERMAP_CLUSTER_NAME = ClusterMapConfig.CLUSTERMAP_CLUSTER_NAME; public static final String ROUTER_GET_OPERATION_DEPRIORITIZE_BOOTSTRAP_REPLICAS = "router.get.operation.deprioritize.bootstrap.replicas"; @@ -781,15 +782,29 @@ public class RouterConfig { */ public final String namedBlobDbFactory; + /** + * This is set in frontendConfig until id converter been fully migrate to router. + */ + public final List pathPrefixesToRemove; + + /** + * This is set in clusterMapConfig. + */ + @Config(CLUSTERMAP_CLUSTER_NAME) + public String clusterName; + /** * Create a RouterConfig instance. * @param verifiableProperties the properties map to refer to. */ public RouterConfig(VerifiableProperties verifiableProperties) { FrontendConfig frontendConfig = new FrontendConfig(verifiableProperties); + ClusterMapConfig clusterMapConfig = new ClusterMapConfig(verifiableProperties); idConverterFactory = frontendConfig.idConverterFactory; idSigningServiceFactory = frontendConfig.idSigningServiceFactory; namedBlobDbFactory = frontendConfig.namedBlobDbFactory; + pathPrefixesToRemove = frontendConfig.pathPrefixesToRemove; + clusterName = clusterMapConfig.clusterMapClusterName; routerBlobMetadataCacheId = verifiableProperties.getString(ROUTER_BLOB_METADATA_CACHE_ID, "routerBlobMetadataCache"); routerMaxNumMetadataCacheEntries = diff --git a/ambry-frontend/src/test/java/com/github/ambry/frontend/FrontendRestRequestServiceTest.java b/ambry-frontend/src/test/java/com/github/ambry/frontend/FrontendRestRequestServiceTest.java index 2b4bad10e0..1b2f806c92 100644 --- a/ambry-frontend/src/test/java/com/github/ambry/frontend/FrontendRestRequestServiceTest.java +++ b/ambry-frontend/src/test/java/com/github/ambry/frontend/FrontendRestRequestServiceTest.java @@ -207,6 +207,7 @@ public FrontendRestRequestServiceTest() throws Exception { configProps.setProperty("frontend.enable.undelete", "true"); configProps.setProperty(FrontendConfig.CONTAINER_METRICS_EXCLUDED_ACCOUNTS, "random-name," + excludedAccountName); CommonTestUtils.populateRequiredRouterProps(configProps); + configProps.put("clustermap.cluster.name", clusterName); verifiableProperties = new VerifiableProperties(configProps); clusterMap = new MockClusterMap(); clusterMap.setPermanentMetricRegistry(metricRegistry); @@ -4369,6 +4370,8 @@ public Future convert(RestRequest restRequest, String input, BlobPropert if ((restRequest.getRestMethod() == RestMethod.PUT || restRequest.getRestMethod() == RestMethod.POST) && RestUtils.getRequestPath(restRequest).matchesOperation(Operations.NAMED_BLOB)) { restRequest.setArg(RestUtils.InternalKeys.NAMED_BLOB_VERSION, -1L); + } else { + returnInputIfTranslationNull = true; } return completeOperation(input, blobProperties, callback); } diff --git a/ambry-frontend/src/test/java/com/github/ambry/frontend/TtlUpdateHandlerTest.java b/ambry-frontend/src/test/java/com/github/ambry/frontend/TtlUpdateHandlerTest.java index edd8dfdab0..cbfb578f76 100644 --- a/ambry-frontend/src/test/java/com/github/ambry/frontend/TtlUpdateHandlerTest.java +++ b/ambry-frontend/src/test/java/com/github/ambry/frontend/TtlUpdateHandlerTest.java @@ -89,7 +89,10 @@ public TtlUpdateHandlerTest() throws Exception { AccountAndContainerInjector accountAndContainerInjector = new AccountAndContainerInjector(ACCOUNT_SERVICE, metrics, config); ReadableStreamChannel channel = new ByteBufferReadableStreamChannel(ByteBuffer.wrap(BLOB_DATA)); - router = new InMemoryRouter(new VerifiableProperties(new Properties()), CLUSTER_MAP, idConverterFactory); + Properties props = new Properties(); + props.setProperty("router.hostname", "localhost"); + props.setProperty("router.datacenter.name", "localDC"); + router = new InMemoryRouter(new VerifiableProperties(props), CLUSTER_MAP, idConverterFactory); blobId = router.putBlob(null, BLOB_PROPERTIES, new byte[0], channel, new PutBlobOptionsBuilder().build(), null, QuotaTestUtils.createTestQuotaChargeCallback(QuotaMethod.WRITE)).get(1, TimeUnit.SECONDS); idConverterFactory.translation = blobId; diff --git a/ambry-router/src/main/java/com/github/ambry/router/NonBlockingRouter.java b/ambry-router/src/main/java/com/github/ambry/router/NonBlockingRouter.java index a28b6f05f0..ec2c76fb82 100644 --- a/ambry-router/src/main/java/com/github/ambry/router/NonBlockingRouter.java +++ b/ambry-router/src/main/java/com/github/ambry/router/NonBlockingRouter.java @@ -33,7 +33,9 @@ import com.github.ambry.quota.QuotaChargeCallback; import com.github.ambry.repair.RepairRequestsDb; import com.github.ambry.repair.RepairRequestsDbFactory; +import com.github.ambry.rest.RequestPath; import com.github.ambry.rest.RestRequest; +import com.github.ambry.rest.RestServiceException; import com.github.ambry.rest.RestUtils; import com.github.ambry.store.StoreKey; import com.github.ambry.utils.Time; @@ -43,6 +45,7 @@ import java.io.Closeable; import java.io.IOException; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.Future; @@ -643,7 +646,7 @@ public Future undeleteBlob(String blobId, String serviceId, Callback @Override public Future updateBlobTtl(RestRequest restRequest, String blobId, String serviceId, long expiresAtMs, Callback callback, QuotaChargeCallback quotaChargeCallback) { - if (restRequest.getArgs().get(RestUtils.InternalKeys.BLOB_ID) != null) { + if (restRequest != null && restRequest.getArgs().get(RestUtils.InternalKeys.BLOB_ID) != null) { blobId = restRequest.getArgs().get(RestUtils.InternalKeys.BLOB_ID).toString(); } FutureResult futureResult = new FutureResult<>(); @@ -664,6 +667,7 @@ public void onCompletion(String convertedBlobId, Exception exception) { if (exception != null) { wrappedCallback.onCompletion(null, exception); } else { + //make sure the blobId does not have the cluster prefix proceedWithTtlUpdate(convertedBlobId, serviceId, expiresAtMs, wrappedCallback, futureResult, quotaChargeCallback); } @@ -678,6 +682,12 @@ public void onCompletion(String convertedBlobId, Exception exception) { return futureResult; } + public String stripPrefixAndExtension(String blobId) throws RestServiceException { + return RestUtils.stripSlashAndExtensionFromId( + RequestPath.parse(blobId, Collections.emptyMap(), routerConfig.pathPrefixesToRemove, routerConfig.clusterName) + .getOperationOrBlobId(false)); + } + /** * Helper method to perform TTL update once blobId is available */ @@ -687,9 +697,22 @@ private void proceedWithTtlUpdate(String blobId, String serviceId, long expiresA routerMetrics.updateBlobTtlOperationRate.mark(); routerMetrics.operationQueuingRate.mark(); + + if (blobId == null) { throw new IllegalArgumentException("blobId must not be null"); } + + //make sure the blobId does not have the cluster prefix + try { + blobId = stripPrefixAndExtension(blobId); + } catch (RestServiceException e) { + RouterException routerException = + new RouterException("TtlUpdateOperation failed because of blobId can't be stripped" + blobId, + RouterErrorCode.InvalidBlobId); + completeOperation(futureResult, callback, null, routerException); + } + if (isOpen.get()) { if (notFoundCache.getIfPresent(blobId) != null) { // If we know that the blob doesn't exist, complete the operation. diff --git a/ambry-test-utils/src/main/java/com/github/ambry/quota/QuotaTestUtils.java b/ambry-test-utils/src/main/java/com/github/ambry/quota/QuotaTestUtils.java index 5fa37303d1..35b09e93bd 100644 --- a/ambry-test-utils/src/main/java/com/github/ambry/quota/QuotaTestUtils.java +++ b/ambry-test-utils/src/main/java/com/github/ambry/quota/QuotaTestUtils.java @@ -15,6 +15,7 @@ import com.github.ambry.account.Account; import com.github.ambry.account.Container; +import com.github.ambry.config.ClusterMapConfig; import com.github.ambry.config.QuotaConfig; import com.github.ambry.config.RouterConfig; import com.github.ambry.config.VerifiableProperties; @@ -27,6 +28,8 @@ import org.json.JSONArray; import org.json.JSONObject; +import static com.github.ambry.config.ClusterMapConfig.*; + /** * Utils for testing and initializing quota. @@ -64,6 +67,7 @@ public static RouterConfig getDefaultRouterConfig() { Properties properties = new Properties(); properties.setProperty(RouterConfig.ROUTER_HOSTNAME, "localhost"); properties.setProperty(RouterConfig.ROUTER_DATACENTER_NAME, "DEV"); + properties.setProperty(RouterConfig.CLUSTERMAP_CLUSTER_NAME, "ambry-test"); return new RouterConfig(new VerifiableProperties(properties)); } diff --git a/ambry-test-utils/src/main/java/com/github/ambry/router/InMemoryRouter.java b/ambry-test-utils/src/main/java/com/github/ambry/router/InMemoryRouter.java index e0068f4ed3..800df45b2c 100644 --- a/ambry-test-utils/src/main/java/com/github/ambry/router/InMemoryRouter.java +++ b/ambry-test-utils/src/main/java/com/github/ambry/router/InMemoryRouter.java @@ -29,7 +29,9 @@ import com.github.ambry.notification.NotificationSystem; import com.github.ambry.protocol.GetOption; import com.github.ambry.quota.QuotaChargeCallback; +import com.github.ambry.rest.RequestPath; import com.github.ambry.rest.RestRequest; +import com.github.ambry.rest.RestServiceException; import com.github.ambry.rest.RestUtils; import com.github.ambry.store.StoreKey; import com.github.ambry.utils.Pair; @@ -370,7 +372,7 @@ public void onCompletion(String convertedBlobId, Exception exception) { public Future updateBlobTtl(RestRequest restRequest, String blobId, String serviceId, long expiresAtMs, Callback callback, QuotaChargeCallback quotaChargeCallback) { //if put before update ttl, this will help avoid go through id converter - if (restRequest.getArgs().get(RestUtils.InternalKeys.BLOB_ID) != null) { + if (restRequest != null && restRequest.getArgs().get(RestUtils.InternalKeys.BLOB_ID) != null) { blobId = restRequest.getArgs().get(RestUtils.InternalKeys.BLOB_ID).toString(); } FutureResult futureResult = new FutureResult<>(); @@ -384,7 +386,7 @@ public Future updateBlobTtl(RestRequest restRequest, String blobId, String Callback wrappedCallback = restRequest != null ? createIdConverterCallbackForTtlUpdate(restRequest, blobId, futureResult, stringCallback) : callback; - if (restRequest == null) { + if (restRequest == null || restRequest.getArgs().get(RestUtils.InternalKeys.BLOB_ID) != null) { proceedWithTtlUpdate(blobId, serviceId, expiresAtMs, wrappedCallback, futureResult); } else { try { @@ -407,6 +409,12 @@ public void onCompletion(String convertedBlobId, Exception exception) { return futureResult; } + public String stripPrefixAndExtension(String blobId) throws RestServiceException { + return RestUtils.stripSlashAndExtensionFromId( + RequestPath.parse(blobId, Collections.emptyMap(), getRouterConfig().pathPrefixesToRemove, + getRouterConfig().clusterName).getOperationOrBlobId(false)); + } + /** * Helper method to perform TTL update once blobId is available */ @@ -421,6 +429,8 @@ private void proceedWithTtlUpdate(String blobId, String serviceId, long expiresA Exception exception = null; try { + //make sure the blobId does not have the cluster prefix + blobId = stripPrefixAndExtension(blobId); // Check blobId before performing the update checkBlobId(blobId);