Skip to content

Fix calculate error of bytesToReadInPage in AlluxioInputHelper.putBuffer#23175

Merged
raunaqmorarka merged 1 commit intotrinodb:masterfrom
coderwf:bugfix_alluxio_cache
Sep 24, 2024
Merged

Fix calculate error of bytesToReadInPage in AlluxioInputHelper.putBuffer#23175
raunaqmorarka merged 1 commit intotrinodb:masterfrom
coderwf:bugfix_alluxio_cache

Conversation

@coderwf
Copy link
Contributor

@coderwf coderwf commented Aug 29, 2024

Description

Additional context and related issues

Fixes #23172

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text:

# Hive, Delta, Iceberg
* Improve cache hit ratio when filesystem cache is enabled. ({issue}`23172`)

@cla-bot
Copy link

cla-bot bot commented Aug 29, 2024

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: wangdongwei.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@coderwf coderwf force-pushed the bugfix_alluxio_cache branch from 04a4efa to 878ba94 Compare August 29, 2024 11:31
@cla-bot
Copy link

cla-bot bot commented Aug 29, 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 cla@trino.io. 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

@coderwf coderwf force-pushed the bugfix_alluxio_cache branch from 878ba94 to 8be0562 Compare August 29, 2024 11:34
@cla-bot
Copy link

cla-bot bot commented Aug 29, 2024

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: wangdongwei.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@coderwf coderwf force-pushed the bugfix_alluxio_cache branch from 8be0562 to e676bee Compare August 30, 2024 07:16
@cla-bot
Copy link

cla-bot bot commented Aug 30, 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 cla@trino.io. 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

@coderwf
Copy link
Contributor Author

coderwf commented Aug 30, 2024

#23172 for this issue

@github-actions github-actions bot added the delta-lake Delta Lake connector label Aug 30, 2024
@cla-bot
Copy link

cla-bot bot commented Aug 30, 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 cla@trino.io. 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

@coderwf coderwf force-pushed the bugfix_alluxio_cache branch from 6822274 to 18d2654 Compare August 30, 2024 08:19
@cla-bot
Copy link

cla-bot bot commented Aug 30, 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 cla@trino.io. 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

@coderwf coderwf force-pushed the bugfix_alluxio_cache branch from 18d2654 to 8110868 Compare September 2, 2024 08:53
@cla-bot
Copy link

cla-bot bot commented Sep 2, 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 cla@trino.io. 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

@coderwf coderwf force-pushed the bugfix_alluxio_cache branch from 8110868 to 19bb5b3 Compare September 3, 2024 07:00
@cla-bot
Copy link

cla-bot bot commented Sep 3, 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 cla@trino.io. 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

@wendigo
Copy link
Contributor

wendigo commented Sep 3, 2024

@raunaqmorarka ptal

@coderwf coderwf force-pushed the bugfix_alluxio_cache branch from 19bb5b3 to b8b060c Compare September 3, 2024 11:39
@cla-bot
Copy link

cla-bot bot commented Sep 3, 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 cla@trino.io. 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

@coderwf coderwf force-pushed the bugfix_alluxio_cache branch from b8b060c to eb16dd3 Compare September 6, 2024 02:16
@cla-bot cla-bot bot added the cla-signed label Sep 6, 2024
@coderwf coderwf force-pushed the bugfix_alluxio_cache branch 3 times, most recently from b45f6a9 to a71ccda Compare September 9, 2024 03:31
@coderwf
Copy link
Contributor Author

coderwf commented Sep 9, 2024

@jkylling @raunaqmorarka take a review please

Copy link
Contributor

@jkylling jkylling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for fixing this!

I'm curious how did you discover this bug?

Copy link
Member

@raunaqmorarka raunaqmorarka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderwf please address the above comments.
Other than that, PR lgtm

@coderwf
Copy link
Contributor Author

coderwf commented Sep 24, 2024

Thank you for fixing this!

I'm curious how did you discover this bug?

@jkylling I used paimon for trino to query the data stored on HDFS service, the data amount is about 60GB and contains lots of small files , so query latency fluctuates greatly when HDFS is unstable, then I enabled the Alluxio filesystem cache which store data on local ssd, but the performance is not very good, so I suspected it is caused by the low cache hit rate, through the opentelemetry service I found that there are indeed a lot of small data queries still requesting hdfs.
I don't know what caused it because I used the same query every time and the data didn't change. Finally, I read this part of the code carefully and found that there seemed to be something wrong with the code logic here, after I fixed it and started query more times, query latency becomes small and stable

@coderwf coderwf force-pushed the bugfix_alluxio_cache branch from a71ccda to 1db728d Compare September 24, 2024 10:24
@raunaqmorarka raunaqmorarka merged commit c7e919a into trinodb:master Sep 24, 2024
@github-actions github-actions bot added this to the 459 milestone Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

AlluxioHelper bug causes cache miss

4 participants