[KYUUBI #6034] Kyuubi Server HA&ZK get server from serverHosts support more strategy

[davidyuan1223]

🔍 Description

Issue References 🔗

This pull request fixes #6034

Describe Your Solution 🔧

Currently, use beeline to connect kyuubiServer with HA mode, the strategy only support random, this will lead to a high load on the machine. So i make this pr to support choose strategy.
[description]
First, we need know, beeline connect kyuubiServer dependency on kyuubi-hive-jdbc, it is isolated from the kyuubi cluster, so the code only support random choose serverHost from zk node /${namespace}. Because kyuubi-hive-jdbc is a stateless module, only run once, cannot store var about get serverHost from zk node.
[Solution]
This pr, we could implement a interface named ChooseServerStrategy to choose serverHost. I implement two strategy

1. poll: it will create a zk node named ${namespace}-counter, when a beeline client want connect kyuubiServer, the node will increment 1, use this value to take the remainder from serverHosts, like counter % serverHost.size, so we could get a order serverHost
2. random: random get serverHost from serverHosts
3. User Definied Class: implemented the ChooseServerStrategy, then put the jar to beeline-jars, it can use your strategy to choose serverHost

Types of changes 🔖

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Test Plan 🧪

Test the Strategy in my test Cluster

Behavior Without This Pull Request ⚰️

Behavior With This Pull Request 🎉

[Use Case]

1. poll: bin/beeline -u 'jdbc:hive2://xxx:2181/;serviceDiscoveryMode=zooKeeper;zooKeeperNamespace=kyuubi;zooKeeperStrategy=poll?spark.yarn.queue=root.kylin;spark.app.name=testspark;spark.shuffle.useOldFetchProtocol=true' -n mfw_hadoop --verbose=true --showNestedErrs=true
2. random: bin/beeline -u 'jdbc:hive2://xxx:2181/;serviceDiscoveryMode=zooKeeper;zooKeeperNamespace=kyuubi;zooKeeperStrategy=random?spark.yarn.queue=root.kylin;spark.app.name=testspark;spark.shuffle.useOldFetchProtocol=true' -n mfw_hadoop --verbose=true --showNestedErrs=true or bin/beeline -u 'jdbc:hive2://xxx:2181/;serviceDiscoveryMode=zooKeeper;zooKeeperNamespace=kyuubi?spark.yarn.queue=root.kylin;spark.app.name=testspark;spark.shuffle.useOldFetchProtocol=true' -n mfw_hadoop --verbose=true --showNestedErrs=true
3. YourStrategy: bin/beeline -u 'jdbc:hive2://xxx:2181/;serviceDiscoveryMode=zooKeeper;zooKeeperNamespace=kyuubi;zooKeeperStrategy=xxx.xxx.xxx.XxxChooseServerStrategy?spark.yarn.queue=root.kylin;spark.app.name=testspark;spark.shuffle.useOldFetchProtocol=true' -n mfw_hadoop --verbose=true --showNestedErrs=true

[Result: The Cluster have two Server (221,233)]

1. poll:
   1.1. zkNode: counterValue
   

1.2. result:

2. random:
   
   
   

3. YourStrategy(the test case only get the first serverHost):
   
   
   

Related Unit Tests

There is no Unit Tests.

Checklist 📝

• This patch was not authored or co-authored using Generative Tooling

Be nice. Be informative.

[davidyuan1223]

@yaooqinn @wForget @pan3793 what do you think?

[yaooqinn]

LGTM in general, can we add some tests?

[davidyuan1223]

LGTM in general, can we add some tests?

i'm not sure did this improvement could execute unit test? But i will try

[yaooqinn]

function level unit test is OK

[cxzl25]

It may be similar to the idea of ​​selecting Engine that we are planning to implement. #5662 cc @lsm1

[davidyuan1223]

@cxzl25 @yaooqinn what do you think about the unit test case in ha module(because we need start a zk docker to test the poll strategy)

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 0% with 32 lines in your changes missing coverage. Please review.

Project coverage is 0.00%. Comparing base (1d668ea) to head (961d3e9).
Report is 4 commits behind head on master.

Files with missing lines	Patch %	Lines
...dbc/hive/strategy/ServerSelectStrategyFactory.java	0.00%	12 Missing ⚠️
...he/kyuubi/jdbc/hive/ZooKeeperHiveClientHelper.java	0.00%	9 Missing ⚠️
...i/jdbc/hive/strategy/zk/PollingSelectStrategy.java	0.00%	9 Missing ⚠️
...bi/jdbc/hive/strategy/zk/RandomSelectStrategy.java	0.00%	2 Missing ⚠️

Additional details and impacted files

@@          Coverage Diff           @@
##           master   #6213   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files         684     687    +3     
  Lines       42354   42439   +85     
  Branches     5776    5793   +17     
======================================
- Misses      42354   42439   +85     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[bowenliang123]

Suggested change

	static final String ZOOKEEPER_STRATEGY = "zooKeeperStrategy";
	static final String ZOOKEEPER_SELECT_POLICY = "zooKeeperSelectPolicy";

Would it be a better name for the parameter? cc @pan3793
The term select policy is similar with existed config kyuubi.engine.pool.selectPolicy of kyuubi server.

[pan3793]

"select policy" is a better phrase here.

technically, the mechanism could be promoted to another service discovery impls, so we should not include "zookeeper" in the parameter name.

so my suggested parameter name is "serverSelectPolicy", and also rename interface "ChooseServerStrategy" to "ServerSelectStrategy"

[bowenliang123]

SGTM. Agree with Pan's suggestion.

[wForget]

LGTM, thanks.