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

[hive] add hive client pool #3004

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

Conversation

zhuangchong
Copy link
Contributor

Purpose

add hive client pool

Tests

API and Format

Documentation

@XuQianJin-Stars
Copy link
Contributor

hi @zhuangchong Is there any difference between this PR and #2567 ?The e2e test has been unstable before.

@zhuangchong
Copy link
Contributor Author

hi @zhuangchong Is there any difference between this PR and #2567 ?The e2e test has been unstable before.

There is no difference. PR 2567 has not been updated for a long time, and this feature is very needed, so I launched a PR again, merging the latest code and solving the UT exception.

If you have time and are willing to update PR 2567, I can close this PR. @XuQianJin-Stars

@XuQianJin-Stars
Copy link
Contributor

hi @zhuangchong Is there any difference between this PR and #2567 ?The e2e test has been unstable before.

There is no difference. PR 2567 has not been updated for a long time, and this feature is very needed, so I launched a PR again, merging the latest code and solving the UT exception.

If you have time and are willing to update PR 2567, I can close this PR. @XuQianJin-Stars

I understand. I don't care about this PR merge. I will close PR 2567. The reason I left a comment is to tell me you want to use this PR 2567.

@caicancai
Copy link
Member

caicancai commented Mar 15, 2024

hi @zhuangchong I think it's ok to add XuQianJin-Stars as co-author

@zhuangchong
Copy link
Contributor Author

hi @zhuangchong I think it's ok to add XuQianJin-Stars as co-author

Where is this added?

@zhangjun0x01
Copy link
Contributor

hi @zhuangchong I think it's ok to add XuQianJin-Stars as co-author

Where is this added?

For this situation, I think you can make changes based on the original pr and then push the modified code to the original author's repository

@caicancai
Copy link
Member

caicancai commented Mar 15, 2024

For this situation, I think you can make changes based on the original pr and then push the modified code to the original author's repository

@zhuangchong Add co-author information in the commit message, for example:

apache/calcite@f0dc2b0

@zhuangchong zhuangchong force-pushed the hive-client-pool branch 2 times, most recently from 217ad91 to 355641a Compare March 20, 2024 04:54
@JingsongLi
Copy link
Contributor

Hi @zhuangchong
What is the status? I remember passing the test? What have you done now?

@zhuangchong
Copy link
Contributor Author

Hi @zhuangchong What is the status? I remember passing the test? What have you done now?

@JingsongLi I merged the latest code, and then ut error package cannot be found, I am looking for the problem.

@JingsongLi
Copy link
Contributor

@zhuangchong You mean you just cherry-picked #2567 ? Why not kust keep your code?

@zhuangchong
Copy link
Contributor Author

@zhuangchong You mean you just cherry-picked #2567 ? Why not kust keep your code?

The CI is experiencing anomalies after merging the latest code into the master branch.

@sunxiaojian
Copy link
Contributor

Is CachedClientPool considering creating an abstraction layer? JdbcClientPool may also need to be implemented through caching

@JingsongLi
Copy link
Contributor

Sorry for the late reply.

I've been thinking for a long time, why do we need a client pool? Are there any user needs?

@zhuangchong
Copy link
Contributor Author

Sorry for the late reply.

I've been thinking for a long time, why do we need a client pool? Are there any user needs?

A user reported that Paimon uses hive catalog, and the number of hive metastore connections exceeds 10,000.
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants