Deprecate http.pipelining setting (#30786)

Tim-Brooks · web-flow · commit 621a26b28152 · 2018-05-23T11:19:18.000-06:00
This is related to #29500. In 7.0 this setting will be removed. This PR marks the setting in 6.x as deprecated.
diff --git a/modules/transport-netty4/src/test/java/org/elasticsearch/http/netty4/Netty4HttpServerPipeliningTests.java b/modules/transport-netty4/src/test/java/org/elasticsearch/http/netty4/Netty4HttpServerPipeliningTests.java
@@ -32,13 +32,16 @@
 import io.netty.handler.codec.http.HttpHeaderNames;
 import io.netty.handler.codec.http.HttpResponseStatus;
 import io.netty.handler.codec.http.HttpVersion;
+import org.elasticsearch.common.network.NetworkModule;
 import org.elasticsearch.common.network.NetworkService;
+import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.transport.TransportAddress;
 import org.elasticsearch.common.util.MockBigArrays;
 import org.elasticsearch.common.util.MockPageCacheRecycler;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
 import org.elasticsearch.http.HttpServerTransport;
+import org.elasticsearch.http.HttpTransportSettings;
 import org.elasticsearch.http.NullDispatcher;
 import org.elasticsearch.http.netty4.pipelining.HttpPipelinedRequest;
 import org.elasticsearch.indices.breaker.NoneCircuitBreakerService;
@@ -110,6 +113,8 @@ public void testThatHttpPipeliningWorksWhenEnabled() throws Exception {
                 assertThat(responseBodies, contains(requests.toArray()));
             }
         }
+
+        assertSettingDeprecationsAndWarnings(new Setting<?>[] {HttpTransportSettings.SETTING_PIPELINING});
     }
 
     public void testThatHttpPipeliningCanBeDisabled() throws Exception {
@@ -152,6 +157,8 @@ public void testThatHttpPipeliningCanBeDisabled() throws Exception {
                 assertThat(slowIds, equalTo(ids));
             }
         }
+
+        assertSettingDeprecationsAndWarnings(new Setting<?>[] {HttpTransportSettings.SETTING_PIPELINING});
     }
 
     class CustomNettyHttpServerTransport extends Netty4HttpServerTransport {
diff --git a/server/src/main/java/org/elasticsearch/http/HttpTransportSettings.java b/server/src/main/java/org/elasticsearch/http/HttpTransportSettings.java
@@ -49,8 +49,9 @@ public final class HttpTransportSettings {
         new Setting<>("http.cors.allow-headers", "X-Requested-With,Content-Type,Content-Length", (value) -> value, Property.NodeScope);
     public static final Setting<Boolean> SETTING_CORS_ALLOW_CREDENTIALS =
         Setting.boolSetting("http.cors.allow-credentials", false, Property.NodeScope);
+    // In 7.0 pipelining support will always be enabled and this setting will be removed.
     public static final Setting<Boolean> SETTING_PIPELINING =
-        Setting.boolSetting("http.pipelining", true, Property.NodeScope);
+        Setting.boolSetting("http.pipelining", true, Property.NodeScope, Property.Deprecated);
     public static final Setting<Integer> SETTING_PIPELINING_MAX_EVENTS =
         Setting.intSetting("http.pipelining.max_events", 10000, Property.NodeScope);
     public static final Setting<Boolean> SETTING_HTTP_COMPRESSION =
diff --git a/test/framework/src/test/java/org/elasticsearch/node/MockNodeTests.java b/test/framework/src/test/java/org/elasticsearch/node/MockNodeTests.java
@@ -25,6 +25,7 @@
 import org.elasticsearch.common.util.BigArrays;
 import org.elasticsearch.common.util.MockBigArrays;
 import org.elasticsearch.env.Environment;
+import org.elasticsearch.http.HttpTransportSettings;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.search.MockSearchService;
 import org.elasticsearch.search.SearchService;
@@ -69,6 +70,6 @@ public void testComponentsMockedByMarkerPlugins() throws IOException {
                 assertSame(searchService.getClass(), SearchService.class);
             }
         }
-        assertSettingDeprecationsAndWarnings(new Setting<?>[] { NetworkModule.HTTP_ENABLED });
+        assertSettingDeprecationsAndWarnings(new Setting<?>[] {NetworkModule.HTTP_ENABLED});
     }
 }
