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

Prevent cache write from exceeding IOV_MAX #9438

Closed
wants to merge 1 commit into from

Conversation

zacw7
Copy link
Contributor

@zacw7 zacw7 commented Apr 10, 2024

Differential Revision: D55941557

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 10, 2024
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D55941557

Copy link

netlify bot commented Apr 10, 2024

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 6e85e53
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/661b33825b4a0a0008b39425

@zacw7 zacw7 force-pushed the export-D55941557 branch from 9ae89ae to 0463349 Compare April 10, 2024 23:06
zacw7 added a commit to zacw7/velox that referenced this pull request Apr 10, 2024
Summary: Pull Request resolved: facebookincubator#9438

Differential Revision: D55941557
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D55941557

@zacw7 zacw7 requested review from xiaoxmeng and amitkdutta April 10, 2024 23:07
@zacw7 zacw7 force-pushed the export-D55941557 branch from 0463349 to d7fb346 Compare April 11, 2024 06:01
zacw7 added a commit to zacw7/velox that referenced this pull request Apr 11, 2024
Summary: Pull Request resolved: facebookincubator#9438

Differential Revision: D55941557
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D55941557

zacw7 added a commit to zacw7/velox that referenced this pull request Apr 11, 2024
Summary: Pull Request resolved: facebookincubator#9438

Differential Revision: D55941557
@zacw7 zacw7 force-pushed the export-D55941557 branch from d7fb346 to fa326ab Compare April 11, 2024 06:01
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D55941557

@zacw7 zacw7 force-pushed the export-D55941557 branch from fa326ab to 8620dc2 Compare April 11, 2024 23:43
zacw7 added a commit to zacw7/velox that referenced this pull request Apr 11, 2024
Summary: Pull Request resolved: facebookincubator#9438

Differential Revision: D55941557
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D55941557

zacw7 added a commit to zacw7/velox that referenced this pull request Apr 11, 2024
Summary: Pull Request resolved: facebookincubator#9438

Differential Revision: D55941557
@zacw7 zacw7 force-pushed the export-D55941557 branch from 8620dc2 to d828f0b Compare April 11, 2024 23:45
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D55941557

@amitkdutta
Copy link
Contributor

This is a very long lasting bug. Thanks @zacw7 for working on it.
CC: @czentgr @majetideepak

Copy link
Contributor

@xiaoxmeng xiaoxmeng left a comment

Choose a reason for hiding this comment

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

@zacw7 thanks for the update % minors

velox/common/caching/SsdFile.cpp Show resolved Hide resolved
velox/common/caching/SsdFile.cpp Outdated Show resolved Hide resolved
velox/common/caching/SsdFile.cpp Outdated Show resolved Hide resolved
velox/common/caching/SsdFile.cpp Outdated Show resolved Hide resolved
velox/common/caching/SsdFile.h Outdated Show resolved Hide resolved
velox/common/caching/tests/SsdFileTest.cpp Show resolved Hide resolved
velox/common/caching/SsdFile.cpp Outdated Show resolved Hide resolved
velox/common/caching/SsdFile.cpp Outdated Show resolved Hide resolved
velox/common/caching/SsdFile.cpp Outdated Show resolved Hide resolved
velox/common/caching/SsdFile.cpp Outdated Show resolved Hide resolved
zacw7 added a commit to zacw7/velox that referenced this pull request Apr 12, 2024
Summary: Pull Request resolved: facebookincubator#9438

Differential Revision: D55941557
@zacw7 zacw7 force-pushed the export-D55941557 branch from d828f0b to 9369aa3 Compare April 12, 2024 05:09
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D55941557

zacw7 added a commit to zacw7/velox that referenced this pull request Apr 12, 2024
Summary: Pull Request resolved: facebookincubator#9438

Differential Revision: D55941557
@zacw7 zacw7 force-pushed the export-D55941557 branch from 9369aa3 to a155b0f Compare April 12, 2024 05:09
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D55941557

Copy link
Contributor

@xiaoxmeng xiaoxmeng left a comment

Choose a reason for hiding this comment

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

@zacw7 LGTM. Please also run this patch in interactive cluster before land. Thanks!

