Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove opendistro settings and endpoints #3326

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

This file was deleted.

41 changes: 0 additions & 41 deletions docs/user/admin/settings.rst
Original file line number Diff line number Diff line change
Expand Up @@ -16,36 +16,6 @@ Introduction

When OpenSearch bootstraps, SQL plugin will register a few settings in OpenSearch cluster settings. Most of the settings are able to change dynamically so you can control the behavior of SQL plugin without need to bounce your cluster. You can update the settings by sending requests to either ``_cluster/settings`` or ``_plugins/_query/settings`` endpoint, though the examples are sending to the latter.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to list the removed settings in breaking change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to list the removed settings in breaking change?

As long as we include this change in the release notes, I think it is less confusing to remove them from the documents.

Breaking Change
===============
opendistro.sql.engine.new.enabled
---------------------------------
The opendistro.sql.engine.new.enabled setting is deprecated and will be removed then. From OpenSearch 1.0, the new engine is always enabled.

opendistro.sql.query.analysis.enabled
-------------------------------------
The opendistro.sql.query.analysis.enabled setting is deprecated and will be removed then. From OpenSearch 1.0, the query analysis in legacy engine is disabled.

opendistro.sql.query.analysis.semantic.suggestion
-------------------------------------------------
The opendistro.sql.query.analysis.semantic.suggestion setting is deprecated and will be removed then. From OpenSearch 1.0, the query analysis suggestion in legacy engine is disabled.

opendistro.sql.query.analysis.semantic.threshold
------------------------------------------------
The opendistro.sql.query.analysis.semantic.threshold setting is deprecated and will be removed then. From OpenSearch 1.0, the query analysis threshold in legacy engine is disabled.

opendistro.sql.query.response.format
------------------------------------
The opendistro.sql.query.response.format setting is deprecated and will be removed then. From OpenSearch 1.0, the query response format is default to JDBC format. `You can change the format by using query parameters<../interfaces/protocol.rst>`_.

opendistro.sql.cursor.enabled
-----------------------------
The opendistro.sql.cursor.enabled setting is deprecated and will be removed then. From OpenSearch 1.0, the cursor feature is enabled by default.

opendistro.sql.cursor.fetch_size
--------------------------------
The opendistro.sql.cursor.fetch_size setting is deprecated and will be removed then. From OpenSearch 1.0, the fetch_size in query body will decide whether create the cursor context. No cursor will be created if the fetch_size = 0.

plugins.sql.enabled
======================

Expand Down Expand Up @@ -86,8 +56,6 @@ Result set::
}
}

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

Example 2
---------

Expand Down Expand Up @@ -150,8 +118,6 @@ Result set::
}
}

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

plugins.sql.cursor.keep_alive
================================

Expand Down Expand Up @@ -194,8 +160,6 @@ 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
================================

Expand Down Expand Up @@ -268,8 +232,6 @@ Result set::
}
}

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

plugins.query.memory_limit
==========================

Expand Down Expand Up @@ -298,9 +260,6 @@ Result set::
"transient": {}
}

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


