From 93d25f71734aa83f7357958acda319b2a3d2fb90 Mon Sep 17 00:00:00 2001 From: penghuo Date: Wed, 23 Jun 2021 11:42:26 -0700 Subject: [PATCH 1/3] Deprecated cursor enabling and fetch size setting Signed-off-by: penghuo --- .../sql/common/setting/LegacySettings.java | 8 ++++++- .../setting/LegacyOpenDistroSettings.java | 22 ++++++++++++++++++- .../org/opensearch/sql/plugin/SQLPlugin.java | 2 ++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/common/src/main/java/org/opensearch/sql/common/setting/LegacySettings.java b/common/src/main/java/org/opensearch/sql/common/setting/LegacySettings.java index ae34475c9a..1520d89f71 100644 --- a/common/src/main/java/org/opensearch/sql/common/setting/LegacySettings.java +++ b/common/src/main/java/org/opensearch/sql/common/setting/LegacySettings.java @@ -56,7 +56,13 @@ public enum Key { /** * Legacy Common Settings. */ - QUERY_SIZE_LIMIT("opendistro.query.size_limit"); + QUERY_SIZE_LIMIT("opendistro.query.size_limit"), + + /** + * Deprecated Settings. + */ + CURSOR_ENABLED("opendistro.sql.cursor.enabled"), + CURSOR_FETCH_SIZE("opendistro.sql.cursor.fetch_size"); @Getter private final String keyValue; diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/LegacyOpenDistroSettings.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/LegacyOpenDistroSettings.java index eea8f50c99..6a8e084dfb 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/LegacyOpenDistroSettings.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/LegacyOpenDistroSettings.java @@ -80,5 +80,25 @@ public class LegacyOpenDistroSettings { Setting.Property.NodeScope, Setting.Property.Dynamic, Setting.Property.Deprecated); - + /** + * Deprecated. + * From OpenSearch 1.0, the cursor is always enabled and the setting will be removed then. + */ + public static final Setting CURSOR_ENABLED = Setting.boolSetting( + LegacySettings.Key.CURSOR_ENABLED.getKeyValue(), + true, + Setting.Property.NodeScope, + Setting.Property.Dynamic, + Setting.Property.Deprecated); + /** + * Deprecated. + * From OpenSearch 1.0, only the fetch_size in the context take effect and the setting will + * be removed then. + */ + public static final Setting CURSOR_FETCH_SIZE = Setting.intSetting( + LegacySettings.Key.CURSOR_FETCH_SIZE.getKeyValue(), + 1000, + Setting.Property.NodeScope, + Setting.Property.Dynamic, + Setting.Property.Deprecated); } diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java b/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java index 75b5184669..1a7662348a 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java @@ -159,6 +159,8 @@ public List> getSettings() { .add(LegacyOpenDistroSettings.PPL_ENABLED_SETTING) .add(LegacyOpenDistroSettings.PPL_QUERY_MEMORY_LIMIT_SETTING) .add(LegacyOpenDistroSettings.QUERY_SIZE_LIMIT_SETTING) + .add(LegacyOpenDistroSettings.CURSOR_ENABLED) + .add(LegacyOpenDistroSettings.CURSOR_FETCH_SIZE) .addAll(OpenSearchSettings.pluginSettings()) .build(); } From 8bea88b5b85e3534002f75477cd639beb9135e3e Mon Sep 17 00:00:00 2001 From: penghuo Date: Wed, 23 Jun 2021 15:21:03 -0700 Subject: [PATCH 2/3] update doc Signed-off-by: penghuo --- docs/user/admin/settings.rst | 10 ++++++++ .../setting/LegacyOpenDistroSettings.java | 24 +++++++++++++++++-- .../org/opensearch/sql/plugin/SQLPlugin.java | 10 +------- 3 files changed, 33 insertions(+), 11 deletions(-) diff --git a/docs/user/admin/settings.rst b/docs/user/admin/settings.rst index 96f93c6020..a32f0b07f2 100644 --- a/docs/user/admin/settings.rst +++ b/docs/user/admin/settings.rst @@ -16,6 +16,16 @@ 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. +Breaking Change +=============== + +opendistro.sql.cursor.enabled +----------------------------- +The opendistro.sql.cursor.enabled 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 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 ====================== diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/LegacyOpenDistroSettings.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/LegacyOpenDistroSettings.java index 6a8e084dfb..8eff73f433 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/LegacyOpenDistroSettings.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/LegacyOpenDistroSettings.java @@ -13,6 +13,8 @@ import static org.opensearch.common.unit.TimeValue.timeValueMinutes; +import com.google.common.collect.ImmutableList; +import java.util.List; import lombok.experimental.UtilityClass; import org.opensearch.common.settings.Setting; import org.opensearch.common.unit.ByteSizeValue; @@ -84,7 +86,7 @@ public class LegacyOpenDistroSettings { * Deprecated. * From OpenSearch 1.0, the cursor is always enabled and the setting will be removed then. */ - public static final Setting CURSOR_ENABLED = Setting.boolSetting( + public static final Setting CURSOR_ENABLED_SETTING = Setting.boolSetting( LegacySettings.Key.CURSOR_ENABLED.getKeyValue(), true, Setting.Property.NodeScope, @@ -95,10 +97,28 @@ public class LegacyOpenDistroSettings { * From OpenSearch 1.0, only the fetch_size in the context take effect and the setting will * be removed then. */ - public static final Setting CURSOR_FETCH_SIZE = Setting.intSetting( + public static final Setting CURSOR_FETCH_SIZE_SETTING = Setting.intSetting( LegacySettings.Key.CURSOR_FETCH_SIZE.getKeyValue(), 1000, Setting.Property.NodeScope, Setting.Property.Dynamic, Setting.Property.Deprecated); + + /** + * Used by Plugin to init Setting. + */ + public static List> legacySettings() { + return new ImmutableList.Builder>() + .add(SQL_ENABLED_SETTING) + .add(SQL_QUERY_SLOWLOG_SETTING) + .add(SQL_CURSOR_KEEPALIVE_SETTING) + .add(METRICS_ROLLING_WINDOW_SETTING) + .add(METRICS_ROLLING_INTERVAL_SETTING) + .add(PPL_ENABLED_SETTING) + .add(PPL_QUERY_MEMORY_LIMIT_SETTING) + .add(QUERY_SIZE_LIMIT_SETTING) + .add(CURSOR_ENABLED_SETTING) + .add(CURSOR_FETCH_SIZE_SETTING) + .build(); + } } diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java b/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java index 1a7662348a..ad7d3f62ac 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java @@ -152,15 +152,7 @@ public List> getExecutorBuilders(Settings settings) { @Override public List> getSettings() { return new ImmutableList.Builder>() - .add(LegacyOpenDistroSettings.SQL_ENABLED_SETTING) - .add(LegacyOpenDistroSettings.SQL_QUERY_SLOWLOG_SETTING) - .add(LegacyOpenDistroSettings.METRICS_ROLLING_WINDOW_SETTING) - .add(LegacyOpenDistroSettings.METRICS_ROLLING_INTERVAL_SETTING) - .add(LegacyOpenDistroSettings.PPL_ENABLED_SETTING) - .add(LegacyOpenDistroSettings.PPL_QUERY_MEMORY_LIMIT_SETTING) - .add(LegacyOpenDistroSettings.QUERY_SIZE_LIMIT_SETTING) - .add(LegacyOpenDistroSettings.CURSOR_ENABLED) - .add(LegacyOpenDistroSettings.CURSOR_FETCH_SIZE) + .addAll(LegacyOpenDistroSettings.legacySettings()) .addAll(OpenSearchSettings.pluginSettings()) .build(); } From c94a5e95664428d49424f4ad2843326c710e0a4e Mon Sep 17 00:00:00 2001 From: penghuo Date: Wed, 23 Jun 2021 17:54:08 -0700 Subject: [PATCH 3/3] add more deprecated settings Signed-off-by: penghuo --- .../sql/common/setting/LegacySettings.java | 9 +- docs/user/admin/settings.rst | 23 ++++- .../setting/LegacyOpenDistroSettings.java | 90 ++++++++++++++++--- .../setting/OpenSearchSettingsTest.java | 7 ++ 4 files changed, 113 insertions(+), 16 deletions(-) diff --git a/common/src/main/java/org/opensearch/sql/common/setting/LegacySettings.java b/common/src/main/java/org/opensearch/sql/common/setting/LegacySettings.java index 1520d89f71..3ff9b81ec5 100644 --- a/common/src/main/java/org/opensearch/sql/common/setting/LegacySettings.java +++ b/common/src/main/java/org/opensearch/sql/common/setting/LegacySettings.java @@ -61,8 +61,13 @@ public enum Key { /** * Deprecated Settings. */ - CURSOR_ENABLED("opendistro.sql.cursor.enabled"), - CURSOR_FETCH_SIZE("opendistro.sql.cursor.fetch_size"); + SQL_NEW_ENGINE_ENABLED("opendistro.sql.engine.new.enabled"), + QUERY_ANALYSIS_ENABLED("opendistro.sql.query.analysis.enabled"), + QUERY_ANALYSIS_SEMANTIC_SUGGESTION("opendistro.sql.query.analysis.semantic.suggestion"), + QUERY_ANALYSIS_SEMANTIC_THRESHOLD("opendistro.sql.query.analysis.semantic.threshold"), + QUERY_RESPONSE_FORMAT("opendistro.sql.query.response.format"), + SQL_CURSOR_ENABLED("opendistro.sql.cursor.enabled"), + SQL_CURSOR_FETCH_SIZE("opendistro.sql.cursor.fetch_size"); @Getter private final String keyValue; diff --git a/docs/user/admin/settings.rst b/docs/user/admin/settings.rst index a32f0b07f2..b5da4e28e2 100644 --- a/docs/user/admin/settings.rst +++ b/docs/user/admin/settings.rst @@ -18,14 +18,33 @@ When OpenSearch bootstraps, SQL plugin will register a few settings in OpenSearc 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 is deprecated and will be removed then. From OpenSearch 1.0, the cursor feature is enabled by default. +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 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. +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 ====================== diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/LegacyOpenDistroSettings.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/LegacyOpenDistroSettings.java index 8eff73f433..7c774a804d 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/LegacyOpenDistroSettings.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/LegacyOpenDistroSettings.java @@ -82,28 +82,89 @@ public class LegacyOpenDistroSettings { Setting.Property.NodeScope, Setting.Property.Dynamic, Setting.Property.Deprecated); + /** - * Deprecated. - * From OpenSearch 1.0, the cursor is always enabled and the setting will be removed then. + * Deprecated and will be removed then. + * From OpenSearch 1.0, the new engine is always enabled. */ - public static final Setting CURSOR_ENABLED_SETTING = Setting.boolSetting( - LegacySettings.Key.CURSOR_ENABLED.getKeyValue(), + public static final Setting SQL_NEW_ENGINE_ENABLED_SETTING = Setting.boolSetting( + LegacySettings.Key.SQL_NEW_ENGINE_ENABLED.getKeyValue(), true, Setting.Property.NodeScope, Setting.Property.Dynamic, Setting.Property.Deprecated); + + /** + * Deprecated and will be removed then. + * From OpenSearch 1.0, the query analysis in legacy engine is disabled. + */ + public static final Setting QUERY_ANALYSIS_ENABLED_SETTING = Setting.boolSetting( + LegacySettings.Key.QUERY_ANALYSIS_ENABLED.getKeyValue(), + false, + Setting.Property.NodeScope, + Setting.Property.Dynamic, + Setting.Property.Deprecated); + /** - * Deprecated. - * From OpenSearch 1.0, only the fetch_size in the context take effect and the setting will - * be removed then. + * Deprecated and will be removed then. + * From OpenSearch 1.0, the query analysis suggestion in legacy engine is disabled. */ - public static final Setting CURSOR_FETCH_SIZE_SETTING = Setting.intSetting( - LegacySettings.Key.CURSOR_FETCH_SIZE.getKeyValue(), - 1000, + public static final Setting QUERY_ANALYSIS_SEMANTIC_SUGGESTION_SETTING = + Setting.boolSetting( + LegacySettings.Key.QUERY_ANALYSIS_SEMANTIC_SUGGESTION.getKeyValue(), + false, Setting.Property.NodeScope, Setting.Property.Dynamic, Setting.Property.Deprecated); + /** + * Deprecated and will be removed then. + * From OpenSearch 1.0, the query analysis threshold in legacy engine is disabled. + */ + public static final Setting QUERY_ANALYSIS_SEMANTIC_THRESHOLD_SETTING = + Setting.intSetting( + LegacySettings.Key.QUERY_ANALYSIS_SEMANTIC_THRESHOLD.getKeyValue(), + 200, + Setting.Property.NodeScope, + Setting.Property.Dynamic, + Setting.Property.Deprecated); + + /** + * Deprecated and will be removed then. + * From OpenSearch 1.0, the query response format is default to JDBC format. + */ + public static final Setting QUERY_RESPONSE_FORMAT_SETTING = + Setting.simpleString( + LegacySettings.Key.QUERY_RESPONSE_FORMAT.getKeyValue(), + "jdbc", + Setting.Property.NodeScope, + Setting.Property.Dynamic, + Setting.Property.Deprecated); + + /** + * Deprecated and will be removed then. + * From OpenSearch 1.0, the cursor feature is enabled by default. + */ + public static final Setting SQL_CURSOR_ENABLED_SETTING = + Setting.boolSetting( + LegacySettings.Key.SQL_CURSOR_ENABLED.getKeyValue(), + true, + Setting.Property.NodeScope, + Setting.Property.Dynamic, + Setting.Property.Deprecated); + /** + * 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. + */ + public static final Setting SQL_CURSOR_FETCH_SIZE_SETTING = + Setting.intSetting( + LegacySettings.Key.SQL_CURSOR_FETCH_SIZE.getKeyValue(), + 1000, + Setting.Property.NodeScope, + Setting.Property.Dynamic, + Setting.Property.Deprecated); + /** * Used by Plugin to init Setting. */ @@ -117,8 +178,13 @@ public static List> legacySettings() { .add(PPL_ENABLED_SETTING) .add(PPL_QUERY_MEMORY_LIMIT_SETTING) .add(QUERY_SIZE_LIMIT_SETTING) - .add(CURSOR_ENABLED_SETTING) - .add(CURSOR_FETCH_SIZE_SETTING) + .add(SQL_NEW_ENGINE_ENABLED_SETTING) + .add(QUERY_ANALYSIS_ENABLED_SETTING) + .add(QUERY_ANALYSIS_SEMANTIC_SUGGESTION_SETTING) + .add(QUERY_ANALYSIS_SEMANTIC_THRESHOLD_SETTING) + .add(QUERY_RESPONSE_FORMAT_SETTING) + .add(SQL_CURSOR_ENABLED_SETTING) + .add(SQL_CURSOR_FETCH_SIZE_SETTING) .build(); } } diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/setting/OpenSearchSettingsTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/setting/OpenSearchSettingsTest.java index f8f45f90c8..f264e5df34 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/setting/OpenSearchSettingsTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/setting/OpenSearchSettingsTest.java @@ -33,6 +33,7 @@ import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.opensearch.common.unit.TimeValue.timeValueMinutes; +import static org.opensearch.sql.opensearch.setting.LegacyOpenDistroSettings.legacySettings; import java.util.List; import org.junit.jupiter.api.Test; @@ -148,4 +149,10 @@ public void updateLegacySettingsFallback() { assertEquals(OpenSearchSettings.METRICS_ROLLING_WINDOW_SETTING.get(settings), 2000L); assertEquals(OpenSearchSettings.METRICS_ROLLING_INTERVAL_SETTING.get(settings), 100L); } + + + @Test + void legacySettingsShouldBeDeprecatedBeforeRemove() { + assertEquals(15, legacySettings().size()); + } }