Skip to content

Commit

Permalink
Fixing test
Browse files Browse the repository at this point in the history
  • Loading branch information
Sophie Guo committed Jan 17, 2025
1 parent 7416e93 commit c165b21
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
15 changes: 15 additions & 0 deletions ambry-api/src/main/java/com/github/ambry/config/RouterConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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<String> 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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -4369,6 +4370,8 @@ public Future<String> 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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -643,7 +646,7 @@ public Future<Void> undeleteBlob(String blobId, String serviceId, Callback<Void>
@Override
public Future<Void> updateBlobTtl(RestRequest restRequest, String blobId, String serviceId, long expiresAtMs,
Callback<Void> 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<Void> futureResult = new FutureResult<>();
Expand All @@ -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);
}
Expand All @@ -678,18 +682,34 @@ 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
*/
private void proceedWithTtlUpdate(String blobId, String serviceId, long expiresAtMs,
Callback<Void> callback, FutureResult<Void> futureResult, QuotaChargeCallback quotaChargeCallback) {
if (blobId == null) {
throw new IllegalArgumentException("blobId must not be null");
}
currentOperationsCount.incrementAndGet();
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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -370,7 +372,7 @@ public void onCompletion(String convertedBlobId, Exception exception) {
public Future<Void> updateBlobTtl(RestRequest restRequest, String blobId, String serviceId, long expiresAtMs,
Callback<Void> 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<Void> futureResult = new FutureResult<>();
Expand All @@ -384,7 +386,7 @@ public Future<Void> updateBlobTtl(RestRequest restRequest, String blobId, String
Callback<Void> 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 {
Expand All @@ -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
*/
Expand All @@ -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);

Expand Down

0 comments on commit c165b21

Please sign in to comment.