diff --git a/plugin/src/main/java/org/opensearch/ml/action/models/DeleteModelTransportAction.java b/plugin/src/main/java/org/opensearch/ml/action/models/DeleteModelTransportAction.java index fa6d41f27a..464faecdeb 100644 --- a/plugin/src/main/java/org/opensearch/ml/action/models/DeleteModelTransportAction.java +++ b/plugin/src/main/java/org/opensearch/ml/action/models/DeleteModelTransportAction.java @@ -41,7 +41,6 @@ import org.opensearch.index.reindex.BulkByScrollResponse; import org.opensearch.index.reindex.DeleteByQueryAction; import org.opensearch.index.reindex.DeleteByQueryRequest; -import org.opensearch.ml.common.FunctionName; import org.opensearch.ml.common.MLModel; import org.opensearch.ml.common.model.MLModelState; import org.opensearch.ml.common.transport.model.MLModelDeleteAction; @@ -116,7 +115,6 @@ protected void doExecute(Task task, ActionRequest request, ActionListener { wrappedListener.onFailure((new OpenSearchStatusException("Failed to find model", RestStatus.NOT_FOUND))); })); } catch (Exception e) { @@ -214,20 +212,20 @@ private void returnFailure(BulkByScrollResponse response, String modelId, Action actionListener.onFailure(new OpenSearchStatusException(errorMessage, RestStatus.INTERNAL_SERVER_ERROR)); } - private void deleteModel(String modelId, FunctionName functionName, ActionListener actionListener) { + private void deleteModel(String modelId, ActionListener actionListener) { DeleteRequest deleteRequest = new DeleteRequest(ML_MODEL_INDEX, modelId).setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE); client.delete(deleteRequest, new ActionListener<>() { @Override public void onResponse(DeleteResponse deleteResponse) { - deleteModelChunksAndController(functionName, actionListener, modelId, deleteResponse); + deleteModelChunksAndController(actionListener, modelId, deleteResponse); } @Override public void onFailure(Exception e) { if (e instanceof ResourceNotFoundException) { - deleteModelChunksAndController(functionName, actionListener, modelId, null); + deleteModelChunksAndController(actionListener, modelId, null); } else { - log.error("Failed to delete model meta data for model, please try again: " + modelId, e); + log.error("Model is not all cleaned up, please try again: " + modelId, e); actionListener.onFailure(e); } } @@ -235,58 +233,36 @@ public void onFailure(Exception e) { } private void deleteModelChunksAndController( - FunctionName functionName, ActionListener actionListener, String modelId, DeleteResponse deleteResponse ) { - if (FunctionName.REMOTE != functionName) { - CountDownLatch countDownLatch = new CountDownLatch(2); - AtomicBoolean bothDeleted = new AtomicBoolean(true); - ActionListener countDownActionListener = ActionListener.wrap(b -> { - countDownLatch.countDown(); - bothDeleted.compareAndSet(true, b); - if (countDownLatch.getCount() == 0) { - if (bothDeleted.get()) { - log.debug("model chunks and model controller for model {} deleted successfully", modelId); - if (deleteResponse != null) { - actionListener.onResponse(deleteResponse); - } else { - actionListener.onFailure(new OpenSearchStatusException("Failed to find model", RestStatus.NOT_FOUND)); - } + CountDownLatch countDownLatch = new CountDownLatch(2); + AtomicBoolean bothDeleted = new AtomicBoolean(true); + ActionListener countDownActionListener = ActionListener.wrap(b -> { + countDownLatch.countDown(); + bothDeleted.compareAndSet(true, b); + if (countDownLatch.getCount() == 0) { + if (bothDeleted.get()) { + log.debug("model chunks and model controller for model {} deleted successfully", modelId); + if (deleteResponse != null) { + actionListener.onResponse(deleteResponse); } else { - actionListener - .onFailure( - new IllegalStateException("Failed to delete model chunks or model controller, please try again: " + modelId) - ); + actionListener.onFailure(new OpenSearchStatusException("Failed to find model", RestStatus.NOT_FOUND)); } - } - }, e -> { - countDownLatch.countDown(); - bothDeleted.compareAndSet(true, false); - if (countDownLatch.getCount() == 0) { - actionListener - .onFailure( - new IllegalStateException("Failed to delete model chunks or model controller, please try again: " + modelId, e) - ); - } - }); - deleteModelChunks(modelId, countDownActionListener); - deleteController(modelId, countDownActionListener); - } else { - ActionListener deleteControllerListener = ActionListener.wrap(b -> { - log.debug("model controller for model {} deleted successfully", modelId); - if (deleteResponse != null) { - actionListener.onResponse(deleteResponse); } else { - actionListener.onFailure(new OpenSearchStatusException("Failed to find model", RestStatus.NOT_FOUND)); + actionListener.onFailure(new IllegalStateException("Model is not all cleaned up, please try again: " + modelId)); } - }, e -> { - log.error("Failed to delete model controller, please try again: " + modelId, e); - actionListener.onFailure(new IllegalStateException("Failed to delete model controller, please try again: " + modelId, e)); - }); - deleteController(modelId, deleteControllerListener); - } + } + }, e -> { + countDownLatch.countDown(); + bothDeleted.compareAndSet(true, false); + if (countDownLatch.getCount() == 0) { + actionListener.onFailure(new IllegalStateException("Model is not all cleaned up, please try again: " + modelId, e)); + } + }); + deleteModelChunks(modelId, countDownActionListener); + deleteController(modelId, countDownActionListener); } /** diff --git a/plugin/src/test/java/org/opensearch/ml/action/models/DeleteModelTransportActionTests.java b/plugin/src/test/java/org/opensearch/ml/action/models/DeleteModelTransportActionTests.java index 791b2f43d0..37c6f8755f 100644 --- a/plugin/src/test/java/org/opensearch/ml/action/models/DeleteModelTransportActionTests.java +++ b/plugin/src/test/java/org/opensearch/ml/action/models/DeleteModelTransportActionTests.java @@ -243,10 +243,7 @@ public void testDeleteLocalModel_deleteModelController_failed() throws IOExcepti deleteModelTransportAction.doExecute(null, mlModelDeleteRequest, actionListener); ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Exception.class); verify(actionListener).onFailure(argumentCaptor.capture()); - assertEquals( - "Failed to delete model chunks or model controller, please try again: test_id", - argumentCaptor.getValue().getMessage() - ); + assertEquals("Model is not all cleaned up, please try again: test_id", argumentCaptor.getValue().getMessage()); } public void testDeleteRemoteModel_deleteModelChunks_failed() throws IOException { @@ -272,10 +269,7 @@ public void testDeleteRemoteModel_deleteModelChunks_failed() throws IOException deleteModelTransportAction.doExecute(null, mlModelDeleteRequest, actionListener); ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Exception.class); verify(actionListener).onFailure(argumentCaptor.capture()); - assertEquals( - "Failed to delete model chunks or model controller, please try again: test_id", - argumentCaptor.getValue().getMessage() - ); + assertEquals("Model is not all cleaned up, please try again: test_id", argumentCaptor.getValue().getMessage()); } public void testDeleteHiddenModel_Success() throws IOException {