velox/common/caching/SsdFile.cpp Outdated Show resolved Hide resolved
velox/common/caching/SsdFile.cpp Outdated Show resolved Hide resolved
velox/common/caching/SsdFile.cpp Show resolved Hide resolved
zacw7 added a commit to zacw7/velox that referenced this pull request Apr 12, 2024
Summary: Pull Request resolved: facebookincubator#9438

Reviewed By: xiaoxmeng

Differential Revision: D55941557
@zacw7 zacw7 force-pushed the export-D55941557 branch from a155b0f to b53b0f3 Compare April 12, 2024 06:58
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D55941557

zacw7 added a commit to zacw7/velox that referenced this pull request Apr 12, 2024
Summary: Pull Request resolved: facebookincubator#9438

Reviewed By: xiaoxmeng

Differential Revision: D55941557
@zacw7 zacw7 force-pushed the export-D55941557 branch from b53b0f3 to 404bc35 Compare April 12, 2024 06:59
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D55941557

@amitkdutta
Copy link
Contributor

@zacw7 Great fix. Will be great if you can add error logs that we see now, which will no longer appear anymore. Also I assume this will translate to better cache hit rate?

zacw7 added a commit to zacw7/velox that referenced this pull request Apr 12, 2024
Summary: Pull Request resolved: facebookincubator#9438

Reviewed By: xiaoxmeng

Differential Revision: D55941557
@zacw7 zacw7 force-pushed the export-D55941557 branch from 404bc35 to c79a065 Compare April 12, 2024 17:08
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D55941557

zacw7 added a commit to zacw7/velox that referenced this pull request Apr 12, 2024
Summary: Pull Request resolved: facebookincubator#9438

Reviewed By: xiaoxmeng

Differential Revision: D55941557
@zacw7 zacw7 force-pushed the export-D55941557 branch from c79a065 to d6bdb2f Compare April 12, 2024 17:09
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D55941557

zacw7 added a commit to zacw7/velox that referenced this pull request Apr 14, 2024
Summary: Pull Request resolved: facebookincubator#9438

Reviewed By: xiaoxmeng

Differential Revision: D55941557
@zacw7 zacw7 force-pushed the export-D55941557 branch from d6bdb2f to 9b332cf Compare April 14, 2024 01:37
Summary: Pull Request resolved: facebookincubator#9438

Reviewed By: xiaoxmeng

Differential Revision: D55941557
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D55941557

@zacw7 zacw7 force-pushed the export-D55941557 branch from 9b332cf to 6e85e53 Compare April 14, 2024 01:38
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D55941557

@zacw7 zacw7 deleted the export-D55941557 branch April 14, 2024 17:33
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 8d09dc9.

Copy link

Conbench analyzed the 1 benchmark run on commit 8d09dc90.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

zacw7 added a commit to zacw7/velox that referenced this pull request May 6, 2024
facebook-github-bot pushed a commit that referenced this pull request May 7, 2024
Summary:
This reverts commit 8d09dc9.

Pull Request resolved: #9722

Reviewed By: amitkdutta

Differential Revision: D57004604

Pulled By: zacw7

fbshipit-source-id: e792ca6ecb376ad8a95d8ce3de819c7faeac5e0a
Joe-Abraham pushed a commit to Joe-Abraham/velox that referenced this pull request May 8, 2024
…#9438)" (facebookincubator#9722)

Summary:
This reverts commit 8d09dc9.

Pull Request resolved: facebookincubator#9722

Reviewed By: amitkdutta

Differential Revision: D57004604

Pulled By: zacw7

fbshipit-source-id: e792ca6ecb376ad8a95d8ce3de819c7faeac5e0a
Joe-Abraham pushed a commit to Joe-Abraham/velox that referenced this pull request Jun 7, 2024
Summary: Pull Request resolved: facebookincubator#9438

Reviewed By: xiaoxmeng

Differential Revision: D55941557

fbshipit-source-id: eb478af2e8b6eb10f9ed338222c424d4805fe005
Joe-Abraham pushed a commit to Joe-Abraham/velox that referenced this pull request Jun 7, 2024
…#9438)" (facebookincubator#9722)

Summary:
This reverts commit 8d09dc9.

Pull Request resolved: facebookincubator#9722

Reviewed By: amitkdutta

Differential Revision: D57004604

Pulled By: zacw7

fbshipit-source-id: e792ca6ecb376ad8a95d8ce3de819c7faeac5e0a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants