Auto resume the DB from Retryable IO Error #6765
Conversation
zhichao-cao
force-pushed
the
auto_resume
branch
from
cd5e8b0 to
8f46055
Compare
zhichao-cao
force-pushed
the
auto_resume
branch
from
8f46055 to
0cfdc6b
Compare
There was a problem hiding this comment.
@zhichao-cao Thanks for the PR! I've partially reviewed and added some comments. Will continue to review.
There was a problem hiding this comment.
@zhichao-cao has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
include/rocksdb/options.h
Outdated
| @@ -1144,6 +1144,22 @@ struct DBOptions { | |||
| // not be used for recovery if best_efforts_recovery is true. | |||
| // Default: false | |||
| bool best_efforts_recovery = false; | |||
|
|
|||
| // It defines How many times call db resume when retryable BGError happens. | |||
There was a problem hiding this comment.
Does somebody want to tune these knobs? How about always making infinite recovery attempts with a one second gap?
There was a problem hiding this comment.
Yes, that's a good idea. We probably just need an option to enable/disable the functionality in case of bugs.
There was a problem hiding this comment.
@anand1976 You mean, I can remove the options of max_bgerror_resume_count and bgerror_resume_retry_interval, set value directly in the resume function. Instead, we give a option for user to enable or disable auto resume? The resume will be infinite attempts as suggested by Andrew?
There was a problem hiding this comment.
@zhichao-cao Thinking about this some more, I think there's value in allowing the retry interval to be adjusted. The flushes initiated by the recovery threads share the same background thread pool as regular flushes, so if you have a mix of good and bad DBs, changing the retry interval will allow a user to control resource consumption for recovery. The retry count is debatable. We could set the default to infinite and user can set it to 0 to disable. I don't know if there would be a reason to choose some value in between. Again, maybe to limit resource consumption.
|
@zhichao-cao has updated the pull request. Re-import the pull request |
1 similar comment
|
@zhichao-cao has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
@zhichao-cao has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@zhichao-cao has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
@zhichao-cao has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@zhichao-cao has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
@zhichao-cao has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@zhichao-cao has updated the pull request. You must reimport the pull request before landing. |
There was a problem hiding this comment.
@zhichao-cao has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@zhichao-cao has updated the pull request. You must reimport the pull request before landing. |
…for auto resume, code works
|
@zhichao-cao has updated the pull request. You must reimport the pull request before landing. |
There was a problem hiding this comment.
@zhichao-cao has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@zhichao-cao merged this pull request in a10f12e. |
This reverts commit a10f12e.
Summary: Remove the 3 testing cases that cause the time out in linux build by #6765 . Will fix them later. Pull Request resolved: #7141 Test Plan: make asan_check, buck run Reviewed By: ajkr Differential Revision: D22593831 Pulled By: zhichao-cao fbshipit-source-id: 14956c36476ecc3393f613178c22e13df843126e
In current codebase, in write path, if Retryable IO Error happens, SetBGError is called. The retryable IO Error is converted to hard error and DB is in read only mode. User or application needs to resume it. In this PR, if Retryable IO Error happens in one DB, SetBGError will create a new thread to call Resume (auto resume). otpions.max_bgerror_resume_count controls if auto resume is enabled or not (if max_bgerror_resume_count<=0, auto resume will not be enabled). options.bgerror_resume_retry_interval controls the time interval to call Resume again if the previous resume fails due to the Retryable IO Error. If non-retryable error happens during resume, auto resume will terminate.
Test plan: Added the unit test cases in error_handler_fs_test and pass make asan_check