Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

davidyuan1223
Copy link
Contributor

@davidyuan1223 davidyuan1223 commented Mar 27, 2024

🔍 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 ⚰️

image
image
image

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
    image

1.2. result:
image
image
image
image

  1. random:
    image
    image
    image

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

Related Unit Tests

There is no Unit Tests.

Checklist 📝

Be nice. Be informative.

@davidyuan1223
Copy link
Contributor Author

davidyuan1223 commented Mar 27, 2024

@yaooqinn @wForget @pan3793 what do you think?

@yaooqinn
Copy link
Member

LGTM in general, can we add some tests?

@davidyuan1223
Copy link
Contributor Author

LGTM in general, can we add some tests?

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

@yaooqinn
Copy link
Member

function level unit test is OK

@cxzl25
Copy link
Contributor

cxzl25 commented Mar 28, 2024

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

@davidyuan1223
Copy link
Contributor Author

@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
Copy link

codecov-commenter commented Mar 28, 2024

Codecov Report

Attention: Patch coverage is 55.17241% with 13 lines in your changes are missing coverage. Please review.

Project coverage is 58.43%. Comparing base (3b2e674) to head (d6c58ee).
Report is 170 commits behind head on master.

Files Patch % Lines
...che/kyuubi/jdbc/hive/strategy/StrategyFactory.java 16.66% 9 Missing and 1 partial ⚠️
...i/jdbc/hive/strategy/zk/PollingChooseStrategy.java 66.66% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #6213      +/-   ##
============================================
- Coverage     61.10%   58.43%   -2.68%     
- Complexity       23       24       +1     
============================================
  Files           622      655      +33     
  Lines         37036    39673    +2637     
  Branches       5024     5456     +432     
============================================
+ Hits          22631    23182     +551     
- Misses        11967    14003    +2036     
- Partials       2438     2488      +50     

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

@cxzl25 cxzl25 added this to the v1.10.0 milestone Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TASK][EASY] Kyuubi Server HA&ZK get server from serverHosts support more strategy
4 participants