[BUG]The exception is unfriendly(Mapper parsing exception) when k-NN circuit breaker is triggered

[heemin32]

What is the bug?
During bulk indexing, when k-NN circuit breaker is triggered, client receive mapper_parsing_exception. This makes user hard to understand the actual root cause of the exception until they look in to a log file.

How can one reproduce the bug?

"type": "knn_vector",
"dimension": 256,
"method": {
    "name": "hnsw",
    "engine": "faiss",
    "space_type": "innerproduct",
    "parameters": {
        "ef_construction": 100,
        "m": 16
    }
}

Do bulk indexing of documents

Error message from client side
failed to parse field [passageVector] of type [knn_vector] in document with id 'asdfasdf'. mapper_parsing_exception'
Preview of fields's value: '[1.23232, 2.123232. ...]', statusCode: 400, type: mapper_parsing_exception

Log
MapperParsingException[failed to parse field [passageVector] of type [knn_vector] in document with id 'asdfasdf'. Preview of field's value: '[1.23232, 2.123232, ...]']; nested: IllegalStateException[Indexing knn vector fields is rejected as circuit breaker triggered. Check _opendistro/_knn/stats for detailed state];

What is the expected behavior?
It should provide the root cause of the exception, k-NN circuit breaker in client side error message

[navneet1v]

@heemin32 how this is bug? I would consider this as an enhancement where messaging is not proper.

[heemin32]

It is a bug because the error message is not just vague but incorrect. If the exception is InternalException or ThrottlingException, it is not a bug. However, mapper parsing exception does not encompass circuit breaker exception but those two are entirely different exceptions.

[ryanbogan]

Following a deep dive, we throw the nested circuit breaker exception in KNNVectorFieldMapper here. This method is called within a parseCreateField method. Tracing the method calls, we eventually arrive at the parse method in the FieldMapper class of the OpenSearch repository, which calls the method in a try/catch and wraps any exception that is caught. The parsing action fails because the circuit breaker is triggered, so the exception is technically correct. There are other instances of mappers checking conditions in this method and throwing exceptions if necessary, such as ParentIdFieldMapper and DocCountFieldMapper.

We have to add the circuit breaker check in this method to stop the bulk indexing from occurring. Our access to the bulk ingestion API/process is limited due to most of the logic being implemented in the core repository. The parseCreateField method allows us to check on the circuit breaker before the indexing begins, which prevents additional memory from being used when resources are limited.

That being said, I think we should create a new type of exception to throw instead of an illegal state: KNNCircuitBreakerException. I also think we can improve the wording of the nested exception message to more clearly describe to the user what action is failing and what it means for their cluster. For example, we could clarify the significance of the circuit breaker being triggered (lack of memory) and clarify that the parse action could not be run with the current status of the circuit breaker.

[nwatab]

Does that mean simply reducing bulk batch size could avoid the error?