elastic / elasticsearch

Free and Open Source, Distributed, RESTful Search Engine
https://www.elastic.co/products/elasticsearch
Other
69.72k stars 24.67k forks source link

Stop calling ElasticsearchMappings#addDocMappingIfMissing from transport thread #87911

Open DaveCTurner opened 2 years ago

DaveCTurner commented 2 years ago

In #87679 we forbid calls to Metadata#findMappings on the latency-sensitive transport threads because it can be rather expensive. This method is called from ElasticsearchMappings#addDocMappingIfMissing which has 17 uses, all in ML-related code, and many of which seem to have no protection against happening on a transport thread. We work around this in that PR by adding some code that makes a decision to fork based on the thread name within the method itself. This is kind of inelegant, it would be better for the callers to choose the right thread in the first place, so I'm opening this issue to track this piece of technical debt.

Relates https://github.com/elastic/elasticsearch/issues/86765 since most of the callers end up on transport threads because they're ultimately called in a response handler for a client action.

elasticsearchmachine commented 1 year ago

Pinging @elastic/ml-core (Team:ML)

MananJethwani commented 1 year ago

@DaveCTurner I would like to contribute to this issue, is this still open?

DaveCTurner commented 1 year ago

Thanks @MananJethwani, the issue is still open indeed. It might be a little tricky for a first contribution since it needs a reasonable level of understanding of Elasticsearch's threading model, but I'll let @droberts195 offer further guidance here since he added the good first issue label.

droberts195 commented 1 year ago

Yes, with hindsight maybe this isn’t a good first issue after all. I’ve removed the label.