Fail multi scan upon Prepare failure or bad scan options #13974
Conversation
|
I suspect the failed tests are caused by the conflict in this change. #13970. You might have to merge. |
| // Set up the user-defined index factory | ||
| auto user_defined_index_factory = | ||
| std::make_shared<TestUserDefinedIndexFactory>(); | ||
| table_options.user_defined_index_factory = user_defined_index_factory; | ||
|
|
||
| // Set up custom flush block policy that flushes every 3 keys | ||
| table_options.flush_block_policy_factory = | ||
| std::make_shared<CustomFlushBlockPolicyFactory>(); | ||
|
|
||
| options.table_factory.reset(NewBlockBasedTableFactory(table_options)); |
There was a problem hiding this comment.
It might be worth to dedup this code with other UDI unit tests.
There was a problem hiding this comment.
Let me defer it to a follow-on PR. I'm trying to land this before the release branch cut.
|
@anand1976 has imported this pull request. If you are a Meta employee, you can view this in D82843944. |
|
Stress test can fail now whenever we abort Prepare()? |
It could. Do you see an issue with it? I think db_stress generates the ranges in order with valid upper bounds, so Prepare() shouldn't fail in the normal course. |
I think L0 file may not work properly as discussed offline. DB iter internal reseeks or range deletion reseeks may also fail the iterator. |
anand1976
force-pushed
the
prepare_fail
branch
from
fe0acf2 to
ddf2dad
Compare
|
@anand1976 has imported this pull request. If you are a Meta employee, you can view this in D82843944. |
| for (auto i = fstart; i <= fend; i++) { | ||
| if (i < flevel_->num_files) { | ||
| auto args = GetMultiScanArgForFile(i); | ||
| auto& args = GetMultiScanArgForFile(i); |
|
@anand1976 has imported this pull request. If you are a Meta employee, you can view this in D82843944. |
|
@anand1976 merged this pull request in afbbc90. |
Summary: Return a failure status for multi scan if Prepare fails, or if the scan options are unsupported, instead of falling back on a regular scan. This PR also fixes a bug in LevelIterator that caused max_prefetch_size to be ignored. Pull Request resolved: #13974 Test Plan: Add new test in db_iterator_test and table_test Reviewed By: xingbowang Differential Revision: D82843944 Pulled By: anand1976 fbshipit-source-id: f12756c40ebd38d8d4e4425e97438b6e766a4663
Return a failure status for multi scan if Prepare fails, or if the scan options are unsupported, instead of falling back on a regular scan. This PR also fixes a bug in LevelIterator that caused max_prefetch_size to be ignored.
Test plan:
Add new test in db_iterator_test and table_test