[FLINK-28150][sql-gateway][hive] Introduce the hiveserver2 endpoint and factory #20101

fsk119 · 2022-06-29T09:34:07Z

What is the purpose of the change

Introduce the HiveServer2 Endpoint and its Factory. With the HiveServer2 Endpoint, users is avaliable to connect to the SqlGateway with Hive JDBC.

Brief change log

Introduce the endpoint and its factory. With the factory, the SqlGateway is able to load the HiveServer2 Endpoint
Expose the required options to configure HiveServer2 Endpoint. The options are discussed in the FLIP-223 except the option thrift.max.message.size, thrift.login.timeout, thrift.exponential.backoff.slot.length. The newly introduced options are align with the hive, please refer to the HiveConf. The PR also introduce the option module.name that aligns with the option catalog.name as the default hive module name.
Supports OpenSession and CloseSession in HiveServer2Endpoint.

Verifying this change

This change added tests and can be verified as follows:

Added ITCase to openSession and closeSession with HiveServerEndoint2
Added test that the endpoint factory can create the expected endpoint.
Added the client can get expected error message with HiveServer2 endpoint

Does this pull request potentially affect one of the following parts:

Dependencies (does it add or upgrade a dependency): (yes / no)
The public API, i.e., is any changed class annotated with @Public(Evolving): (yes / no)
The serializers: (yes / no / don't know)
The runtime per-record code paths (performance sensitive): (yes / no / don't know)
Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (yes / no / don't know)
The S3 file system connector: (yes / no / don't know)

Documentation

Does this pull request introduce a new feature? (yes / no)
If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

flinkbot · 2022-06-29T09:40:57Z

CI report:

e866c29 UNKNOWN
d889c33 Azure: SUCCESS

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot run azure re-run the last Azure build

link3280

@fsk119 The codes generally look good to me. I left some minor comments, PTAL. Thanks!

...k-connector-hive/src/main/java/org/apache/flink/table/endpoint/hive/HiveServer2Endpoint.java

link3280 · 2022-07-19T11:41:11Z

...ive/src/main/java/org/apache/flink/table/endpoint/hive/HiveServer2EndpointConfigOptions.java

+ // Server Options
+ // --------------------------------------------------------------------------------------------
+
+ public static final ConfigOption<Integer> THRIFT_PORT =


Since these options are scoped to Hive endpoint, should we add a prefix like hive-endpoint.?

Do you mean the config option name should contain a prefix? Actually, we already finished this in the SqlGatewayEndpointFactoryUtils#createSqlGatewayEndpoint. The util will automatically search the options that begin with the sql-gateway.endpoint.<identifier>, e.g. sql-gateway.endpoint.hiveserver2 here. You can take a look at the test in the SqlGatewayEndpointFactoryUtilsTest#testCreateEndpoints.

Thanks! LGTM now.

...gateway-api/src/main/java/org/apache/flink/table/gateway/api/session/SessionEnvironment.java

link3280 · 2022-07-19T13:24:50Z

...sql-gateway/src/main/java/org/apache/flink/table/gateway/service/context/SessionContext.java

+ public void close() {
+ operationManager.close();
+ for (String name : sessionState.catalogManager.listCatalogs()) {
+ sessionState.catalogManager.getCatalog(name).ifPresent(Catalog::close);


There might be a CatalogException when closing a catalog. I think it'd be better to wrap it with a try-catch.

Yes. It might get a resource leak if one of the catalogs fails to close.

luoyuxia

Thanks for contribution. I left some comments.

luoyuxia · 2022-07-15T10:24:53Z

...k-connector-hive/src/main/java/org/apache/flink/table/endpoint/hive/HiveServer2Endpoint.java

+ ? Collections.emptyMap()
+ : tOpenSessionReq.getConfiguration();
+ Map<String, String> sessionConfig = new HashMap<>();
+ sessionConfig.put(TABLE_SQL_DIALECT.key(), SqlDialect.HIVE.name());


Hive dialect is the default dialect?

Yes. We want the HiveSever2 endpoint is compatible with HiveServer2. With the HiveServer2 endpoint, users can use hive dialect directly.

...or-hive/src/main/java/org/apache/flink/table/endpoint/hive/util/ThriftObjectConversions.java

luoyuxia · 2022-07-15T12:13:37Z

...k-connector-hive/src/main/java/org/apache/flink/table/endpoint/hive/HiveServer2Endpoint.java

+ TProtocolVersion clientProtocol = tOpenSessionReq.getClient_protocol();
+ HiveServer2EndpointVersion sessionVersion =
+ HiveServer2EndpointVersion.valueOf(
+ TProtocolVersion.findByValue(


TProtocolVersion.findByValue may return null, then HiveServer2EndpointVersion.valueOf(null) will throw the exception will be the message Unknown TProtocolVersion: null, which may not user friendly.

Actually it's impossible because we use min(server_version, ...) here. The negotiation results should be less than the Server Version.

luoyuxia · 2022-07-20T08:15:45Z

...ctor-hive/src/main/java/org/apache/flink/table/endpoint/hive/HiveServer2EndpointVersion.java

+
+ HIVE_CLI_SERVICE_PROTOCOL_V9(TProtocolVersion.HIVE_CLI_SERVICE_PROTOCOL_V9),
+
+ HIVE_CLI_SERVICE_PROTOCOL_V10(TProtocolVersion.HIVE_CLI_SERVICE_PROTOCOL_V10);


The version for Hive3 will be up to TProtocolVersion.HIVE_CLI_SERVICE_PROTOCOL_V11. But since miss it seems won't cause any problem, it's just a reminder.

Thanks for pointing out! After offline discussion, I think the only way to support HiveSever2 in Hive 3.x is to expose getHiveServer2Endpoint and getHiveServer2EndpointVersion in HiveShim. This is because HiveServer2Endpoint implements TCLIService.Iface and the interface is different in the different hive version.

Considering the HiveServer2EndpointVersion and HiveServer2Endpoint is not annotated with @PublicEvolving, I think we can expose this in the future.

luoyuxia · 2022-07-20T09:12:05Z

...ector-hive/src/test/java/org/apache/flink/table/endpoint/hive/HiveServer2EndpointITCase.java

+ }
+
+ @Test
+ public void testOpenSessionWithConfig() throws Exception {


Is it for open session with config? Seems we haven't set configuration in this test.

Yes. I add more configs here.

...ector-hive/src/test/java/org/apache/flink/table/endpoint/hive/HiveServer2EndpointITCase.java

…nd factory

luoyuxia

LGTM

fsk119 · 2022-07-23T10:48:20Z

@flinkbot run azure

…nd factory This closes apache#20101

fsk119 force-pushed the hivesever2-endpoint branch from 37c3c38 to 5dda594 Compare June 29, 2022 13:41

fsk119 force-pushed the hivesever2-endpoint branch 6 times, most recently from 40d67f3 to 4b5c49f Compare July 19, 2022 07:11

link3280 reviewed Jul 19, 2022

View reviewed changes

luoyuxia reviewed Jul 20, 2022

View reviewed changes

fsk119 force-pushed the hivesever2-endpoint branch from 7622bbe to e866c29 Compare July 20, 2022 12:51

fsk119 added 7 commits July 22, 2022 18:03

[FLINK-28150][sql-gateway][hive] Introduce the hiveserver2 endpoint a…

0964fed

…nd factory

[fix] introduce endpoint api dependencies

b85d07d

[fix] fix arch test can not use multiple annotations

3d800e5

[fix] minor improvements

6bff40f

[fix] fix arch test for sql-gateway-api and sql-gateway module

0b9f264

[fix] address comments and minor code improvements

a9d6ff1

[fix] address comments

d889c33

fsk119 force-pushed the hivesever2-endpoint branch from 62480f7 to 104a4d7 Compare July 22, 2022 10:10

luoyuxia approved these changes Jul 23, 2022

View reviewed changes

fsk119 force-pushed the hivesever2-endpoint branch from 104a4d7 to d889c33 Compare July 23, 2022 10:50

fsk119 closed this in d067629 Jul 24, 2022

huangxiaofeng10047 pushed a commit to huangxiaofeng10047/flink that referenced this pull request Nov 3, 2022

[FLINK-28150][sql-gateway][hive] Introduce the hiveserver2 endpoint a…

0ca5995

…nd factory This closes apache#20101

fsk119 deleted the hivesever2-endpoint branch August 9, 2023 08:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FLINK-28150][sql-gateway][hive] Introduce the hiveserver2 endpoint and factory #20101

[FLINK-28150][sql-gateway][hive] Introduce the hiveserver2 endpoint and factory #20101

fsk119 commented Jun 29, 2022

flinkbot commented Jun 29, 2022 •

edited

Loading

link3280 left a comment

link3280 Jul 19, 2022

fsk119 Jul 20, 2022

link3280 Jul 20, 2022

link3280 Jul 19, 2022

fsk119 Jul 20, 2022

luoyuxia left a comment

luoyuxia Jul 15, 2022

fsk119 Jul 20, 2022

luoyuxia Jul 15, 2022

fsk119 Jul 20, 2022

luoyuxia Jul 20, 2022

fsk119 Jul 20, 2022

fsk119 Jul 20, 2022

luoyuxia Jul 20, 2022

fsk119 Jul 20, 2022

luoyuxia left a comment

fsk119 commented Jul 23, 2022


		HIVE_CLI_SERVICE_PROTOCOL_V9(TProtocolVersion.HIVE_CLI_SERVICE_PROTOCOL_V9),

		HIVE_CLI_SERVICE_PROTOCOL_V10(TProtocolVersion.HIVE_CLI_SERVICE_PROTOCOL_V10);

[FLINK-28150][sql-gateway][hive] Introduce the hiveserver2 endpoint and factory #20101

[FLINK-28150][sql-gateway][hive] Introduce the hiveserver2 endpoint and factory #20101

Conversation

fsk119 commented Jun 29, 2022

What is the purpose of the change

Brief change log

Verifying this change

Does this pull request potentially affect one of the following parts:

Documentation

flinkbot commented Jun 29, 2022 • edited Loading

CI report:

link3280 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luoyuxia left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luoyuxia left a comment

Choose a reason for hiding this comment

fsk119 commented Jul 23, 2022

flinkbot commented Jun 29, 2022 •

edited

Loading