Fix model delete failed after model undeployed

[zane-neo]

Description

When deleting a model, we need to check the model status by querying the model index: https://github.com/opensearch-project/ml-commons/blob/main/plugin/src/main/java/org/opensearch/ml/action/models/DeleteModelTransportAction.java#L108, which means when undepdeloying model response returns, we have to make sure the model status in index is updated to DELETED.

But in UndeployModelTransportAction there's no such guarantee, when updating the model index: https://github.com/opensearch-project/ml-commons/blob/main/plugin/src/main/java/org/opensearch/ml/action/undeploy/TransportUndeployModelAction.java#L192, the response returns almost at the same time when the each node's undeploy finished: https://github.com/opensearch-project/OpenSearch/blob/main/server/src/main/java/org/opensearch/action/support/nodes/TransportNodesAction.java#L315. Which means updating index and response to client run in parallel, so this issue happens occasionally.

So we need a mechanism to guarantee only when the update model index is completed, the response to client get run by adding a countdownlatch to block the response client action when updating index. This is safe to block since this newResponse method runs in a threadpool instead of the event loop threads. I also changed the threadpool to model deploy threadpool which is dedicated for deploy/undeploy model instead of management threadpool.

Issues Resolved

#2419
#2560

Check List

• New functionality includes testing.
  • All tests pass
• New functionality has been documented.
  • New functionality has javadoc added
• Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[peterzhuamazon]

Please rebase to take changes from #2627. Thanks.

[Hailong-am]

what's the root cause?

[zane-neo]

what's the root cause?

Added more details in the description.

[Zhangxunmt]

Which flaky IT is fixed from this change?

[Zhangxunmt]

Is it possible that multiple Undeploy actions blocking the Deploy_Thread_Pool will cause "Deploy" to fail?

[zane-neo]

Most of cases, the update index completes immediately which means there's no actual blocking here. In edge cases, if the update completes relatively slow, the requests will be queued. In a more edge case, the queue is full, then the deploy model will fail. But this usually mean the system is not in a correct status(updating index taking too long time). Also this failure is recoverable which doesn't seem a critical issue.

[Zhangxunmt]

Is this 1 second too large? In most cases the update index happen pretty fast.

[zane-neo]

This is a at most value, if the update index happens within 1s, the await will break immediately after it's done. If update takes more than 1s, then await waits at most 1s to break.

[ylwu-amzn]

This will block current thread, which not recommended. We saw issues several times for such blocking thread code

[zane-neo]

If you're not blocking event loop thread, it's safe.

[zane-neo]

Which flaky IT is fixed from this change?

Check this issue: #2560

[ylwu-amzn]

@zane-neo I remember we have add immediate refresh policy . Why that can't work?

[zane-neo]

@zane-neo I remember we have add immediate refresh policy . Why that can't work?

Please check the description, simply put, this is a race condition issue that updating document and responding to user happens at the same time, no sequence guarantee.

[zane-neo]

Checked again on the issue, this fix is not fixing the root cause. When using a script like below to reproduce the issue:

reg_res = client.post('localhost:9200/_plugins/_ml/models/_register', register_model_payload)
model_id = extract_model_id(reg_res)
deploy_res = client.post('localhost:9200/_plugins/_ml/models/{model_id}/_load')
undeploy_res = client.post('localhost:9200/_plugins/_ml/models/{model_id}/_unload')
delete_res = client.post('localhost:9200/_plugins/_ml/models/{model_id}')

This issue can be reproduce occasionally, but if adding time.sleep(1) after deploy_res returned, the issue can't be reproduced anymore.
The reason is deploy model is in an async approach and when client received the response, the model status in index is not guaranteed to deployed, this doesn't have a simple fix since a lot of files are involved in deploy flow.

This issue is not able to reproduced easily on local, so will add logs in the delete model action to clearly show the model status when it's not in a deletable status, and closing this PR now.