diff --git a/ambry-account/src/test/java/com/github/ambry/account/MockRouter.java b/ambry-account/src/test/java/com/github/ambry/account/MockRouter.java index 6c72500104..5985ef5de4 100644 --- a/ambry-account/src/test/java/com/github/ambry/account/MockRouter.java +++ b/ambry-account/src/test/java/com/github/ambry/account/MockRouter.java @@ -140,7 +140,7 @@ public Future stitchBlob(BlobProperties blobProperties, byte[] userMetad } @Override - public Future deleteBlob(String blobId, String serviceId, Callback callback, + public Future deleteBlob(RestRequest restRequest, String blobId, String serviceId, Callback callback, QuotaChargeCallback quotaChargeCallback) { lock.lock(); try { diff --git a/ambry-api/src/main/java/com/github/ambry/rest/RestUtils.java b/ambry-api/src/main/java/com/github/ambry/rest/RestUtils.java index 985cac41c6..81472a753f 100644 --- a/ambry-api/src/main/java/com/github/ambry/rest/RestUtils.java +++ b/ambry-api/src/main/java/com/github/ambry/rest/RestUtils.java @@ -504,6 +504,8 @@ public static final class InternalKeys { * content-length header */ public static final String CONTENT_RANGE_LENGTH = KEY_PREFIX + "content-range-length"; + + public static final String BLOB_ID = KEY_PREFIX + "blob-id"; } /** diff --git a/ambry-api/src/main/java/com/github/ambry/router/Router.java b/ambry-api/src/main/java/com/github/ambry/router/Router.java index abee6dde90..52c9c5d3dd 100644 --- a/ambry-api/src/main/java/com/github/ambry/router/Router.java +++ b/ambry-api/src/main/java/com/github/ambry/router/Router.java @@ -84,13 +84,15 @@ Future stitchBlob(BlobProperties blobProperties, byte[] userMetadata, Li /** * Requests for a blob to be deleted asynchronously and invokes the {@link Callback} when the request completes. - * @param blobId The ID of the blob that needs to be deleted. - * @param serviceId The service ID of the service deleting the blob. This can be null if unknown. - * @param callback The {@link Callback} which will be invoked on the completion of a request. + * + * @param restRequest The {@link RestRequest} to delete the blob. + * @param blobId The ID of the blob that needs to be deleted. + * @param serviceId The service ID of the service deleting the blob. This can be null if unknown. + * @param callback The {@link Callback} which will be invoked on the completion of a request. * @param quotaChargeCallback Listener interface to charge quota cost for the operation. * @return A future that would contain information about whether the deletion succeeded or not, eventually. */ - Future deleteBlob(String blobId, String serviceId, Callback callback, + Future deleteBlob(RestRequest restRequest, String blobId, String serviceId, Callback callback, QuotaChargeCallback quotaChargeCallback); /** @@ -212,7 +214,7 @@ default CompletableFuture putBlob(BlobProperties blobProperties, byte[] */ default CompletableFuture deleteBlob(String blobId, String serviceId) { CompletableFuture future = new CompletableFuture<>(); - deleteBlob(blobId, serviceId, CallbackUtils.fromCompletableFuture(future), null); + deleteBlob(null, blobId, serviceId, CallbackUtils.fromCompletableFuture(future), null); return future; } diff --git a/ambry-api/src/test/java/com/github/ambry/rest/MockRestRequestService.java b/ambry-api/src/test/java/com/github/ambry/rest/MockRestRequestService.java index ec07fac3bf..6e784e2372 100644 --- a/ambry-api/src/test/java/com/github/ambry/rest/MockRestRequestService.java +++ b/ambry-api/src/test/java/com/github/ambry/rest/MockRestRequestService.java @@ -170,7 +170,7 @@ public void handlePut(RestRequest restRequest, RestResponseChannel restResponseC public void handleDelete(RestRequest restRequest, RestResponseChannel restResponseChannel) { if (shouldProceed(restRequest, restResponseChannel)) { String blobId = getBlobId(restRequest); - router.deleteBlob(blobId, null, new MockDeleteCallback(this, restRequest, restResponseChannel), null); + router.deleteBlob(null, blobId, null, new MockDeleteCallback(this, restRequest, restResponseChannel), null); } } diff --git a/ambry-frontend/src/main/java/com/github/ambry/frontend/DeleteBlobHandler.java b/ambry-frontend/src/main/java/com/github/ambry/frontend/DeleteBlobHandler.java index 4cc1305569..bfc4ef191a 100644 --- a/ambry-frontend/src/main/java/com/github/ambry/frontend/DeleteBlobHandler.java +++ b/ambry-frontend/src/main/java/com/github/ambry/frontend/DeleteBlobHandler.java @@ -116,40 +116,26 @@ private void start() { private Callback securityProcessRequestCallback() { return buildCallback(metrics.deleteBlobSecurityProcessRequestMetrics, result -> { String blobIdStr = getRequestPath(restRequest).getOperationOrBlobId(false); - idConverter.convert(restRequest, blobIdStr, idConverterCallback()); - LOGGER.debug("Blob Id to convert: " + blobIdStr); - }, restRequest.getUri(), LOGGER, finalCallback); - } - - /** - * After {@link IdConverter#convert} finishes, call {@link SecurityService#postProcessRequest} to perform - * request time security checks that rely on the request being fully parsed and any additional arguments set. - * @return a {@link Callback} to be used with {@link IdConverter#convert}. - */ - private Callback idConverterCallback() { - return buildCallback(metrics.deleteBlobIdConversionMetrics, convertedBlobId -> { - BlobId blobId = FrontendUtils.getBlobIdFromString(convertedBlobId, clusterMap); - if (restRequest.getArgs().get(InternalKeys.TARGET_ACCOUNT_KEY) == null) { - // Inject account and container when they are missing from the rest request. - accountAndContainerInjector.injectTargetAccountAndContainerFromBlobId(blobId, restRequest, - metrics.deleteBlobMetricsGroup); + if (!RequestPath.matchesOperation(blobIdStr, Operations.NAMED_BLOB)) { + blobIdStr = blobIdStr.startsWith("/") ? blobIdStr.substring(1) : blobIdStr; + BlobId convertedBlobId = FrontendUtils.getBlobIdFromString(blobIdStr, clusterMap); + restRequest.setArg(RestUtils.InternalKeys.BLOB_ID, convertedBlobId); + accountAndContainerInjector.injectTargetAccountAndContainerFromBlobId(convertedBlobId, restRequest, + metrics.updateBlobTtlMetricsGroup); } - LOGGER.debug("Converted Blob Id: " + blobId); - securityService.postProcessRequest(restRequest, securityPostProcessRequestCallback(blobId)); + securityService.postProcessRequest(restRequest, securityPostProcessRequestCallback()); }, restRequest.getUri(), LOGGER, finalCallback); } /** * After {@link SecurityService#postProcessRequest} finishes, call {@link Router#deleteBlob} to delete * the blob in the storage layer. - * @param blobId the {@link BlobId} to undelete * @return a {@link Callback} to be used with {@link SecurityService#postProcessRequest}. */ - private Callback securityPostProcessRequestCallback(BlobId blobId) { + private Callback securityPostProcessRequestCallback() { return buildCallback(metrics.deleteBlobSecurityPostProcessRequestMetrics, result -> { String serviceId = RestUtils.getHeader(restRequest.getArgs(), RestUtils.Headers.SERVICE_ID, false); - LOGGER.debug("Start deleting the blob for blobId " + blobId); - router.deleteBlob(blobId.getID(), serviceId, routerCallback(), + router.deleteBlob(restRequest, null, serviceId, routerCallback(), QuotaUtils.buildQuotaChargeCallback(restRequest, quotaManager, false)); }, restRequest.getUri(), LOGGER, finalCallback); } 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 4289252275..d6ed28caa4 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 @@ -3512,7 +3512,8 @@ private void doIdConverterExceptionTest(FrontendTestIdConverterFactory converter accountAndContainerInjector, datacenterName, hostname, clusterName, accountStatsStore, QUOTA_MANAGER); frontendRestRequestService.setupResponseHandler(responseHandler); frontendRestRequestService.start(); - RestMethod[] restMethods = {RestMethod.POST, RestMethod.GET, RestMethod.DELETE, RestMethod.HEAD}; + //Regular blob delete won't need to go through id converter + RestMethod[] restMethods = {RestMethod.POST, RestMethod.GET, RestMethod.HEAD}; doExternalServicesBadInputTest(restMethods, expectedExceptionMsg, false); } @@ -4444,7 +4445,7 @@ public Future stitchBlob(BlobProperties blobProperties, byte[] userMetad } @Override - public Future deleteBlob(String blobId, String serviceId, Callback callback, + public Future deleteBlob(RestRequest restRequest, String blobId, String serviceId, Callback callback, QuotaChargeCallback quotaChargeCallback) { deleteServiceId = serviceId; return completeOperation(null, callback, OpType.DeleteBlob); diff --git a/ambry-frontend/src/test/java/com/github/ambry/frontend/UndeleteHandlerTest.java b/ambry-frontend/src/test/java/com/github/ambry/frontend/UndeleteHandlerTest.java index 2e3e1ea336..2c4b799d6d 100644 --- a/ambry-frontend/src/test/java/com/github/ambry/frontend/UndeleteHandlerTest.java +++ b/ambry-frontend/src/test/java/com/github/ambry/frontend/UndeleteHandlerTest.java @@ -104,7 +104,7 @@ public void setupBlob() throws Exception { blobId = router.putBlob(BLOB_PROPERTIES, new byte[0], channel, new PutBlobOptionsBuilder().build()) .get(1, TimeUnit.SECONDS); idConverterFactory.translation = blobId; - router.deleteBlob(blobId, SERVICE_ID, null, QuotaTestUtils.createTestQuotaChargeCallback(QuotaMethod.WRITE)) + router.deleteBlob(null, blobId, SERVICE_ID, null, QuotaTestUtils.createTestQuotaChargeCallback(QuotaMethod.WRITE)) .get(1, TimeUnit.SECONDS); } 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 389b60ce9e..ebae40d160 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 @@ -34,6 +34,7 @@ import com.github.ambry.repair.RepairRequestsDb; import com.github.ambry.repair.RepairRequestsDbFactory; import com.github.ambry.rest.RestRequest; +import com.github.ambry.rest.RestUtils; import com.github.ambry.store.StoreKey; import com.github.ambry.utils.Time; import com.github.ambry.utils.Utils; @@ -52,6 +53,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static com.github.ambry.rest.RestUtils.*; + /** * Streaming, non-blocking router implementation for Ambry. @@ -471,21 +474,73 @@ public Future stitchBlob(BlobProperties blobProperties, byte[] userMetad /** * Requests for a blob to be deleted asynchronously and invokes the {@link Callback} when the request completes. - * @param blobId The ID of the blob that needs to be deleted. - * @param serviceId The service ID of the service deleting the blob. This can be null if unknown. - * @param callback The {@link Callback} which will be invoked on the completion of a request. + * + * @param restRequest The {@link RestRequest} to delete the blob. + * @param blobId The ID of the blob that needs to be deleted. + * @param serviceId The service ID of the service deleting the blob. This can be null if unknown. + * @param callback The {@link Callback} which will be invoked on the completion of a request. * @return A future that would contain information about whether the deletion succeeded or not, eventually. */ @Override - public Future deleteBlob(String blobId, String serviceId, Callback callback, + public Future deleteBlob(RestRequest restRequest, String blobId, String serviceId, Callback callback, QuotaChargeCallback quotaChargeCallback) { - if (blobId == null) { - throw new IllegalArgumentException("blobId must not be null"); + FutureResult futureResult = new FutureResult<>(); + if (restRequest == null) { + if (blobId == null) { + throw new IllegalArgumentException("blobId must not be null"); + } + proceedWithDelete(blobId, serviceId, callback, futureResult, quotaChargeCallback); + } else { + //if the blobId is not named blob based, it could bypass the first round of d converter logic by checking InternalKeys.BLOB_ID. + //First round is to convert named blob to blob Id. + if (restRequest.getArgs().get(RestUtils.InternalKeys.BLOB_ID) != null) { + String blobIdStr = + removeLeadingSlashIfNeeded(restRequest.getArgs().get(RestUtils.InternalKeys.BLOB_ID).toString()); + proceedWithDelete(blobIdStr, serviceId, callback, futureResult, quotaChargeCallback); + } else { + try { + //If the blobId is named blob, need to go through convert first. + String blobIdStr = getRequestPath(restRequest).getOperationOrBlobId(true); + + // Convert asynchronously and proceed once blobId is available + idConverter.convert(restRequest, blobIdStr, null, new Callback() { + @Override + public void onCompletion(String convertedBlobId, Exception exception) { + if (exception != null) { + callback.onCompletion(null, exception); + } else { + // Call proceedWithTtlUpdate once blobId is available + proceedWithDelete(convertedBlobId, serviceId, callback, futureResult, quotaChargeCallback); + } + } + }); + return futureResult; // Return early since we're waiting for the async operation + } catch (Exception e) { + callback.onCompletion(null, e); + return futureResult; + } + } } + return futureResult; + } + + private String removeLeadingSlashIfNeeded(String blobId) { + return blobId.startsWith("/") ? blobId.substring(1) : blobId; + } + + /** + * Helper method to perform delete once the blob Id is available. + */ + private void proceedWithDelete(String blobId, String serviceId, Callback callback, + FutureResult futureResult, QuotaChargeCallback quotaChargeCallback) { currentOperationsCount.incrementAndGet(); routerMetrics.deleteBlobOperationRate.mark(); routerMetrics.operationQueuingRate.mark(); - FutureResult futureResult = new FutureResult<>(); + + if (blobId == null) { + throw new IllegalArgumentException("blobId must not be null"); + } + if (isOpen.get()) { if (notFoundCache.getIfPresent(blobId) != null) { // If we know that blob doesn't exist, complete the operation @@ -509,7 +564,6 @@ public Future deleteBlob(String blobId, String serviceId, Callback c routerMetrics.onDeleteBlobError(routerException); completeOperation(futureResult, callback, null, routerException); } - return futureResult; } /** @@ -579,7 +633,8 @@ public Future updateBlobTtl(RestRequest restRequest, String blobId, String callback.onCompletion(null, exception); }; Callback wrappedCallback = - restRequest != null ? createIdConverterCallbackForTtlUpdate(restRequest, blobId, futureResult, stringCallback) : callback; + restRequest != null ? createIdConverterCallbackForTtlUpdateAndDelete(restRequest, blobId, futureResult, + stringCallback) : callback; if (isOpen.get()) { if (notFoundCache.getIfPresent(blobId) != null) { // If we know that blob doesn't exist, complete the operation. @@ -853,7 +908,7 @@ private Callback createIdConverterCallbackForPut(RestRequest restRequest * @param blobId the blobId to update ttl. * @return */ - private Callback createIdConverterCallbackForTtlUpdate(RestRequest restRequest, String blobId, + private Callback createIdConverterCallbackForTtlUpdateAndDelete(RestRequest restRequest, String blobId, FutureResult futureResult, Callback callback) { return (result, exception) -> { if (exception != null) { diff --git a/ambry-router/src/test/java/com/github/ambry/router/DeleteManagerTest.java b/ambry-router/src/test/java/com/github/ambry/router/DeleteManagerTest.java index 982ae553fc..a7da4d327e 100644 --- a/ambry-router/src/test/java/com/github/ambry/router/DeleteManagerTest.java +++ b/ambry-router/src/test/java/com/github/ambry/router/DeleteManagerTest.java @@ -174,7 +174,7 @@ public void testAndAssert(RouterErrorCode expectedError) throws Exception { List futures = new ArrayList<>(); for (int i = 0; i < 5; i++) { if (i == 1) { - futures.add(router.deleteBlob(blobIdString, null, new Callback() { + futures.add(router.deleteBlob(null, blobIdString, null, new Callback() { @Override public void onCompletion(Void result, Exception exception) { callbackCalled.countDown(); @@ -675,7 +675,7 @@ RouterErrorCode.OperationTimedOut, new ErrorCodeChecker() { public void testAndAssert(RouterErrorCode expectedError) throws Exception { CountDownLatch operationCompleteLatch = new CountDownLatch(1); future = - router.deleteBlob(blobIdString, null, new ClientCallback(operationCompleteLatch), quotaChargeCallback); + router.deleteBlob(null, blobIdString, null, new ClientCallback(operationCompleteLatch), quotaChargeCallback); do { // increment mock time mockTime.sleep(1000); @@ -707,7 +707,7 @@ public void testSelectorError() throws Exception { mockSelectorState.set(state); setServerErrorCodes(serverErrorCodes, partition, serverLayout, localDc); CountDownLatch operationCompleteLatch = new CountDownLatch(1); - future = router.deleteBlob(blobIdString, null, new ClientCallback(operationCompleteLatch), quotaChargeCallback); + future = router.deleteBlob(null, blobIdString, null, new ClientCallback(operationCompleteLatch), quotaChargeCallback); do { // increment mock time mockTime.sleep(1000); diff --git a/ambry-router/src/test/java/com/github/ambry/router/NonBlockingQuotaTest.java b/ambry-router/src/test/java/com/github/ambry/router/NonBlockingQuotaTest.java index 4b7319167e..117ce678cb 100644 --- a/ambry-router/src/test/java/com/github/ambry/router/NonBlockingQuotaTest.java +++ b/ambry-router/src/test/java/com/github/ambry/router/NonBlockingQuotaTest.java @@ -218,7 +218,7 @@ public QuotaConfig getQuotaConfig() { router.getBlob(blobId, new GetBlobOptionsBuilder().operationType(GetBlobOptions.OperationType.BlobInfo).build(), null, quotaChargeCallback).get(); assertEquals(expectedChargeCallbackCount += quotaAccountingSize, listenerCalledCount.get()); - router.deleteBlob(blobId, null, null, quotaChargeCallback).get(); + router.deleteBlob(null, blobId, null, null, quotaChargeCallback).get(); assertEquals(expectedChargeCallbackCount += quotaAccountingSize, listenerCalledCount.get()); try { router.getBlob(blobId, new GetBlobOptionsBuilder().build(), null, quotaChargeCallback).get(); @@ -273,7 +273,7 @@ public QuotaConfig getQuotaConfig() { assertEquals(0, routerMetrics.updateOptimizedCount.getCount()); assertEquals(1, routerMetrics.updateUnOptimizedCount.getCount()); - router.deleteBlob(stitchedBlobId, null, null, quotaChargeCallback).get(); + router.deleteBlob(null, stitchedBlobId, null, null, quotaChargeCallback).get(); assertEquals(expectedChargeCallbackCount + quotaAccountingSize, listenerCalledCount.get()); } finally { router.close(); @@ -363,7 +363,7 @@ public QuotaConfig getQuotaConfig() { assertEquals(1, routerMetrics.updateOptimizedCount.getCount()); assertEquals(0, routerMetrics.updateUnOptimizedCount.getCount()); - router.deleteBlob(compositeBlobId, null, null, quotaChargeCallback).get(); + router.deleteBlob(null, compositeBlobId, null, null, quotaChargeCallback).get(); assertEquals(expectedChargeCallbackCount + quotaAccountingSize, listenerCalledCount.get()); } finally { router.close(); diff --git a/ambry-router/src/test/java/com/github/ambry/router/NonBlockingRouterTest.java b/ambry-router/src/test/java/com/github/ambry/router/NonBlockingRouterTest.java index 95a107c177..19a0241d3f 100644 --- a/ambry-router/src/test/java/com/github/ambry/router/NonBlockingRouterTest.java +++ b/ambry-router/src/test/java/com/github/ambry/router/NonBlockingRouterTest.java @@ -2380,7 +2380,7 @@ void routerMetadataCacheErrorOnDeleteCompositeBlob(ServerErrorCode serverErrorCo // failed delete mockServerLayout.getMockServers().forEach(mockServer -> mockServer.setServerErrorForAllRequests(serverErrorCode)); TestCallback testCallback = new TestCallback<>(); - Future future = router.deleteBlob(blobId, null, testCallback, null); + Future future = router.deleteBlob(null, blobId, null, testCallback, null); assertFailureAndCheckErrorCode(future, testCallback, routerErrorCode); assertNotNull("Blob metadata must be present in metadata cache", router.getBlobMetadataCache().getObject(blobId)); mockServerLayout.getMockServers().forEach(mockServer -> mockServer.resetServerErrors()); @@ -2455,7 +2455,7 @@ public void testBlobNotFoundCache() throws Exception { assertFailureAndCheckErrorCode(ttlUpdateFuture, ttlUpdateTestCallback, RouterErrorCode.BlobDoesNotExist); TestCallback deleteTestCallback = new TestCallback<>(); - Future deleteFuture = router.deleteBlob(blobId, updateServiceId, deleteTestCallback, null); + Future deleteFuture = router.deleteBlob(null, blobId, updateServiceId, deleteTestCallback, null); assertFailureAndCheckErrorCode(deleteFuture, deleteTestCallback, RouterErrorCode.BlobDoesNotExist); TestCallback unDeleteTestCallback = new TestCallback<>(); diff --git a/ambry-server/src/integration-test/java/com/github/ambry/server/RouterServerTestFramework.java b/ambry-server/src/integration-test/java/com/github/ambry/server/RouterServerTestFramework.java index 00670deba8..f1ebf2d4fb 100644 --- a/ambry-server/src/integration-test/java/com/github/ambry/server/RouterServerTestFramework.java +++ b/ambry-server/src/integration-test/java/com/github/ambry/server/RouterServerTestFramework.java @@ -429,7 +429,7 @@ void check() throws Exception { opChain.testFutures.add(testFuture); return; } - Future future = router.deleteBlob(fraudId.getID(), null, callback, quotaChargeCallback); + Future future = router.deleteBlob(null, fraudId.getID(), null, callback, quotaChargeCallback); TestFuture testFuture = new TestFuture(future, genLabel("deleteBlobAuthorizationFail", true), opChain) { @Override void check() throws Exception { @@ -463,7 +463,7 @@ void check() throws Exception { */ private void startDeleteBlob(final OperationChain opChain) { Callback callback = new TestCallback<>(opChain, false); - Future future = router.deleteBlob(opChain.blobId, null, callback, quotaChargeCallback); + Future future = router.deleteBlob(null, opChain.blobId, null, callback, quotaChargeCallback); TestFuture testFuture = new TestFuture(future, genLabel("deleteBlob", false), opChain) { @Override void check() throws Exception { 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 dc4991c734..e7a79ac58c 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 @@ -30,6 +30,7 @@ import com.github.ambry.protocol.GetOption; import com.github.ambry.quota.QuotaChargeCallback; import com.github.ambry.rest.RestRequest; +import com.github.ambry.rest.RestUtils; import com.github.ambry.store.StoreKey; import com.github.ambry.utils.Pair; import com.github.ambry.utils.SystemTime; @@ -52,6 +53,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import static com.github.ambry.rest.RestUtils.*; import static com.github.ambry.utils.Utils.*; @@ -305,30 +307,46 @@ public Future stitchBlob(BlobProperties blobProperties, byte[] userMetad } @Override - public Future deleteBlob(String blobId, String serviceId, Callback callback, QuotaChargeCallback quotaChargeCallback) { + public Future deleteBlob(RestRequest restRequest, String blobId, String serviceId, Callback callback, QuotaChargeCallback quotaChargeCallback) { FutureResult futureResult = new FutureResult<>(); if (!handlePrechecks(futureResult, callback)) { return futureResult; } - Exception exception = null; - try { - checkBlobId(blobId); - if (!deletedBlobs.contains(blobId) && blobs.containsKey(blobId)) { - deletedBlobs.add(blobId); - undeletedBlobs.remove(blobId); - if (notificationSystem != null) { - notificationSystem.onBlobDeleted(blobId, serviceId, null, null); + if (restRequest == null) { + proceedWithDelete(blobId, serviceId, callback, futureResult); + } else { + // Extract blobId or convert it if necessary + if (restRequest.getArgs().get(RestUtils.InternalKeys.BLOB_ID) != null) { + // Blob ID is already available + String blobIdStr = + removeLeadingSlashIfNeeded(restRequest.getArgs().get(RestUtils.InternalKeys.BLOB_ID).toString()); + proceedWithDelete(blobIdStr, serviceId, callback, futureResult); + } else { + try { + //If the blobId is named blob, need to go through convert first. + String blobIdStr = getRequestPath(restRequest).getOperationOrBlobId(true); + + // Call idConverter to get blobId asynchronously + idConverter.convert(restRequest, blobIdStr, null, new Callback() { + @Override + public void onCompletion(String convertedBlobId, Exception exception) { + if (exception != null) { + // Handle error in conversion + callback.onCompletion(null, exception); + } else { + // Continue with TTL update once blobId is available + proceedWithDelete(convertedBlobId, serviceId, callback, futureResult); + } + } + }); + } catch (Exception e) { + // Handle synchronous errors during header extraction + callback.onCompletion(null, e); + return futureResult; } - } else if (!deletedBlobs.contains(blobId)) { - exception = new RouterException("Blob not found", RouterErrorCode.BlobDoesNotExist); } - } catch (RouterException e) { - exception = e; - } catch (Exception e) { - exception = new RouterException(e, RouterErrorCode.UnexpectedInternalError); - } finally { - completeOperation(futureResult, callback, null, exception); } + // Blob ID is not available, use idConverter to get it return futureResult; } @@ -554,6 +572,40 @@ protected static void completeOperation(FutureResult futureResult, Callback call } } + private String removeLeadingSlashIfNeeded(String blobId) { + return blobId.startsWith("/") ? blobId.substring(1) : blobId; + } + + /** + * Helper method to perform delete once the blob Id is available. + */ + private void proceedWithDelete(String blobId, String serviceId, Callback callback, + FutureResult futureResult) { + if (blobId == null) { + throw new IllegalArgumentException("blobId must not be null"); + } + Exception exception = null; + + try { + checkBlobId(blobId); + if (!deletedBlobs.contains(blobId) && blobs.containsKey(blobId)) { + deletedBlobs.add(blobId); + undeletedBlobs.remove(blobId); + if (notificationSystem != null) { + notificationSystem.onBlobDeleted(blobId, serviceId, null, null); + } + } else if (!deletedBlobs.contains(blobId)) { + exception = new RouterException("Blob not found", RouterErrorCode.BlobDoesNotExist); + } + } catch (RouterException e) { + exception = e; + } catch (Exception e) { + exception = new RouterException(e, RouterErrorCode.UnexpectedInternalError); + } finally { + completeOperation(futureResult, callback, null, exception); + } + } + /** * Thread to read the post data async and store it. */ diff --git a/ambry-tools/src/main/java/com/github/ambry/tools/admin/ConcurrencyTestTool.java b/ambry-tools/src/main/java/com/github/ambry/tools/admin/ConcurrencyTestTool.java index 6302d1df47..d97c37786f 100644 --- a/ambry-tools/src/main/java/com/github/ambry/tools/admin/ConcurrencyTestTool.java +++ b/ambry-tools/src/main/java/com/github/ambry/tools/admin/ConcurrencyTestTool.java @@ -838,7 +838,7 @@ public Future deleteBlobAndValidate(final String blobId, final RouterErrorCode e final FutureResult futureResult = new FutureResult(); try { final Long startTimeGetBlobInMs = SystemTime.getInstance().milliseconds(); - router.deleteBlob(blobId, null, new Callback() { + router.deleteBlob(null, blobId, null, new Callback() { @Override public void onCompletion(Void result, Exception exception) { long latencyPerBlob = SystemTime.getInstance().milliseconds() - startTimeGetBlobInMs; diff --git a/ambry-tools/src/main/java/com/github/ambry/tools/perf/rest/PerfRouter.java b/ambry-tools/src/main/java/com/github/ambry/tools/perf/rest/PerfRouter.java index a80a3d4ffd..b2226e52e2 100644 --- a/ambry-tools/src/main/java/com/github/ambry/tools/perf/rest/PerfRouter.java +++ b/ambry-tools/src/main/java/com/github/ambry/tools/perf/rest/PerfRouter.java @@ -156,13 +156,15 @@ public Future stitchBlob(BlobProperties blobProperties, byte[] userMetad /** * Does nothing. Simply indicates success immediately. - * @param blobId (ignored). - * @param serviceId (ignored). - * @param callback the {@link Callback} to invoke on operation completion. + * + * @param restRequest The {@link RestRequest} of deleteBlob. + * @param blobId (ignored). + * @param serviceId (ignored). + * @param callback the {@link Callback} to invoke on operation completion. * @return a {@link FutureResult} that will eventually contain the result of the operation. */ @Override - public Future deleteBlob(String blobId, String serviceId, Callback callback, + public Future deleteBlob(RestRequest restRequest, String blobId, String serviceId, Callback callback, QuotaChargeCallback quotaChargeCallback) { logger.trace("Received deleteBlob call"); FutureResult futureResult = new FutureResult();