Fix a bug in table builder

Summary:
In talbe.cc, when reading the metablock, it uses BytewiseComparator();
However in table_builder.cc, we use r->options.comparator. After tracing
the creation of r->options.comparator, I found this comparator is an
InternalKeyComparator, which wraps the user defined comparator(details
can be found in DBImpl::SanitizeOptions().

I encountered this problem when adding metadata about "bloom filter"
before. With different comparator, we may fail to do the binary sort.

Current code works well since there is only one entry in meta block.

Test Plan:
make all check

I've also tested this change in https://reviews.facebook.net/D8283 before.

Reviewers: dhruba, haobo

CC: leveldb

Differential Revision: https://reviews.facebook.net/D13335
main
Kai Liu 11 years ago
parent fa46ddb41f
commit 1f8ade6bd6
  1. 20
      table/block_builder.cc
  2. 14
      table/block_builder.h
  3. 6
      table/table_builder.cc

@ -36,15 +36,21 @@
namespace rocksdb { namespace rocksdb {
BlockBuilder::BlockBuilder(const Options* options) BlockBuilder::BlockBuilder(int block_restart_interval,
: options_(options), const Comparator* comparator)
: block_restart_interval_(block_restart_interval),
comparator_(comparator),
restarts_(), restarts_(),
counter_(0), counter_(0),
finished_(false) { finished_(false) {
assert(options->block_restart_interval >= 1); assert(block_restart_interval_ >= 1);
restarts_.push_back(0); // First restart point is at offset 0 restarts_.push_back(0); // First restart point is at offset 0
} }
BlockBuilder::BlockBuilder(const Options* options)
: BlockBuilder(options->block_restart_interval, options->comparator) {
}
void BlockBuilder::Reset() { void BlockBuilder::Reset() {
buffer_.clear(); buffer_.clear();
restarts_.clear(); restarts_.clear();
@ -64,7 +70,7 @@ size_t BlockBuilder::EstimateSizeAfterKV(const Slice& key, const Slice& value)
const { const {
size_t estimate = CurrentSizeEstimate(); size_t estimate = CurrentSizeEstimate();
estimate += key.size() + value.size(); estimate += key.size() + value.size();
if (counter_ >= options_->block_restart_interval) { if (counter_ >= block_restart_interval_) {
estimate += sizeof(uint32_t); // a new restart entry. estimate += sizeof(uint32_t); // a new restart entry.
} }
@ -88,11 +94,11 @@ Slice BlockBuilder::Finish() {
void BlockBuilder::Add(const Slice& key, const Slice& value) { void BlockBuilder::Add(const Slice& key, const Slice& value) {
Slice last_key_piece(last_key_); Slice last_key_piece(last_key_);
assert(!finished_); assert(!finished_);
assert(counter_ <= options_->block_restart_interval); assert(counter_ <= block_restart_interval_);
assert(buffer_.empty() // No values yet? assert(buffer_.empty() // No values yet?
|| options_->comparator->Compare(key, last_key_piece) > 0); || comparator_->Compare(key, last_key_piece) > 0);
size_t shared = 0; size_t shared = 0;
if (counter_ < options_->block_restart_interval) { if (counter_ < block_restart_interval_) {
// See how much sharing to do with previous string // See how much sharing to do with previous string
const size_t min_length = std::min(last_key_piece.size(), key.size()); const size_t min_length = std::min(last_key_piece.size(), key.size());
while ((shared < min_length) && (last_key_piece[shared] == key[shared])) { while ((shared < min_length) && (last_key_piece[shared] == key[shared])) {

@ -11,9 +11,11 @@
namespace rocksdb { namespace rocksdb {
struct Options; struct Options;
class Comparator;
class BlockBuilder { class BlockBuilder {
public: public:
BlockBuilder(int block_builder, const Comparator* comparator);
explicit BlockBuilder(const Options* options); explicit BlockBuilder(const Options* options);
// Reset the contents as if the BlockBuilder was just constructed. // Reset the contents as if the BlockBuilder was just constructed.
@ -41,11 +43,13 @@ class BlockBuilder {
} }
private: private:
const Options* options_; const int block_restart_interval_;
std::string buffer_; // Destination buffer const Comparator* comparator_;
std::vector<uint32_t> restarts_; // Restart points
int counter_; // Number of entries emitted since restart std::string buffer_; // Destination buffer
bool finished_; // Has Finish() been called? std::vector<uint32_t> restarts_; // Restart points
int counter_; // Number of entries emitted since restart
bool finished_; // Has Finish() been called?
std::string last_key_; std::string last_key_;
// No copying allowed // No copying allowed

@ -269,7 +269,11 @@ Status TableBuilder::Finish() {
// Write metaindex block // Write metaindex block
if (ok()) { if (ok()) {
BlockBuilder meta_index_block(&r->options); // We use `BytewiseComparator` as the comparator for meta block.
BlockBuilder meta_index_block(
r->options.block_restart_interval,
BytewiseComparator()
);
if (r->filter_block != nullptr) { if (r->filter_block != nullptr) {
// Add mapping from "filter.Name" to location of filter data // Add mapping from "filter.Name" to location of filter data
std::string key = "filter."; std::string key = "filter.";

Loading…
Cancel
Save