plugins.sql.delete.enabled
======================

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
import static org.opensearch.sql.legacy.TestUtils.createIndexByRestClient;
import static org.opensearch.sql.legacy.TestUtils.isIndexExist;
import static org.opensearch.sql.legacy.TestUtils.loadDataByRestClient;
import static org.opensearch.sql.legacy.plugin.RestSqlAction.LEGACY_QUERY_API_ENDPOINT;
import static org.opensearch.sql.legacy.plugin.RestSqlAction.QUERY_API_ENDPOINT;
import static org.opensearch.sql.plugin.rest.RestQuerySettingsAction.LEGACY_SQL_SETTINGS_API_ENDPOINT;
import static org.opensearch.sql.sql.LegacyAPICompatibilityIT.LEGACY_QUERY_API_ENDPOINT;
import static org.opensearch.sql.sql.LegacyAPICompatibilityIT.LEGACY_SQL_SETTINGS_API_ENDPOINT;
import static org.opensearch.sql.util.MatcherUtils.rows;
import static org.opensearch.sql.util.MatcherUtils.schema;
import static org.opensearch.sql.util.MatcherUtils.verifyDataRows;
Expand Down Expand Up @@ -93,21 +93,33 @@ public void testBackwardsCompatibility() throws Exception {
List<Map<String, Object>> plugins = (List<Map<String, Object>>) response.get("plugins");
Set<Object> pluginNames =
plugins.stream().map(map -> map.get("name")).collect(Collectors.toSet());
String version = (String) response.get("version");

boolean isBackwardsIncompatibleVersion = version.startsWith("2.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would we test 2.x using main code? I thought we would remove 2.x from build.gradle so this won't happen, since they are not backward compatible

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would we test 2.x using main code? I thought we would remove 2.x from build.gradle so this won't happen, since they are not backward compatible

I think the majority of the features are still backward compatible, besides those planned to deparcate in 3.0. Maybe discuss the bwc version of main branch on thread #1880.


switch (CLUSTER_TYPE) {
case OLD:
Assert.assertTrue(pluginNames.contains("opensearch-sql"));
updateLegacySQLSettings();
if (isBackwardsIncompatibleVersion) {
updateLegacySQLSettings();
}
loadIndex(Index.ACCOUNT);
verifySQLQueries(LEGACY_QUERY_API_ENDPOINT);
verifySQLQueries(
isBackwardsIncompatibleVersion ? LEGACY_QUERY_API_ENDPOINT : QUERY_API_ENDPOINT);
break;
case MIXED:
Assert.assertTrue(pluginNames.contains("opensearch-sql"));
verifySQLSettings();
verifySQLQueries(LEGACY_QUERY_API_ENDPOINT);
if (isBackwardsIncompatibleVersion) {
verifySQLSettings();
} else {
// For upgraded nodes, we don't need to verify legacy settings
}
verifySQLQueries(
isBackwardsIncompatibleVersion ? LEGACY_QUERY_API_ENDPOINT : QUERY_API_ENDPOINT);
break;
case UPGRADED:
Assert.assertTrue(pluginNames.contains("opensearch-sql"));
verifySQLSettings();
// For fully upgraded clusters, we don't need to verify legacy settings
verifySQLQueries(QUERY_API_ENDPOINT);
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import java.nio.file.Paths;
import java.util.Locale;
import java.util.Map;
import java.util.concurrent.Callable;
import javax.management.MBeanServerInvocationHandler;
import javax.management.ObjectName;
import javax.management.remote.JMXConnector;
Expand All @@ -68,6 +69,7 @@
import org.opensearch.client.Request;
import org.opensearch.client.RequestOptions;
import org.opensearch.client.Response;
import org.opensearch.client.ResponseException;
import org.opensearch.client.RestClient;
import org.opensearch.sql.common.setting.Settings;
import org.opensearch.sql.datasource.model.DataSourceMetadata;
Expand Down Expand Up @@ -252,6 +254,18 @@ protected Request getSqlCursorCloseRequest(String cursorRequest) {
return sqlRequest;
}

protected void assertBadRequest(Callable<Response> operation) {
try {
operation.call();
Assert.fail("Expected ResponseException was not thrown");
} catch (ResponseException e) {
Assert.assertEquals(400, e.getResponse().getStatusLine().getStatusCode());
Assert.assertEquals("Bad Request", e.getResponse().getStatusLine().getReasonPhrase());
} catch (Exception e) {
Assert.fail("Unexpected exception: " + e.getMessage());
}
}

protected String executeQuery(String query, String requestType) {
return executeQuery(query, requestType, Map.of());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@
*/
package org.opensearch.sql.ppl;

import static org.opensearch.sql.plugin.rest.RestPPLQueryAction.LEGACY_EXPLAIN_API_ENDPOINT;
import static org.opensearch.sql.plugin.rest.RestPPLQueryAction.LEGACY_QUERY_API_ENDPOINT;
import static org.opensearch.sql.plugin.rest.RestPPLStatsAction.PPL_LEGACY_STATS_API_ENDPOINT;
import static org.opensearch.sql.plugin.rest.RestQuerySettingsAction.SETTINGS_API_ENDPOINT;

import java.io.IOException;
Expand All @@ -18,49 +15,60 @@

/** For backward compatibility, check if legacy API endpoints are accessible. */
public class LegacyAPICompatibilityIT extends PPLIntegTestCase {
public static final String LEGACY_PPL_API_ENDPOINT = "/_opendistro/_ppl";
public static final String LEGACY_PPL_EXPLAIN_API_ENDPOINT = "/_opendistro/_ppl/_explain";
public static final String LEGACY_PPL_SETTINGS_API_ENDPOINT = "/_opendistro/_ppl/settings";
public static final String LEGACY_PPL_STATS_API_ENDPOINT = "/_opendistro/_ppl/stats";

@Override
public void init() throws IOException {
loadIndex(Index.ACCOUNT);
}

@Test
public void query() throws IOException {
public void query() {
String query = "source=opensearch-sql_test_index_account | where age > 30";
Request request = buildRequest(query, LEGACY_QUERY_API_ENDPOINT);
Response response = client().performRequest(request);
Assert.assertEquals(200, response.getStatusLine().getStatusCode());
Request request = buildRequest(query, LEGACY_PPL_API_ENDPOINT);
assertBadRequest(() -> client().performRequest(request));
}

@Test
public void explain() throws IOException {
public void explain() {
String query = "source=opensearch-sql_test_index_account | where age > 30";
Request request = buildRequest(query, LEGACY_EXPLAIN_API_ENDPOINT);
Response response = client().performRequest(request);
Assert.assertEquals(200, response.getStatusLine().getStatusCode());
Request request = buildRequest(query, LEGACY_PPL_EXPLAIN_API_ENDPOINT);
assertBadRequest(() -> client().performRequest(request));
}

@Test
public void stats() throws IOException {
Request request = new Request("GET", PPL_LEGACY_STATS_API_ENDPOINT);
Response response = client().performRequest(request);
Assert.assertEquals(200, response.getStatusLine().getStatusCode());
public void stats() {
Request request = new Request("GET", LEGACY_PPL_STATS_API_ENDPOINT);
assertBadRequest(() -> client().performRequest(request));
}

@Test
public void legacySettingNewEndpoint() throws IOException {
public void legacyPPLSettingNewEndpoint() {
String requestBody =
"{"
+ " \"persistent\": {"
+ " \"opendistro.ppl.query.memory_limit\": \"80%\""
+ " }"
+ "}";
Response response = updateSetting(SETTINGS_API_ENDPOINT, requestBody);
Assert.assertEquals(200, response.getStatusLine().getStatusCode());
assertBadRequest(() -> updateSetting(SETTINGS_API_ENDPOINT, requestBody));
}

@Test
public void newPPLSettingsLegacyEndpoint() {
String requestBody =
"{"
+ " \"persistent\": {"
+ " \"plugins.ppl.query.memory_limit\": \"90%\""
+ " }"
+ "}";
assertBadRequest(() -> updateSetting(LEGACY_PPL_SETTINGS_API_ENDPOINT, requestBody));
}

@Test
public void newSettingNewEndpoint() throws IOException {
public void newPPLSettingNewEndpoint() throws IOException {
String requestBody =
"{" + " \"persistent\": {" + " \"plugins.query.size_limit\": \"100\"" + " }" + "}";
Response response = updateSetting(SETTINGS_API_ENDPOINT, requestBody);
Expand Down
Loading
Loading