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

Run Phoenix product test after 5.2.1 upgrade #24519

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

virajjasani
Copy link

@virajjasani virajjasani commented Dec 18, 2024

Description

Enable Phoenix product tests.

HBase 2.5.10 has a fix required to stabilize Phoenix tests in the docker environment. Since Phoenix 5.2.1 with HBase 2.5.10-hadoop3 are already used by Trino, the fix is available for trino to consume.

Release notes

(x) This is not user-visible or is docs only, and no release notes are required.

Copy link

cla-bot bot commented Dec 18, 2024

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

Copy link

cla-bot bot commented Dec 18, 2024

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

@virajjasani
Copy link
Author

@wendigo could you please take a look?

@ebyhr
Copy link
Member

ebyhr commented Dec 18, 2024

Could you run stress tests likes #21480? Removing unnecessary jobs and duplicating a Phoenix job.

Copy link

cla-bot bot commented Dec 18, 2024

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

@virajjasani
Copy link
Author

Could you run stress tests likes #21480?

Could you please help me understand how I can run it? So far, I confirmed the changes with
testing/trino-product-tests-launcher/bin/run-launcher test run --config config-default --environment multinode-phoenix5 -- -g configured_features,phoenix

@ebyhr
Copy link
Member

ebyhr commented Dec 18, 2024

@virajjasani I mean removing unnecessary jobs and duplicating a Phoenix job (10 lines) in ci.yml.

@ebyhr ebyhr requested a review from lhofhansl December 18, 2024 21:52
@ebyhr
Copy link
Member

ebyhr commented Dec 18, 2024

@virajjasani
Copy link
Author

Is #21569 a client side issue?

No, it was server side issue, which is fixed with HBase 2.5.9 (Jira: HBASE-28567)

@virajjasani
Copy link
Author

virajjasani commented Dec 18, 2024

Our testing target version is still 5.2.0.

Thanks for pointing this! Can i update it in addition to this PR?

@ebyhr ebyhr changed the title Run Phoenix integration test after 5.2.1 upgrade Run Phoenix product test after 5.2.1 upgrade Dec 18, 2024
Copy link

cla-bot bot commented Dec 18, 2024

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

@virajjasani
Copy link
Author

Upgrade Phoenix version in docker image to 5.2.1: trinodb/docker-images#220

@ebyhr ebyhr marked this pull request as draft December 18, 2024 22:32
@virajjasani
Copy link
Author

@ebyhr I guess I might have removed even some necessary job, all the trino-phoenix5 jobs are failing. Probably I can restore everything back and still repeat trino-phoenix5 at least 20 times for stress testing?

Copy link

cla-bot bot commented Dec 18, 2024

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

Copy link

cla-bot bot commented Dec 19, 2024

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

@virajjasani
Copy link
Author

@ebyhr @lhofhansl The build result of stress test looks good, let me revert the changes and make PR ready for review.

Screenshot 2024-12-18 at 6 06 39 PM

Copy link

cla-bot bot commented Dec 19, 2024

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

@virajjasani virajjasani marked this pull request as ready for review December 19, 2024 02:09
@ebyhr
Copy link
Member

ebyhr commented Dec 19, 2024

@virajjasani You should run product tests pt, not integration tests test because you updated testing/trino-product-tests/src/main/java/io/trino/tests/product/phoenix/TestPhoenix.java

@ebyhr ebyhr marked this pull request as draft December 19, 2024 02:16
Copy link

cla-bot bot commented Dec 19, 2024

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

@virajjasani
Copy link
Author

@virajjasani You should run product tests pt, not integration tests test because you updated testing/trino-product-tests/src/main/java/io/trino/tests/product/phoenix/TestPhoenix.java

I am bit confused. It seems Trino does not have SuitePhoenix* or suite-phoenix* in ci.yaml. Also, #21569 build results were also focused on trino-phoenix5 unless I am mistaken. I might be missing something fundamental here.

@ebyhr
Copy link
Member

ebyhr commented Dec 19, 2024

@virajjasani You can check Suite6NonGeneric.

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

Successfully merging this pull request may close these issues.

2 participants