[WIP][SPARK-29108][SQL] Add new module sql/thriftserver with all code and UT #26340

AngersZhuuuu · 2019-10-31T08:41:20Z

What changes were proposed in this pull request?

As comment #26221 (comment)
Raise a pr with entire code and pass origin hive-thriftserver 's UT

Changes:

Impelemnt Type in scala since spark con't support all type of hive
Implement Service/AbstractService prepare for remove hive conf in future
Construct RowSet with StructType and Row
Implement HiveAuthFactory since between 1.2.1/2.3.5, their delegation token managerment changed. Impelment on DelegationTokenMnagerment by scala
MV tableTypeString from SparkMetadataOperationUtils to SparkMetadataOperation
Since there are tableTypeString in SparkMetadataOperation remove ClassicTypeMapping, HiveTableTypeMapping, TableTypeMapping and TableTypeMappingFactory
Implement all oepration for spark since it execute in different way
Add new method GetQueryId, SetClientInfo for thrift version v11 in ThriftCLIService
Add statementid to Operation for implement GetQueryId
Remove GlobalHivercFileProcessor setFetchSize processGlobalInitFileetc
Remove unused openSession openSessionWithImpersonation in CLIService
Copy hive-thriftserver test to spark-thriftserver and remove ThriftSeverShimUtils about test UT, since we won't need it.
Implement it's own thrift client code. CLIServiceClient ThrictCLIServiceClient

Why are the changes needed?

Solve hive version conflics and finally build a thriftserver make hive plugability.

Does this PR introduce any user-facing change?

start and stop

./sbin/start-spark-thriftserver.sh
./sbin/stop-spark-thriftserver.sh

use beeline connect as origin.

How was this patch tested?

UT

…ft-maven-plugin

Maven generate thrift source code

… into SPARK-29018-V11

This reverts commit 95d8137.

juliuszsompolski · 2019-11-04T15:58:50Z

Thanks @AngersZhuuuu . I will have time to come back to look into it more in a few days.
In the meanwhile, could you get the PR to compile and run tests? I think that for that you have to commit the thrift generated sources after all. We could keep the maven build step as an optional manual step that can be executed when somebody wants to regenerate it, and add instructions for it to sql/thriftserver/README.md.
Can the maven plugin enforce the thrift compiler version? We could add instructions to README to use 0.9.3, and get and install it from https://www.apache.org/dist/thrift/0.9.3/. Otherwise, I found that even small thrift version differences lead to tiny annoying differences in all the generated files that make a huge diff when I regenerate it.

I will look at @yaooqinn Kyuubi more as well. I wasn't aware of it's existence before! I think it could be great if improvements from there could be intergrated into mainline Spark. But I don't know how compatible it is with current thriftserver deployments? Kyuubi seems to not have a HTTP server right now, and also seems to be dependent on hive-1.2.1?

AngersZhuuuu · 2019-11-05T14:01:28Z

Thanks @AngersZhuuuu . I will have time to come back to look into it more in a few days.
In the meanwhile, could you get the PR to compile and run tests? I think that for that you have to commit the thrift generated sources after all. We could keep the maven build step as an optional manual step that can be executed when somebody wants to regenerate it, and add instructions for it to sql/thriftserver/README.md.

I will make it pass build and test.

yaooqinn · 2019-11-05T15:09:09Z

@juliuszsompolski thanks for instesting in kyuubi project. The hive deps can be removed easily but still existing because of lack of time and not a big deal before Spark 3.0.0. The only interface for kyuubi to access metastore should be
through SparkSession. The kyuubi stays as a standalone project is just for multiple spark version support （2.1-2.4 for now）. I am also willing to help with Spark to apply kyuubi’s features. There are much more scenarios that use the cluster manager i.e. yarn，k8s as computing resousce serving layer not just inside one spark application. Currently，kyuubi supports only “client” mode，hacking a bit but solved with Spark’s ChildFirstClassLoader. When “cluster” mode will be supported, the kyuubi need not to hack spark to support multi tenancy

juliuszsompolski · 2019-11-06T10:31:48Z

...rc/gen/thrift/gen-javabean/org/apache/spark/sql/thriftserver/cli/thrift/TArrayTypeEntry.java

