not sending failure message when model index isn't present

[dhrubo-os]

Description

[
For a newly created cluster stats api response:

{
    "ml_model_index_status": "non-existent",
    "ml_connector_index_status": "non-existent",
    "ml_model_count": 0,
    "ml_controller_index_status": "non-existent",
    "ml_task_index_status": "non-existent",
    "ml_config_index_status": "green",
    "ml_connector_count": 0,
    "nodes": {
        "TLaJyhhfS0KeqLbdTRMJSA": {
            "ml_request_count": 0,
            "ml_executing_task_count": 0,
            "ml_deployed_model_count": 0,
            "ml_circuit_breaker_trigger_count": 0,
            "ml_jvm_heap_usage": 27,
            "ml_failure_count": 0,
            "algorithms": {},
            "models": {}
        }
    }
}

Profile api response:

{}

]

Issues Resolved

[List any issues this PR will resolve]

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.

[ylwu-amzn]

Can you add more details and test results to description part ?

[dhrubo-os]

Can you add more details and test results to description part ?

added in the description

[ylwu-amzn]

Is this same with the old response?

[dhrubo-os]

Yes.

{}

[jngz-es]

I didn't get why we return empty content here if failing to search hidden models. In an example case, super-admin call profile api, we could continue to exeute MLProfileAction to return profiles even if failed to search hidden models, as user is super-admin.

[dhrubo-os]

This is to keep the experience same. Currently if there's any model in the index/cache, we return an empty object when we call profile api. I'm just keeping the experience same.

[ylwu-amzn]

If failed to search, is it possible the profile API will have other valid response which should be returned to user ? If yes, we should not just return empty {}

[dhrubo-os]

I tested in the newly created cluster (2.13.0) where any model isn't created. If no models in the cluster or the cluster is new, the profile output is {}

[ylwu-amzn]

I mean what if for other reason the search failed, not just for newly created cluster which has no model index

[dhrubo-os]

Even for other reason if the search fails, we need to make sure we aren't exposing any information of hidden model. If the search fails then we don't have any visibility of which one is the hidden model.

[jngz-es]

But super-admin should have permission to get everything even if hidden models, right? We shouldn't block super-admin to get information.

[dhrubo-os]

Yeah that's true. But super admin won't know which one is the hidden model. Because depending on that search we mark hidden model. If the search isn't successful, then that marking won't happen.

[rbhavna]

Shouldn't we check if the failure is due to index not found exception and continue the stats execution? For every other failures, we can throw exception.

[dhrubo-os]

We don't want to block/throw exception for search, because that way the behavior will be different. We want to keep the experience similar like before. Please check the description for the current output.

[rbhavna]

Yeah but what if this is not a new cluster and the model index is already present. But search failed due to some other reason. In that case, we should actually throw an exception right? Just like how we were doing before we added the hidden model filter. Something like this

[rbhavna]

okay makes sense. user should know nothing of hidden model. got it