Fix a bug in C-binding causing iterator to return incorrect result (#9343)

Summary:
Fixes https://github.com/facebook/rocksdb/issues/9339

When writing SST file, the name, computed as `prefix_extractor->GetId()` will be written to the properties block.
When the SST is opened again in the future, `CreateFromString()` will take the name as argument and try
to create a prefix extractor object. Without this fix, the C API will pass a `Wrapper` pointer to the underlying
DB's `prefix_extractor`. `Wrapper::GetId()`, in this case, will be missing the prefix length component, causing a
prefix extractor of length 0 to be silently created and used.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/9343

Test Plan:
```
make c_test
./c_test
```

Reviewed By: mrambacher

Differential Revision: D33355549

Pulled By: riversand963

fbshipit-source-id: c92c3acd8be262c3bff8794b4229e42b9ee31203
main
Yanqin Jin 3 years ago committed by Facebook GitHub Bot
parent a931bacf5d
commit 677d2b4a8f
  1. 1
      HISTORY.md
  2. 13
      db/c.cc
  3. 45
      db/c_test.c

@ -16,6 +16,7 @@
* Fixed a bug causing two duplicate entries to be appended to a file opened in non-direct mode and tracked by `FaultInjectionTestFS`. * Fixed a bug causing two duplicate entries to be appended to a file opened in non-direct mode and tracked by `FaultInjectionTestFS`.
* Fixed a bug in TableOptions.prepopulate_block_cache to support block-based filters also. * Fixed a bug in TableOptions.prepopulate_block_cache to support block-based filters also.
* Block cache keys no longer use `FSRandomAccessFile::GetUniqueId()` (previously used when available), so a filesystem recycling unique ids can no longer lead to incorrect result or crash (#7405). For files generated by RocksDB >= 6.24, the cache keys are stable across DB::Open and DB directory move / copy / import / export / migration, etc. Although collisions are still theoretically possible, they are (a) impossible in many common cases, (b) not dependent on environmental factors, and (c) much less likely than a CPU miscalculation while executing RocksDB. * Block cache keys no longer use `FSRandomAccessFile::GetUniqueId()` (previously used when available), so a filesystem recycling unique ids can no longer lead to incorrect result or crash (#7405). For files generated by RocksDB >= 6.24, the cache keys are stable across DB::Open and DB directory move / copy / import / export / migration, etc. Although collisions are still theoretically possible, they are (a) impossible in many common cases, (b) not dependent on environmental factors, and (c) much less likely than a CPU miscalculation while executing RocksDB.
* Fixed a bug in C bindings causing iterator to return incorrect result (#9343).
### Behavior Changes ### Behavior Changes
* MemTableList::TrimHistory now use allocated bytes when max_write_buffer_size_to_maintain > 0(default in TrasactionDB, introduced in PR#5022) Fix #8371. * MemTableList::TrimHistory now use allocated bytes when max_write_buffer_size_to_maintain > 0(default in TrasactionDB, introduced in PR#5022) Fix #8371.

@ -4595,10 +4595,11 @@ void rocksdb_slicetransform_destroy(rocksdb_slicetransform_t* st) {
delete st; delete st;
} }
struct Wrapper : public rocksdb_slicetransform_t { struct SliceTransformWrapper : public rocksdb_slicetransform_t {
const SliceTransform* rep_; const SliceTransform* rep_;
~Wrapper() override { delete rep_; } ~SliceTransformWrapper() override { delete rep_; }
const char* Name() const override { return rep_->Name(); } const char* Name() const override { return rep_->Name(); }
std::string GetId() const override { return rep_->GetId(); }
Slice Transform(const Slice& src) const override { Slice Transform(const Slice& src) const override {
return rep_->Transform(src); return rep_->Transform(src);
} }
@ -4610,18 +4611,18 @@ struct Wrapper : public rocksdb_slicetransform_t {
}; };
rocksdb_slicetransform_t* rocksdb_slicetransform_create_fixed_prefix(size_t prefixLen) { rocksdb_slicetransform_t* rocksdb_slicetransform_create_fixed_prefix(size_t prefixLen) {
Wrapper* wrapper = new Wrapper; SliceTransformWrapper* wrapper = new SliceTransformWrapper;
wrapper->rep_ = ROCKSDB_NAMESPACE::NewFixedPrefixTransform(prefixLen); wrapper->rep_ = ROCKSDB_NAMESPACE::NewFixedPrefixTransform(prefixLen);
wrapper->state_ = nullptr; wrapper->state_ = nullptr;
wrapper->destructor_ = &Wrapper::DoNothing; wrapper->destructor_ = &SliceTransformWrapper::DoNothing;
return wrapper; return wrapper;
} }
rocksdb_slicetransform_t* rocksdb_slicetransform_create_noop() { rocksdb_slicetransform_t* rocksdb_slicetransform_create_noop() {
Wrapper* wrapper = new Wrapper; SliceTransformWrapper* wrapper = new SliceTransformWrapper;
wrapper->rep_ = ROCKSDB_NAMESPACE::NewNoopTransform(); wrapper->rep_ = ROCKSDB_NAMESPACE::NewNoopTransform();
wrapper->state_ = nullptr; wrapper->state_ = nullptr;
wrapper->destructor_ = &Wrapper::DoNothing; wrapper->destructor_ = &SliceTransformWrapper::DoNothing;
return wrapper; return wrapper;
} }

@ -2956,6 +2956,51 @@ int main(int argc, char** argv) {
CheckNoError(err); CheckNoError(err);
} }
StartPhase("filter_with_prefix_seek");
{
rocksdb_close(db);
rocksdb_destroy_db(options, dbname, &err);
CheckNoError(err);
rocksdb_options_set_prefix_extractor(
options, rocksdb_slicetransform_create_fixed_prefix(1));
rocksdb_filterpolicy_t* filter_policy =
rocksdb_filterpolicy_create_bloom_full(8.0);
rocksdb_block_based_options_set_filter_policy(table_options, filter_policy);
rocksdb_options_set_block_based_table_factory(options, table_options);
db = rocksdb_open(options, dbname, &err);
CheckNoError(err);
int i;
for (i = 0; i < 10; ++i) {
char key = '0' + (char)i;
rocksdb_put(db, woptions, &key, 1, "", 1, &err);
CheckNoError(err);
}
// Flush to generate an L0 so that filter will be used later.
rocksdb_flushoptions_t* flush_options = rocksdb_flushoptions_create();
rocksdb_flushoptions_set_wait(flush_options, 1);
rocksdb_flush(db, flush_options, &err);
rocksdb_flushoptions_destroy(flush_options);
CheckNoError(err);
rocksdb_readoptions_t* ropts = rocksdb_readoptions_create();
rocksdb_iterator_t* iter = rocksdb_create_iterator(db, ropts);
rocksdb_iter_seek(iter, "0", 1);
int cnt = 0;
while (rocksdb_iter_valid(iter)) {
++cnt;
rocksdb_iter_next(iter);
}
CheckCondition(10 == cnt);
rocksdb_iter_destroy(iter);
rocksdb_readoptions_destroy(ropts);
}
StartPhase("cancel_all_background_work"); StartPhase("cancel_all_background_work");
rocksdb_cancel_all_background_work(db, 1); rocksdb_cancel_all_background_work(db, 1);

Loading…
Cancel
Save