@@ -0,0 +1,363 @@
+/**
+ * Autogenerated by Thrift Compiler (0.12.0)


The code generated in Hive master seems to use 0.9.3. To avoid diffs, could you compile and use 0.9.3 from https://www.apache.org/dist/thrift/0.9.3/, and add to the readme to use it?

The code generated in Hive master seems to use 0.9.3. To avoid diffs, could you compile and use 0.9.3 from https://www.apache.org/dist/thrift/0.9.3/, and add to the readme to use it?

Since local is 0.12..... I will use 0.93 version to rebuild this.

@AngersZhuuuu Could we just add hive-service-rpc to our maven dependency?

<dependency> <groupId>org.apache.hive</groupId> <artifactId>hive-service-rpc</artifactId> <version>2.3.6</version> </dependency>

@wangyum We would be creating a dependency on a specific Hive version then again, because ThriftCLIService.java and ThriftCLIServiceClient.java and a few others depend on the exact version of the RPCs. Better to embed these.

juliuszsompolski · 2019-11-06T10:33:42Z

sql/thriftserver/README.md

@@ -0,0 +1,3 @@
+Thrift commands to generate files from TCLIService.thrift:
+--------------------
+thrift --gen java:beans,hashcode -o src/gen/thrift if/TCLIService.thrift


Could you add generated_annotations=undated to avoid diffs?
And add a sentence like Please use Thrift 0.9.3 available from https://www.apache.org/dist/thrift/0.9.3/.

Could you add generated_annotations=undated to avoid diffs?
And add a sentence like Please use Thrift 0.9.3 available from https://www.apache.org/dist/thrift/0.9.3/.

Sorry for didn't notice this point. I'll update this later.

wangyum · 2019-11-06T11:46:47Z

sql/thriftserver/pom.xml

+        </dependency>
+        <dependency>
+        <groupId>${hive.group}</groupId>
+        <artifactId>hive-service</artifactId>


Our thriftserver module is similar to hive-service. Could we adapt hive-beeline and hive-jdbc to this module?

👍 for doing that - to have our own client as a separate module, but not in the scope of this.

For now (in one of followup PR to this), we could start by pulling the client side into a new module (sql/jdbc-client?). I think thriftserver doesn't need to depend on hive-service and hive-beeline. These are needed only for a client.

Our thriftserver module is similar to hive-service. Could we adapt hive-beeline and hive-jdbc to this module?

Can't agree more to do this. Will make spark more independent, and we can have our own jdbc schema as jdbc:spark such as impala and presto.

.github/workflows/master.yml

cloud-fan · 2019-11-07T14:56:40Z

Does this PR re-implement the thrift server on its own to get rid of Hive dependency? Is this a new architecture like https://github.com/yaooqinn/kyuubi ?

AngersZhuuuu · 2019-11-08T00:59:04Z

Does this PR re-implement the thrift server on its own to get rid of Hive dependency? Is this a new architecture like https://github.com/yaooqinn/kyuubi ?

No, what we want to do:

make a thriftserver can cover current function and fit for different hive version, then we won't nee to make folder like v1.2.1/v2.3.5 to support different hive version
Build our own beeline client and jdbc client, then we can remove rely on hive-beeline hive-jdbc
Finally make hive as a plugin. Build a thriftserver can run without/with hive.

cloud-fan · 2019-11-08T01:59:39Z

make a thriftserver can cover current function and fit for different hive version

How is this done?

AngersZhuuuu · 2019-11-08T02:27:34Z

How is this done?

Currently only rely on hive's basic class such as HiveConf SessionState etc.. finally we will extract it out.
Thrift protocol class we build our own, code about ThriftServer framework we use ourself's code rely on our own Thrift protocol file.
Some class we implement one in spark's own code such as HiveAuthUtils, these class is independent so we can do like this.
Remove unused code and remove unused function.

cloud-fan · 2019-11-08T02:33:36Z

can we do it incrementally? e.g each PR remove one file/class/method from v1.2.1/v2.3.5 and add necessary utils, and eventually make v1.2.1/v2.3.5 disappear.

AngersZhuuuu · 2019-11-08T02:41:40Z

can we do it incrementally? e.g each PR remove one file/class/method from v1.2.1/v2.3.5 and add necessary utils, and eventually make v1.2.1/v2.3.5 disappear.

Hard to do like that, we have discussed a log about this, @juliuszsompolski @wangyum
Change based on origin hive-thriftserver is also hard to review.

cloud-fan · 2019-11-08T02:56:33Z

This PR is too hard to review. If it's necessary to spend a lot of effort to re-implement a thrift server, shall we consider merging https://github.com/yaooqinn/kyuubi into Spark?

AngersZhuuuu · 2019-11-08T03:00:54Z

This PR is too hard to review. If it's necessary to spend a lot of effort to re-implement a thrift server, shall we consider merging https://github.com/yaooqinn/kyuubi into Spark?

kyuubi is based on hive-1.2.1 so if we want to use, still need to fix.
and it's mode is one user one sparkcontext (you know this will make a lot problem).
And kyuubi remove a lot functions.

yaooqinn · 2019-11-08T03:03:30Z

This PR is too hard to review. If it's necessary to spend a lot of effort to re-implement a thrift server, shall we consider merging https://github.com/yaooqinn/kyuubi into Spark?

kyuubi is based on hive-1.2.1 so if we want to use, still need to fix.

kyuubi only build with hive-1.2.1 to reuse its OperationLog logic， which can be replaced

and it's mode is one user one sparkcontext (you know this will make a lot problem).

Yes, it has been used on a production environment for more than two years，we had a lot of problems fixed. one user one sparkcontext in single jvm is the current usage, we may have a better idea to launch sparkcontext clusterly

And kyuubi remove a lot functions.

The newest kyuubi have no function removed, only the http thrift server support

cloud-fan · 2019-11-08T04:46:01Z

@AngersZhuuuu do we have a doc to help review? i.e. which part is server and which part is client, how these classes interact with each other, etc.

AngersZhuuuu · 2019-11-08T06:26:06Z

@AngersZhuuuu do we have a doc to help review? i.e. which part is server and which part is client, how these classes interact with each other, etc.

Ok, will make it more clear.

github-actions · 2020-03-09T00:13:36Z

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

AngersZhuuuu and others added 30 commits October 22, 2019 15:28

Add new module and add v11 thrift protocol

8b56400

add pom lincense

e30f686

update code folder name

4a50bc9

all like hive

7ffdc3b

remove py/cpp/r/php

b74d5e0

Maven generate thrift source code

fc2648f

org.apache.thrift.tools:maven-thrift-plugin -> org.apache.thrift:thri…

5365dcf

…ft-maven-plugin

save some basic code

95d8137

Merge pull request #2 from wangyum/SPARK-29108

4dc5c7e

Maven generate thrift source code

Merge branch 'SPARK-29018-V11' of https://github.com/AngersZhuuuu/spark…

24ce6d4

… into SPARK-29018-V11

Revert "save some basic code"

5efe8cb

This reverts commit 95d8137.

Update TCLIService.thrift

6f7d48a

Add basic data

cf24306

save code

24fce6b

change

bc36bdf

format code

c438673

format code

a7ec8b0

Update SparkSQLEnv.scala

5cf7eb0

fix logger conflict

0023bcb

fix scala style

723450b

start with execution hive

3eb7672

format code add file header

5aa4d9d

FOR BUILD spark-thriftserver

782d36b

add UT class

f2b5346

fix UT case, remove can't supported UT

a14a9e9

fix for UT

e1bb6e1

fix UT

74fb240

fix for UT

2f90ed9

fix return error

cd92f3f

fix client convert row map

60dc24f

dongjoon-hyun added the TESTS label Nov 3, 2019

AngersZhuuuu added 3 commits November 6, 2019 14:39

add thrift file

f8b7351

not check current java code now

272ba3c

remove import jdk.tools dependency

0de3191

juliuszsompolski reviewed Nov 6, 2019

View reviewed changes

follow comment

6374f42

wangyum reviewed Nov 6, 2019

View reviewed changes

wangyum reviewed Nov 7, 2019

View reviewed changes

.github/workflows/master.yml Outdated Show resolved Hide resolved

AngersZhuuuu added 3 commits November 7, 2019 09:51

Merge branch 'master' into SPARK-29018-V11-STEP4-ADD-TEST

f464773

fix for mvn install

13da926

fix for mvn install

d0de49f

juliuszsompolski mentioned this pull request Dec 3, 2019

Rename HIVE_CLI_SERVICE_PROTOCOL to SPARK_SQL_SERVICE_PROTOCOL spark-thriftserver/spark-thriftserver#18

Closed

javierivanov mentioned this pull request Dec 4, 2019

[SPARK-29685][SQL] Adds format to the spark-sql output like its implemented by Dataset showString #26698

Closed

github-actions bot added the Stale label Mar 9, 2020

github-actions bot closed this Mar 10, 2020

		@@ -0,0 +1,363 @@
		/**
		* Autogenerated by Thrift Compiler (0.12.0)

[WIP][SPARK-29108][SQL] Add new module sql/thriftserver with all code and UT #26340

[WIP][SPARK-29108][SQL] Add new module sql/thriftserver with all code and UT #26340

Uh oh!

Conversation

AngersZhuuuu commented Oct 31, 2019

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

juliuszsompolski commented Nov 4, 2019

Uh oh!

AngersZhuuuu commented Nov 5, 2019

Uh oh!

yaooqinn commented Nov 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cloud-fan commented Nov 7, 2019

Uh oh!

AngersZhuuuu commented Nov 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cloud-fan commented Nov 8, 2019

Uh oh!

AngersZhuuuu commented Nov 8, 2019

Uh oh!

cloud-fan commented Nov 8, 2019

Uh oh!

AngersZhuuuu commented Nov 8, 2019

Uh oh!

cloud-fan commented Nov 8, 2019

Uh oh!

AngersZhuuuu commented Nov 8, 2019

Uh oh!

yaooqinn commented Nov 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cloud-fan commented Nov 8, 2019

Uh oh!

AngersZhuuuu commented Nov 8, 2019

Uh oh!

github-actions bot commented Mar 9, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

yaooqinn commented Nov 5, 2019 •

edited

Loading

AngersZhuuuu commented Nov 8, 2019 •

edited

Loading

yaooqinn commented Nov 8, 2019 •

edited

Loading