From 42fb47f6edab02d4abb814631c6615dd4661d7d2 Mon Sep 17 00:00:00 2001 From: Hans Wennborg Date: Mon, 14 Nov 2011 17:06:16 +0000 Subject: [PATCH] Pass system's CFLAGS, remove exit time destructor, sstable bug fix. - Pass system's values of CFLAGS,LDFLAGS. Don't override OPT if it's already set. Original patch by Alessio Treglia : http://code.google.com/p/leveldb/issues/detail?id=27#c6 - Remove 1 exit time destructor from leveldb. See http://crbug.com/101600 - Fix problem where sstable building code would pass an internal key to the user comparator. (Sync with uptream at 25436817.) --- Makefile | 8 ++++---- db/db_test.cc | 48 ++++++++++++++++++++++++++++++++++------------ db/dbformat.cc | 14 ++++++++------ util/comparator.cc | 7 +++++-- 4 files changed, 53 insertions(+), 24 deletions(-) diff --git a/Makefile b/Makefile index 18386f28b..8385d4481 100644 --- a/Makefile +++ b/Makefile @@ -8,9 +8,9 @@ CC = g++ # Uncomment exactly one of the lines labelled (A), (B), and (C) below # to switch between compilation modes. -OPT = -O2 -DNDEBUG # (A) Production use (optimized mode) -# OPT = -g2 # (B) Debug mode, w/ full line-level debugging symbols -# OPT = -O2 -g2 -DNDEBUG # (C) Profiling mode: opt, but w/debugging symbols +OPT ?= -O2 -DNDEBUG # (A) Production use (optimized mode) +# OPT ?= -g2 # (B) Debug mode, w/ full line-level debugging symbols +# OPT ?= -O2 -g2 -DNDEBUG # (C) Profiling mode: opt, but w/debugging symbols #----------------------------------------------- # detect what platform we're building on @@ -38,7 +38,7 @@ endif CFLAGS = -c -I. -I./include $(PORT_CFLAGS) $(PLATFORM_CFLAGS) $(OPT) $(SNAPPY_CFLAGS) -LDFLAGS=$(PLATFORM_LDFLAGS) $(SNAPPY_LDFLAGS) $(GOOGLE_PERFTOOLS_LDFLAGS) +LDFLAGS += $(PLATFORM_LDFLAGS) $(SNAPPY_LDFLAGS) $(GOOGLE_PERFTOOLS_LDFLAGS) LIBOBJECTS = \ ./db/builder.o \ diff --git a/db/db_test.cc b/db/db_test.cc index f1cb94921..5dc3b026d 100644 --- a/db/db_test.cc +++ b/db/db_test.cc @@ -1146,26 +1146,50 @@ TEST(DBTest, CustomComparator) { public: virtual const char* Name() const { return "test.NumberComparator"; } virtual int Compare(const Slice& a, const Slice& b) const { - return (strtol(a.ToString().c_str(), NULL, 0) - - strtol(b.ToString().c_str(), NULL, 0)); + return ToNumber(a) - ToNumber(b); + } + virtual void FindShortestSeparator(std::string* s, const Slice& l) const { + ToNumber(*s); // Check format + ToNumber(l); // Check format + } + virtual void FindShortSuccessor(std::string* key) const { + ToNumber(*key); // Check format + } + private: + static int ToNumber(const Slice& x) { + // Check that there are no extra characters. + ASSERT_TRUE(x.size() >= 2 && x[0] == '[' && x[x.size()-1] == ']') + << EscapeString(x); + int val; + char ignored; + ASSERT_TRUE(sscanf(x.ToString().c_str(), "[%i]%c", &val, &ignored) == 1) + << EscapeString(x); + return val; } - virtual void FindShortestSeparator(std::string* s, const Slice& l) const {} - virtual void FindShortSuccessor(std::string* key) const {} }; NumberComparator cmp; Options new_options; new_options.create_if_missing = true; new_options.comparator = &cmp; + new_options.write_buffer_size = 1000; // Compact more often DestroyAndReopen(&new_options); - ASSERT_OK(Put("10", "ten")); - ASSERT_OK(Put("0x14", "twenty")); + ASSERT_OK(Put("[10]", "ten")); + ASSERT_OK(Put("[0x14]", "twenty")); for (int i = 0; i < 2; i++) { - ASSERT_EQ("ten", Get("10")); - ASSERT_EQ("ten", Get("0xa")); - ASSERT_EQ("twenty", Get("20")); - ASSERT_EQ("twenty", Get("0x14")); - Compact("0", "9999"); - fprintf(stderr, "ss\n%s\n", DumpSSTableList().c_str()); + ASSERT_EQ("ten", Get("[10]")); + ASSERT_EQ("ten", Get("[0xa]")); + ASSERT_EQ("twenty", Get("[20]")); + ASSERT_EQ("twenty", Get("[0x14]")); + Compact("[0]", "[9999]"); + } + + for (int run = 0; run < 2; run++) { + for (int i = 0; i < 1000; i++) { + char buf[100]; + snprintf(buf, sizeof(buf), "[%d]", i*10); + ASSERT_OK(Put(buf, buf)); + } + Compact("[0]", "[1000000]"); } } diff --git a/db/dbformat.cc b/db/dbformat.cc index 4594a5700..9168f99df 100644 --- a/db/dbformat.cc +++ b/db/dbformat.cc @@ -73,9 +73,10 @@ void InternalKeyComparator::FindShortestSeparator( Slice user_limit = ExtractUserKey(limit); std::string tmp(user_start.data(), user_start.size()); user_comparator_->FindShortestSeparator(&tmp, user_limit); - if (user_comparator_->Compare(*start, tmp) < 0) { - // User key has become larger. Tack on the earliest possible - // number to the shortened user key. + if (tmp.size() < user_start.size() && + user_comparator_->Compare(user_start, tmp) < 0) { + // User key has become shorter physically, but larger logically. + // Tack on the earliest possible number to the shortened user key. PutFixed64(&tmp, PackSequenceAndType(kMaxSequenceNumber,kValueTypeForSeek)); assert(this->Compare(*start, tmp) < 0); assert(this->Compare(tmp, limit) < 0); @@ -87,9 +88,10 @@ void InternalKeyComparator::FindShortSuccessor(std::string* key) const { Slice user_key = ExtractUserKey(*key); std::string tmp(user_key.data(), user_key.size()); user_comparator_->FindShortSuccessor(&tmp); - if (user_comparator_->Compare(user_key, tmp) < 0) { - // User key has become larger. Tack on the earliest possible - // number to the shortened user key. + if (tmp.size() < user_key.size() && + user_comparator_->Compare(user_key, tmp) < 0) { + // User key has become shorter physically, but larger logically. + // Tack on the earliest possible number to the shortened user key. PutFixed64(&tmp, PackSequenceAndType(kMaxSequenceNumber,kValueTypeForSeek)); assert(this->Compare(*key, tmp) < 0); key->swap(tmp); diff --git a/util/comparator.cc b/util/comparator.cc index cfb49ceed..eed9d2fcd 100644 --- a/util/comparator.cc +++ b/util/comparator.cc @@ -64,10 +64,13 @@ class BytewiseComparatorImpl : public Comparator { } }; } // namespace -static const BytewiseComparatorImpl bytewise; + +// Intentionally not destroyed to prevent destructor racing +// with background threads. +static const Comparator* bytewise = new BytewiseComparatorImpl; const Comparator* BytewiseComparator() { - return &bytewise; + return bytewise; } } // namespace leveldb