diff --git a/test/framework/src/test/java/org/elasticsearch/test/test/InternalTestClusterTests.java b/test/framework/src/test/java/org/elasticsearch/test/test/InternalTestClusterTests.java
@@ -31,6 +31,7 @@
 import org.elasticsearch.discovery.DiscoverySettings;
 import org.elasticsearch.discovery.zen.ZenDiscovery;
 import org.elasticsearch.env.NodeEnvironment;
+import org.elasticsearch.http.HttpTransportSettings;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.InternalTestCluster;
@@ -72,6 +73,8 @@
 @LuceneTestCase.SuppressFileSystems("ExtrasFS") // doesn't work with potential multi data path from test cluster yet
 public class InternalTestClusterTests extends ESTestCase {
 
+    private static final Setting<?>[] DEPRECATED_SETTINGS = {NetworkModule.HTTP_ENABLED, HttpTransportSettings.SETTING_PIPELINING};
+
     public void testInitializiationIsConsistent() {
         long clusterSeed = randomLong();
         boolean masterNodes = randomBoolean();
@@ -233,7 +236,7 @@ public Settings transportClientSettings() {
             assertArrayEquals(cluster0.getNodeNames(), cluster1.getNodeNames());
             if (cluster0.getNodeNames().length > 0) {
                 shouldAssertSettingsDeprecationsAndWarnings = true;
-                assertSettingDeprecationsAndWarnings(new Setting<?>[]{NetworkModule.HTTP_ENABLED});
+                assertSettingDeprecationsAndWarnings(DEPRECATED_SETTINGS);
             }
             Iterator<Client> iterator1 = cluster1.getClients().iterator();
             for (Client client : cluster0.getClients()) {
@@ -248,7 +251,7 @@ public Settings transportClientSettings() {
         } finally {
             IOUtils.close(cluster0, cluster1);
             if (shouldAssertSettingsDeprecationsAndWarnings) {
-                assertSettingDeprecationsAndWarnings(new Setting<?>[]{NetworkModule.HTTP_ENABLED});
+                assertSettingDeprecationsAndWarnings(new Setting<?>[] {NetworkModule.HTTP_ENABLED});
             }
         }
     }
@@ -355,7 +358,7 @@ public Settings transportClientSettings() {
         } finally {
             cluster.close();
         }
-        assertSettingDeprecationsAndWarnings(new Setting<?>[] { NetworkModule.HTTP_ENABLED });
+        assertSettingDeprecationsAndWarnings(DEPRECATED_SETTINGS);
     }
 
     private Path[] getNodePaths(InternalTestCluster cluster, String name) {
@@ -456,7 +459,7 @@ public Settings transportClientSettings() {
         } finally {
             cluster.close();
         }
-        assertSettingDeprecationsAndWarnings(new Setting<?>[] { NetworkModule.HTTP_ENABLED });
+        assertSettingDeprecationsAndWarnings(DEPRECATED_SETTINGS);
     }
 
     public void testTwoNodeCluster() throws Exception {
@@ -518,7 +521,7 @@ public Settings onNodeStopped(String nodeName) throws Exception {
         } finally {
             cluster.close();
         }
-        assertSettingDeprecationsAndWarnings(new Setting<?>[] { NetworkModule.HTTP_ENABLED });
+        assertSettingDeprecationsAndWarnings(DEPRECATED_SETTINGS);
     }
 
     /**

Original file line number	Diff line number	Diff line change
`@@ -32,13 +32,16 @@`
`32`	`32`	`import io.netty.handler.codec.http.HttpHeaderNames;`
`33`	`33`	`import io.netty.handler.codec.http.HttpResponseStatus;`
`34`	`34`	`import io.netty.handler.codec.http.HttpVersion;`
	`35`	`+import org.elasticsearch.common.network.NetworkModule;`
`35`	`36`	`import org.elasticsearch.common.network.NetworkService;`
	`37`	`+import org.elasticsearch.common.settings.Setting;`
`36`	`38`	`import org.elasticsearch.common.settings.Settings;`
`37`	`39`	`import org.elasticsearch.common.transport.TransportAddress;`
`38`	`40`	`import org.elasticsearch.common.util.MockBigArrays;`
`39`	`41`	`import org.elasticsearch.common.util.MockPageCacheRecycler;`
`40`	`42`	`import org.elasticsearch.common.util.concurrent.ThreadContext;`
`41`	`43`	`import org.elasticsearch.http.HttpServerTransport;`
	`44`	`+import org.elasticsearch.http.HttpTransportSettings;`
`42`	`45`	`import org.elasticsearch.http.NullDispatcher;`
`43`	`46`	`import org.elasticsearch.http.netty4.pipelining.HttpPipelinedRequest;`
`44`	`47`	`import org.elasticsearch.indices.breaker.NoneCircuitBreakerService;`
`@@ -110,6 +113,8 @@ public void testThatHttpPipeliningWorksWhenEnabled() throws Exception {`
`110`	`113`	`assertThat(responseBodies, contains(requests.toArray()));`
`111`	`114`	`}`
`112`	`115`	`}`
	`116`	`+`
	`117`	`+ assertSettingDeprecationsAndWarnings(new Setting<?>[] {HttpTransportSettings.SETTING_PIPELINING});`
`113`	`118`	`}`
`114`	`119`
`115`	`120`	`public void testThatHttpPipeliningCanBeDisabled() throws Exception {`
`@@ -152,6 +157,8 @@ public void testThatHttpPipeliningCanBeDisabled() throws Exception {`
`152`	`157`	`assertThat(slowIds, equalTo(ids));`
`153`	`158`	`}`
`154`	`159`	`}`
	`160`	`+`
	`161`	`+ assertSettingDeprecationsAndWarnings(new Setting<?>[] {HttpTransportSettings.SETTING_PIPELINING});`
`155`	`162`	`}`
`156`	`163`
`157`	`164`	`class CustomNettyHttpServerTransport extends Netty4HttpServerTransport {`
Original file line number	Diff line number	Diff line change
`@@ -25,6 +25,7 @@`
`25`	`25`	`import org.elasticsearch.common.util.BigArrays;`
`26`	`26`	`import org.elasticsearch.common.util.MockBigArrays;`
`27`	`27`	`import org.elasticsearch.env.Environment;`
	`28`	`+import org.elasticsearch.http.HttpTransportSettings;`
`28`	`29`	`import org.elasticsearch.plugins.Plugin;`
`29`	`30`	`import org.elasticsearch.search.MockSearchService;`
`30`	`31`	`import org.elasticsearch.search.SearchService;`
`@@ -69,6 +70,6 @@ public void testComponentsMockedByMarkerPlugins() throws IOException {`
`69`	`70`	`assertSame(searchService.getClass(), SearchService.class);`
`70`	`71`	`}`
`71`	`72`	`}`
`72`		`- assertSettingDeprecationsAndWarnings(new Setting<?>[] { NetworkModule.HTTP_ENABLED });`
	`73`	`+ assertSettingDeprecationsAndWarnings(new Setting<?>[] {NetworkModule.HTTP_ENABLED});`
`73`	`74`	`}`
`74`	`75`	`}`
Original file line number	Diff line number	Diff line change
`@@ -31,6 +31,7 @@`
`31`	`31`	`import org.elasticsearch.discovery.DiscoverySettings;`
`32`	`32`	`import org.elasticsearch.discovery.zen.ZenDiscovery;`
`33`	`33`	`import org.elasticsearch.env.NodeEnvironment;`
	`34`	`+import org.elasticsearch.http.HttpTransportSettings;`
`34`	`35`	`import org.elasticsearch.plugins.Plugin;`
`35`	`36`	`import org.elasticsearch.test.ESTestCase;`
`36`	`37`	`import org.elasticsearch.test.InternalTestCluster;`
`@@ -72,6 +73,8 @@`
`72`	`73`	`@LuceneTestCase.SuppressFileSystems("ExtrasFS") // doesn't work with potential multi data path from test cluster yet`
`73`	`74`	`public class InternalTestClusterTests extends ESTestCase {`
`74`	`75`
	`76`	`+ private static final Setting<?>[] DEPRECATED_SETTINGS = {NetworkModule.HTTP_ENABLED, HttpTransportSettings.SETTING_PIPELINING};`
	`77`	`+`
`75`	`78`	`public void testInitializiationIsConsistent() {`
`76`	`79`	`long clusterSeed = randomLong();`
`77`	`80`	`boolean masterNodes = randomBoolean();`
`@@ -233,7 +236,7 @@ public Settings transportClientSettings() {`
`233`	`236`	`assertArrayEquals(cluster0.getNodeNames(), cluster1.getNodeNames());`
`234`	`237`	`if (cluster0.getNodeNames().length > 0) {`
`235`	`238`	`shouldAssertSettingsDeprecationsAndWarnings = true;`
`236`		`- assertSettingDeprecationsAndWarnings(new Setting<?>[]{NetworkModule.HTTP_ENABLED});`
	`239`	`+ assertSettingDeprecationsAndWarnings(DEPRECATED_SETTINGS);`
`237`	`240`	`}`
`238`	`241`	`Iterator<Client> iterator1 = cluster1.getClients().iterator();`
`239`	`242`	`for (Client client : cluster0.getClients()) {`
`@@ -248,7 +251,7 @@ public Settings transportClientSettings() {`
`248`	`251`	`} finally {`
`249`	`252`	`IOUtils.close(cluster0, cluster1);`
`250`	`253`	`if (shouldAssertSettingsDeprecationsAndWarnings) {`
`251`		`- assertSettingDeprecationsAndWarnings(new Setting<?>[]{NetworkModule.HTTP_ENABLED});`
	`254`	`+ assertSettingDeprecationsAndWarnings(new Setting<?>[] {NetworkModule.HTTP_ENABLED});`
`252`	`255`	`}`
`253`	`256`	`}`
`254`	`257`	`}`
`@@ -355,7 +358,7 @@ public Settings transportClientSettings() {`
`355`	`358`	`} finally {`
`356`	`359`	`cluster.close();`
`357`	`360`	`}`
`358`		`- assertSettingDeprecationsAndWarnings(new Setting<?>[] { NetworkModule.HTTP_ENABLED });`
	`361`	`+ assertSettingDeprecationsAndWarnings(DEPRECATED_SETTINGS);`
`359`	`362`	`}`
`360`	`363`
`361`	`364`	`private Path[] getNodePaths(InternalTestCluster cluster, String name) {`
`@@ -456,7 +459,7 @@ public Settings transportClientSettings() {`
`456`	`459`	`} finally {`
`457`	`460`	`cluster.close();`
`458`	`461`	`}`
`459`		`- assertSettingDeprecationsAndWarnings(new Setting<?>[] { NetworkModule.HTTP_ENABLED });`
	`462`	`+ assertSettingDeprecationsAndWarnings(DEPRECATED_SETTINGS);`
`460`	`463`	`}`
`461`	`464`
`462`	`465`	`public void testTwoNodeCluster() throws Exception {`
`@@ -518,7 +521,7 @@ public Settings onNodeStopped(String nodeName) throws Exception {`
`518`	`521`	`} finally {`
`519`	`522`	`cluster.close();`
`520`	`523`	`}`
`521`		`- assertSettingDeprecationsAndWarnings(new Setting<?>[] { NetworkModule.HTTP_ENABLED });`
	`524`	`+ assertSettingDeprecationsAndWarnings(DEPRECATED_SETTINGS);`
`522`	`525`	`}`
`523`	`526`
`524`	`527`	`/**`