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

Fix incorrect Status::NoSpace() status check #8504

Closed
wants to merge 1 commit into from

Conversation

zhichao-cao
Copy link
Contributor

If we want to check whether a Status s is NoSpace() or not, we should check the subcode instread of using s==Status::NoSpace(). Fix some of the incorrect check in the ErrorHandler.

test plan: make check

@facebook-github-bot
Copy link
Contributor

@zhichao-cao has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@zhichao-cao
Copy link
Contributor Author

Looks like it causes the testing issue in db_sst_test-DBSSTTest.DBWithMaxSpaceAllowedRandomized, need to be investigated

Copy link
Contributor

@anand1976 anand1976 left a comment

Choose a reason for hiding this comment

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

LGTM

@anand1976
Copy link
Contributor

Nit: Update HISTORY.md

@facebook-github-bot
Copy link
Contributor

@zhichao-cao has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot
Copy link
Contributor

@zhichao-cao has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@zhichao-cao merged this pull request in 87e82a4.

jaepil pushed a commit to deepsearch-hq/rocksdb-backup that referenced this pull request Jul 23, 2021
Summary:
If we want to check whether a Status s is NoSpace() or not, we should check the subcode instread of using s==Status::NoSpace(). Fix some of the incorrect check in the ErrorHandler.

Pull Request resolved: facebook#8504

Test Plan: make check

Reviewed By: anand1976

Differential Revision: D29601764

Pulled By: zhichao-cao

fbshipit-source-id: cdab56a827891c23746bba9cbb53f169fe35f086
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.

3 participants