OpenSearch-Dashboards icon indicating copy to clipboard operation
OpenSearch-Dashboards copied to clipboard

[MD] batch concurrent search is not likely to work with multiple data source

Open zhongnansu opened this issue 3 years ago • 2 comments

image

batch search will issue _msearch requests to OpenSearch, which is not likely to work with multiple data sources for the reasons below.

https://github.com/opensearch-project/OpenSearch-Dashboards/blob/4dd7f142ad4c8845bc2dd40a3947f8eb6752e891/src/plugins/data/common/search/search_source/search_source.ts#L282-L283

~~1. Conceptually batch_search keeps search unterminated. But for multi datasource, we are maintaining an LRU cache of clients to different datasources. The cache has size limit to not to harm the performance. With existing logic, if new connection/client is not spawned or called explicitly, it has a chance to be dumped by the cache and closed. We can't guarantee any search that's unterminated by design, so it's conflict with the original batch search on single default cluster.~~

  1. Batch_search was once upon a time labeled as deprecated while we fork Kibana and create OpenSearch Dashboards. Even in the codebase, the functionality is exposed from data plugin -> search_service, alongside the search, and it's called legacy -> getMsearch(). Our datasource solution is based on the common uses case of search() and search strategy. While msearch solution doesn't rely on search strategy So I'd rather wait for the following 2 things and then make a decision to support it. Until then, data source feature won't support batch_search, and we could make that clear in doc and help text.
    1. the plan to remove legacy code in OSD.
    2. If there's user need for batch_search to be supported on data sources.

https://github.com/opensearch-project/OpenSearch-Dashboards/blob/4a06f5a6fe404a65b11775d292afaff4b8677c33/src/plugins/data/server/search/search_service.ts#L211-L213

https://github.com/opensearch-project/OpenSearch-Dashboards/blob/a5289655fdeff60feb7ac4ccd81beb841e8e2c1c/src/plugins/data/server/search/routes/call_msearch.ts#L72

Note: there are other search related advanced kibana settings, we need to consider supporting them all

Solution

Code changes to adopt needs to be made here. Pass in data source client as opensearchClient, based on the if the request body/param has dataSourceId. To get data source client, we use the standard api context.data_source.opensearch.getClient(<dataSourceId>) https://github.com/opensearch-project/OpenSearch-Dashboards/blob/a5289655fdeff60feb7ac4ccd81beb841e8e2c1c/src/plugins/data/server/search/routes/msearch.ts#L74

zhongnansu avatar Aug 19 '22 17:08 zhongnansu

I wouldn't disable batch requests as it appears to be a highly used setting.

kavilla avatar Sep 06 '22 19:09 kavilla

I wouldn't disable batch requests as it appears to be a highly used setting.

Got it. I saw the commit that removes the deprecated label of batch search setting. https://github.com/opensearch-project/OpenSearch-Dashboards/pull/735

zhongnansu avatar Sep 07 '22 18:09 zhongnansu

@zhongnansu Is this still targeting 2.5.0? I don't see a linked PR.

joshuarrrr avatar Jan 05 '23 19:01 joshuarrrr

@zhongnansu Is this still targeting 2.5.0? I don't see a linked PR.

This will be for later release. Removed 2.5 label

zhongnansu avatar Jan 05 '23 21:01 zhongnansu