Presto: Prepared statements and session properties run into HTTP request buffer size limits

[cawallin]

Prepared statement variables/replacements and session properties, among other things, are sent in the HTTP request headers. However, the default Jetty HTTP request buffer size is 4096 bytes, so you can easily run into issues if you are substituting long varchars into prepared statements, or if you set a particularly long session property.

I can see a couple of options:

1. Configure the request buffer size on the client side (in JettyHttpClient for the CLI, etc). That way, you could send up to 64K of data in the headers. This could be used as a stop-gap, though it's a bit awkward to add configuration properties like this on the client side.
2. Change the protocol such that prepared statement variables/replacements and session properties are sent in the body of the HTTP request. If we don't send session properties in the body of the HTTP request, there should be an explicit limit to their size, because the server accepts whatever properties the client sends, and then subsequent requests fail.
3. Change the protocol such that the Presto server stores state so that none of these things need to be passed in the HTTP headers. Admittedly, this is the most involved solution but possibly the cleanest.

@martint @dain thoughts?

[dain]

For session properties, I think 4kB is a reasonable limit, but for prepared statements, I would expect that even 64kB is way to small. As for the specific changes:

1) I'm not sure what the impact would be on Jetty.
2) Most likely the right approach for prepared statements.
3) I don't think that making the Presto server stateful is a good idea.

[cawallin]

From #5868, @electrum and @dain came up with a strawman proposal for the protocol changes that @martint will review: https://gist.github.com/electrum/a382d3f8e10fde5e2e0c1bfc1ea3182f.

[Downchuck]

Where did this v2 proposal land? The Teradata Presto connectors appear to rely on patches such as this.

[electrum]

We need to review this with @martint and finalize the protocol specification.

[xerial]

We also have encountered this issue.

[ilfrin]

@martint could you please proceed with the review of the protocol?
Can we help with that in any way?
We're happy to do the implementation, but @electrum indicated you wanted to review this together prior to that.

[OleksiiDuzhyi]

Encountered the same issue:

SET SESSION catalog.session_property='11 KiB of Data';

Getting:

java.lang.RuntimeException: Error fetching next at http://127.0.0.1:54063/v1/statement/20190228_205100_00000_u69mz/2 returned an invalid response: JsonResponse{statusCode=500, statusMessage=Server Error, headers={connection=[close]}, hasValue=false} [Error: ]
    at com.facebook.presto.client.StatementClientV1.requestFailedException(StatementClientV1.java:436)
    at com.facebook.presto.client.StatementClientV1.advance(StatementClientV1.java:383)
    at com.facebook.presto.tests.AbstractTestingPrestoClient.execute(AbstractTestingPrestoClient.java:92)
    at com.facebook.presto.tests.DistributedQueryRunner.execute(DistributedQueryRunner.java:380)
    at com.wixpress.bi.presto.devlogs.TestQueryRunner.testQueryRunner(TestQueryRunner.java:131)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:104)
    at org.testng.internal.Invoker.invokeMethod(Invoker.java:645)
    at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:851)
    at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1177)
    at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:129)
    at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:112)
    at org.testng.TestRunner.privateRun(TestRunner.java:756)
    at org.testng.TestRunner.run(TestRunner.java:610)
    at org.testng.SuiteRunner.runTest(SuiteRunner.java:387)
    at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:382)
    at org.testng.SuiteRunner.privateRun(SuiteRunner.java:340)
    at org.testng.SuiteRunner.run(SuiteRunner.java:289)
    at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
    at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86)
    at org.testng.TestNG.runSuitesSequentially(TestNG.java:1293)
    at org.testng.TestNG.runSuitesLocally(TestNG.java:1218)
    at org.testng.TestNG.runSuites(TestNG.java:1133)
    at org.testng.TestNG.run(TestNG.java:1104)
    at org.testng.IDEARemoteTestNG.run(IDEARemoteTestNG.java:73)
    at org.testng.RemoteTestNGStarter.main(RemoteTestNGStarter.java:123)

How can I avoid such problem? Is v2 protocol still experimental - like it said in documentation? How to enable v2 in local tests - AbstractTestQueryFramework?