Disable socket factory for pre JDK11

[Lewuathe]

Purpose

As discussed in #1169, the socket channel is the cause of the slowdown with JDK 11. As JDK-8131133 is fixed from JDK 9, the workaround is not necessary anymore. We can safely disable the socket factory usage if the runtime is JDK 11 or later. Confirmed the communication between server and client works well even with HTTP 2.

Overview

• Add JavaVersion class from presto-server module to parse Java version string. Although we expected to use Runtime.getVersion, it does not exist in pre-JDK 9.
• Check runtime version and disable socket factory if JDK 11 or later.

[rohangarg]

should this change (disabling SocketChannelSocketFactory) also be done for PrestoDriver?

[Lewuathe]

@rohangarg Good catch. Thanks! The same problem happens for presto-jdbc too.
I'm going to update accordingly.

[Lewuathe]

Checkstyle error happening in CI seems to be unrelated to the change.

[ERROR] src/main/java/io/prestosql/tests/product/launcher/env/EnvironmentModule.java:[40] (imports) ImportOrder: Wrong order for 'io.prestosql.tests.product.launcher.env.environment.SinglenodeLdap' import.

[ebyhr]

Could you rebase and push again? It should be fixed in da29dad.

[martint]

There's a build failure:

[ERROR] src/main/java/io/prestosql/cli/QueryRunner.java:[16,8] (imports) UnusedImports: Unused import - com.google.common.base.StandardSystemProperty.

The change looks good. Can you squash the commits? I'll merge it once you do that and the build passes.

[electrum]

Let’s simplify the JDK 11 detection to be more robust.

[electrum]

I’m concerned about having this complex code (which has had bugs in the past) as part of the JDBC driver, which runs in various environments. We also plan to keep the driver on Java 8 for the foreseeable future, so this code won’t be removed anytime soon.

Some alternatives:

1. Just split on a period and take the first item, since that works for all versions.

2. Use reflection to check for and call the Runtime.Version API. We can check for and use the feature method which is in JDK 10+.

3. Use reflection to look for the existence of a “marker” class like java.net.http.HttpClient which was added in JDK 11.

[electrum]

This is the code using Runtime.version() reflectively:

@SuppressWarnings("JavaReflectionMemberAccess")
private static boolean isAtLeastJava11()
{
    try {
        Method versionMethod = Runtime.class.getMethod("version");
        Method featureMethod = versionMethod.getReturnType().getMethod("feature");
        Object version = versionMethod.invoke(null);
        int feature = (int) featureMethod.invoke(version);
        return feature >= 11;
    }
    catch (ReflectiveOperationException e) {
        return false;
    }
}

[electrum]

Code to safely parse the version:

private static boolean isAtLeastJava11()
{
    String feature = Splitter.on(".").split(StandardSystemProperty.JAVA_VERSION.value()).next();
    try {
        return Integer.parseInt(feature) >= 11;
    }
    catch (NumberFormatException e) {
        return false;
    }
}

[electrum]

@dain agrees we should go with option 1, since that's simple and reliable

[electrum]

Please squash the commits when updating

[electrum]

Thanks!