Skip to content

Commit

Permalink
Merge feature/pit branch to main (#2936)
Browse files Browse the repository at this point in the history
* Add pit for join queries (#2703)

* Add search after for join

Signed-off-by: Rupal Mahajan <[email protected]>

* Enable search after by default

Signed-off-by: Rupal Mahajan <[email protected]>

* Add pit

Signed-off-by: Rupal Mahajan <[email protected]>

* nit

Signed-off-by: Rupal Mahajan <[email protected]>

* Fix tests

Signed-off-by: Rupal Mahajan <[email protected]>

* ignore joinWithGeoIntersectNL

Signed-off-by: Rupal Mahajan <[email protected]>

* Rerun CI with scroll

Signed-off-by: Rupal Mahajan <[email protected]>

* Remove unused code and retrigger CI with search_after true

Signed-off-by: Rupal Mahajan <[email protected]>

* Address comments

Signed-off-by: Rupal Mahajan <[email protected]>

* Remove unused code change

Signed-off-by: Rupal Mahajan <[email protected]>

* Update pit keep alive time with SQL_CURSOR_KEEP_ALIVE

Signed-off-by: Rupal Mahajan <[email protected]>

* Fix scroll condition

Signed-off-by: Rupal Mahajan <[email protected]>

* nit

Signed-off-by: Rupal Mahajan <[email protected]>

* Add pit before query execution

Signed-off-by: Rupal Mahajan <[email protected]>

* nit

Signed-off-by: Rupal Mahajan <[email protected]>

* Move pit from join request builder to executor

Signed-off-by: Rupal Mahajan <[email protected]>

* Remove unused methods

Signed-off-by: Rupal Mahajan <[email protected]>

* Add pit in parent class's run()

Signed-off-by: Rupal Mahajan <[email protected]>

* Add comment for fetching subsequent result in NestedLoopsElasticExecutor

Signed-off-by: Rupal Mahajan <[email protected]>

* Update comment

Signed-off-by: Rupal Mahajan <[email protected]>

* Add javadoc for pit handler

Signed-off-by: Rupal Mahajan <[email protected]>

* Add pit interface

Signed-off-by: Rupal Mahajan <[email protected]>

* Add pit handler unit test

Signed-off-by: Rupal Mahajan <[email protected]>

* Fix failed unit test CI

Signed-off-by: Rupal Mahajan <[email protected]>

* Fix spotless error

Signed-off-by: Rupal Mahajan <[email protected]>

* Rename pit class and add logs

Signed-off-by: Rupal Mahajan <[email protected]>

* Fix pit delete unit test

Signed-off-by: Rupal Mahajan <[email protected]>

---------

Signed-off-by: Rupal Mahajan <[email protected]>

* Add pit for multi query (#2753)

* Add search after for join

Signed-off-by: Rupal Mahajan <[email protected]>

* Enable search after by default

Signed-off-by: Rupal Mahajan <[email protected]>

* Add pit

Signed-off-by: Rupal Mahajan <[email protected]>

* nit

Signed-off-by: Rupal Mahajan <[email protected]>

* Fix tests

Signed-off-by: Rupal Mahajan <[email protected]>

* ignore joinWithGeoIntersectNL

Signed-off-by: Rupal Mahajan <[email protected]>

* Rerun CI with scroll

Signed-off-by: Rupal Mahajan <[email protected]>

* draft

Signed-off-by: Rupal Mahajan <[email protected]>

* Remove unused code and retrigger CI with search_after true

Signed-off-by: Rupal Mahajan <[email protected]>

* Address comments

Signed-off-by: Rupal Mahajan <[email protected]>

* Remove unused code change

Signed-off-by: Rupal Mahajan <[email protected]>

* Update pit keep alive time with SQL_CURSOR_KEEP_ALIVE

Signed-off-by: Rupal Mahajan <[email protected]>

* Fix scroll condition

Signed-off-by: Rupal Mahajan <[email protected]>

* nit

Signed-off-by: Rupal Mahajan <[email protected]>

* Add pit before query execution

Signed-off-by: Rupal Mahajan <[email protected]>

* Refactor get response with pit method

Signed-off-by: Rupal Mahajan <[email protected]>

* Update remaining scroll search calls

Signed-off-by: Rupal Mahajan <[email protected]>

* Fix integ test failures

Signed-off-by: Rupal Mahajan <[email protected]>

* nit

Signed-off-by: Rupal Mahajan <[email protected]>

* Move pit from join request builder to executor

Signed-off-by: Rupal Mahajan <[email protected]>

* Remove unused methods

Signed-off-by: Rupal Mahajan <[email protected]>

* Move pit from request to executor

Signed-off-by: Rupal Mahajan <[email protected]>

* Fix pit.delete call missed while merge

Signed-off-by: Rupal Mahajan <[email protected]>

* Move getResponseWithHits method to util class

Signed-off-by: Rupal Mahajan <[email protected]>

* add try catch for create delete pit in minus executor

Signed-off-by: Rupal Mahajan <[email protected]>

* move all common fields to ElasticHitsExecutor

Signed-off-by: Rupal Mahajan <[email protected]>

* add javadoc for ElasticHitsExecutor

Signed-off-by: Rupal Mahajan <[email protected]>

* Add missing javadoc

Signed-off-by: Rupal Mahajan <[email protected]>

* Forcing an empty commit as last commit is stuck processing updates

Signed-off-by: Rupal Mahajan <[email protected]>

---------

Signed-off-by: Rupal Mahajan <[email protected]>

---------

Signed-off-by: Rupal Mahajan <[email protected]>
Signed-off-by: Manasvini B S <[email protected]>
Co-authored-by: Rupal Mahajan <[email protected]>
(cherry picked from commit 7815c96)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
github-actions[bot] and rupal-bq committed Aug 15, 2024
1 parent 3a2de37 commit 4ec6d8a
Show file tree
Hide file tree
Showing 15 changed files with 557 additions and 158 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public enum Key {
SQL_SLOWLOG("plugins.sql.slowlog"),
SQL_CURSOR_KEEP_ALIVE("plugins.sql.cursor.keep_alive"),
SQL_DELETE_ENABLED("plugins.sql.delete.enabled"),
SQL_PAGINATION_API_SEARCH_AFTER("plugins.sql.pagination.api"),

/** PPL Settings. */
PPL_ENABLED("plugins.ppl.enabled"),
Expand Down
40 changes: 40 additions & 0 deletions docs/dev/Pagination.md
Original file line number Diff line number Diff line change
Expand Up @@ -477,4 +477,44 @@ Response:
}
```

#### plugins.sql.pagination.api

This setting controls whether the SQL search queries in OpenSearch use Point-In-Time (PIT) with search_after or the traditional scroll mechanism for fetching paginated results.

- Default Value: true
- Possible Values: true or false
- When set to true, the search query in the background uses PIT with search_after instead of scroll to retrieve paginated results. The Cursor Id returned to the user will encode relevant pagination query-related information, which will be used to fetch the subsequent pages of results.
- This setting is node-level.
- This setting can be updated dynamically.

Example:

```
>> curl -H 'Content-Type: application/json' -X PUT localhost:9200/_cluster/settings -d '{
"transient" : {
"plugins.sql.pagination.api" : "true"
}
}'
```

Response:

```
{
"acknowledged" : true,
"persistent" : { },
"transient" : {
"plugins" : {
"sql" : {
"pagination" : {
"api" : "true"
}
}
}
}
}
```
44 changes: 44 additions & 0 deletions docs/user/admin/settings.rst
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,50 @@ Result set::

Note: the legacy settings of ``opendistro.sql.cursor.keep_alive`` is deprecated, it will fallback to the new settings if you request an update with the legacy name.

plugins.sql.pagination.api
================================

Description
-----------

This setting controls whether the SQL search queries in OpenSearch use Point-In-Time (PIT) with search_after or the traditional scroll mechanism for fetching paginated results.

1. Default Value: true
2. Possible Values: true or false
3. When set to true, the search query in the background uses PIT with search_after instead of scroll to retrieve paginated results. The Cursor Id returned to the user will encode relevant pagination query-related information, which will be used to fetch the subsequent pages of results.
4. This setting is node-level.
5. This setting can be updated dynamically.


Example
-------

You can update the setting with a new value like this.

SQL query::

>> curl -H 'Content-Type: application/json' -X PUT localhost:9200/_plugins/_query/settings -d '{
"transient" : {
"plugins.sql.pagination.api" : "true"
}
}'

Result set::

{
"acknowledged" : true,
"persistent" : { },
"transient" : {
"plugins" : {
"sql" : {
"pagination" : {
"api" : "true"
}
}
}
}
}

plugins.query.size_limit
===========================

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,8 @@ public void hintMultiSearchCanRunFewTimesNL() throws IOException {
Assert.assertThat(hits.length(), equalTo(42));
}

// TODO: Fix joinWithGeoIntersectNL test when SQL_PAGINATION_API_SEARCH_AFTER is true
@Ignore
@Test
public void joinWithGeoIntersectNL() throws IOException {

Expand Down Expand Up @@ -455,7 +457,7 @@ public void joinParseCheckSelectedFieldsSplitNLConditionOrderGT() throws IOExcep
"SELECT /*! USE_NL*/ a.firstname, a.lastname, a.gender, d.firstname, d.age FROM %s a"
+ " JOIN %s d on a.age < d.age WHERE (d.firstname = 'Lynn' OR d.firstname ="
+ " 'Obrien') AND a.firstname = 'Mcgee'",
TEST_INDEX_PEOPLE,
TEST_INDEX_PEOPLE2,
TEST_INDEX_ACCOUNT);

JSONObject result = executeQuery(query);
Expand Down Expand Up @@ -501,7 +503,7 @@ public void joinParseCheckSelectedFieldsSplitNLConditionOrderLT() throws IOExcep
"SELECT /*! USE_NL*/ a.firstname, a.lastname, a.gender, d.firstname, d.age FROM %s a"
+ " JOIN %s d on a.age > d.age WHERE (d.firstname = 'Sandoval' OR d.firstname ="
+ " 'Hewitt') AND a.firstname = 'Fulton'",
TEST_INDEX_PEOPLE,
TEST_INDEX_PEOPLE2,
TEST_INDEX_ACCOUNT);

JSONObject result = executeQuery(query);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,96 @@

package org.opensearch.sql.legacy.executor;

import static org.opensearch.search.sort.FieldSortBuilder.DOC_FIELD_NAME;
import static org.opensearch.search.sort.SortOrder.ASC;
import static org.opensearch.sql.common.setting.Settings.Key.SQL_CURSOR_KEEP_ALIVE;
import static org.opensearch.sql.common.setting.Settings.Key.SQL_PAGINATION_API_SEARCH_AFTER;

import java.io.IOException;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.opensearch.action.search.SearchRequestBuilder;
import org.opensearch.action.search.SearchResponse;
import org.opensearch.client.Client;
import org.opensearch.common.unit.TimeValue;
import org.opensearch.search.SearchHits;
import org.opensearch.search.builder.PointInTimeBuilder;
import org.opensearch.sql.legacy.domain.Select;
import org.opensearch.sql.legacy.esdomain.LocalClusterState;
import org.opensearch.sql.legacy.exception.SqlParseException;
import org.opensearch.sql.legacy.pit.PointInTimeHandler;

/** Executor for search requests with pagination. */
public abstract class ElasticHitsExecutor {
protected static final Logger LOG = LogManager.getLogger();
protected PointInTimeHandler pit;
protected Client client;

/**
* Executes search request
*
* @throws IOException If an input or output exception occurred
* @throws SqlParseException If parsing exception occurred
*/
protected abstract void run() throws IOException, SqlParseException;

/**
* Get search hits after execution
*
* @return Search hits
*/
protected abstract SearchHits getHits();

/**
* Get response for search request with pit/scroll
*
* @param request search request
* @param select sql select
* @param size fetch size
* @param previousResponse response for previous request
* @param pit point in time
* @return search response for subsequent request
*/
public SearchResponse getResponseWithHits(
SearchRequestBuilder request,
Select select,
int size,
SearchResponse previousResponse,
PointInTimeHandler pit) {
// Set Size
request.setSize(size);
SearchResponse responseWithHits;

/** Created by Eliran on 21/8/2016. */
public interface ElasticHitsExecutor {
void run() throws IOException, SqlParseException;
if (LocalClusterState.state().getSettingValue(SQL_PAGINATION_API_SEARCH_AFTER)) {
// Set sort field for search_after
boolean ordered = select.isOrderdSelect();
if (!ordered) {
request.addSort(DOC_FIELD_NAME, ASC);
}
// Set PIT
request.setPointInTime(new PointInTimeBuilder(pit.getPitId()));
// from and size is alternate method to paginate result.
// If select has from clause, search after is not required.
if (previousResponse != null && select.getFrom().isEmpty()) {
request.searchAfter(previousResponse.getHits().getSortFields());
}
responseWithHits = request.get();
} else {
// Set scroll
TimeValue keepAlive = LocalClusterState.state().getSettingValue(SQL_CURSOR_KEEP_ALIVE);
if (previousResponse != null) {
responseWithHits =
client
.prepareSearchScroll(previousResponse.getScrollId())
.setScroll(keepAlive)
.execute()
.actionGet();
} else {
request.setScroll(keepAlive);
responseWithHits = request.get();
}
}

SearchHits getHits();
return responseWithHits;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,34 +5,33 @@

package org.opensearch.sql.legacy.executor.join;

import static org.opensearch.sql.common.setting.Settings.Key.SQL_PAGINATION_API_SEARCH_AFTER;

import java.io.IOException;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import java.util.stream.Stream;
import org.apache.lucene.search.TotalHits;
import org.apache.lucene.search.TotalHits.Relation;
import org.opensearch.action.search.SearchRequestBuilder;
import org.opensearch.action.search.SearchResponse;
import org.opensearch.client.Client;
import org.opensearch.common.document.DocumentField;
import org.opensearch.common.unit.TimeValue;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.core.xcontent.XContentBuilder;
import org.opensearch.index.mapper.MapperService;
import org.opensearch.rest.BytesRestResponse;
import org.opensearch.rest.RestChannel;
import org.opensearch.search.SearchHit;
import org.opensearch.search.SearchHits;
import org.opensearch.search.sort.FieldSortBuilder;
import org.opensearch.search.sort.SortOrder;
import org.opensearch.sql.legacy.domain.Field;
import org.opensearch.sql.legacy.esdomain.LocalClusterState;
import org.opensearch.sql.legacy.exception.SqlParseException;
import org.opensearch.sql.legacy.executor.ElasticHitsExecutor;
import org.opensearch.sql.legacy.pit.PointInTimeHandlerImpl;
import org.opensearch.sql.legacy.query.SqlElasticRequestBuilder;
import org.opensearch.sql.legacy.query.join.HashJoinElasticRequestBuilder;
import org.opensearch.sql.legacy.query.join.JoinRequestBuilder;
Expand All @@ -41,23 +40,25 @@
import org.opensearch.sql.legacy.query.planner.HashJoinQueryPlanRequestBuilder;

/** Created by Eliran on 15/9/2015. */
public abstract class ElasticJoinExecutor implements ElasticHitsExecutor {
private static final Logger LOG = LogManager.getLogger();
public abstract class ElasticJoinExecutor extends ElasticHitsExecutor {

protected List<SearchHit> results; // Keep list to avoid copy to new array in SearchHits
protected MetaSearchResult metaResults;
protected final int MAX_RESULTS_ON_ONE_FETCH = 10000;
private Set<String> aliasesOnReturn;
private boolean allFieldsReturn;
protected String[] indices;

protected ElasticJoinExecutor(JoinRequestBuilder requestBuilder) {
protected ElasticJoinExecutor(Client client, JoinRequestBuilder requestBuilder) {
metaResults = new MetaSearchResult();
aliasesOnReturn = new HashSet<>();
List<Field> firstTableReturnedField = requestBuilder.getFirstTable().getReturnedFields();
List<Field> secondTableReturnedField = requestBuilder.getSecondTable().getReturnedFields();
allFieldsReturn =
(firstTableReturnedField == null || firstTableReturnedField.size() == 0)
&& (secondTableReturnedField == null || secondTableReturnedField.size() == 0);
indices = getIndices(requestBuilder);
this.client = client;
}

public void sendResponse(RestChannel channel) throws IOException {
Expand Down Expand Up @@ -85,10 +86,22 @@ public void sendResponse(RestChannel channel) throws IOException {
}

public void run() throws IOException, SqlParseException {
long timeBefore = System.currentTimeMillis();
results = innerRun();
long joinTimeInMilli = System.currentTimeMillis() - timeBefore;
this.metaResults.setTookImMilli(joinTimeInMilli);
try {
long timeBefore = System.currentTimeMillis();
if (LocalClusterState.state().getSettingValue(SQL_PAGINATION_API_SEARCH_AFTER)) {
pit = new PointInTimeHandlerImpl(client, indices);
pit.create();
}
results = innerRun();
long joinTimeInMilli = System.currentTimeMillis() - timeBefore;
this.metaResults.setTookImMilli(joinTimeInMilli);
} catch (Exception e) {
LOG.error("Failed during join query run.", e);
} finally {
if (LocalClusterState.state().getSettingValue(SQL_PAGINATION_API_SEARCH_AFTER)) {
pit.delete();
}
}
}

protected abstract List<SearchHit> innerRun() throws IOException, SqlParseException;
Expand All @@ -103,7 +116,7 @@ public SearchHits getHits() {
public static ElasticJoinExecutor createJoinExecutor(
Client client, SqlElasticRequestBuilder requestBuilder) {
if (requestBuilder instanceof HashJoinQueryPlanRequestBuilder) {
return new QueryPlanElasticExecutor((HashJoinQueryPlanRequestBuilder) requestBuilder);
return new QueryPlanElasticExecutor(client, (HashJoinQueryPlanRequestBuilder) requestBuilder);
} else if (requestBuilder instanceof HashJoinElasticRequestBuilder) {
HashJoinElasticRequestBuilder hashJoin = (HashJoinElasticRequestBuilder) requestBuilder;
return new HashJoinElasticExecutor(client, hashJoin);
Expand Down Expand Up @@ -256,23 +269,22 @@ protected void updateMetaSearchResults(SearchResponse searchResponse) {
this.metaResults.updateTimeOut(searchResponse.isTimedOut());
}

protected SearchResponse scrollOneTimeWithMax(
Client client, TableInJoinRequestBuilder tableRequest) {
SearchRequestBuilder scrollRequest =
tableRequest
.getRequestBuilder()
.setScroll(new TimeValue(60000))
.setSize(MAX_RESULTS_ON_ONE_FETCH);
boolean ordered = tableRequest.getOriginalSelect().isOrderdSelect();
if (!ordered) {
scrollRequest.addSort(FieldSortBuilder.DOC_FIELD_NAME, SortOrder.ASC);
}
SearchResponse responseWithHits = scrollRequest.get();
// on ordered select - not using SCAN , elastic returns hits on first scroll
// es5.0 elastic always return docs on scan
// if(!ordered)
// responseWithHits = client.prepareSearchScroll(responseWithHits.getScrollId())
// .setScroll(new TimeValue(600000)).get();
return responseWithHits;
public SearchResponse getResponseWithHits(
TableInJoinRequestBuilder tableRequest, int size, SearchResponse previousResponse) {

return getResponseWithHits(
tableRequest.getRequestBuilder(),
tableRequest.getOriginalSelect(),
size,
previousResponse,
pit);
}

public String[] getIndices(JoinRequestBuilder joinRequestBuilder) {
return Stream.concat(
Stream.of(joinRequestBuilder.getFirstTable().getOriginalSelect().getIndexArr()),
Stream.of(joinRequestBuilder.getSecondTable().getOriginalSelect().getIndexArr()))
.distinct()
.toArray(String[]::new);
}
}
Loading

0 comments on commit 4ec6d8a

Please sign in to comment.